-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dvl/klaus/webadm readable msg #85
Open
klausfmh
wants to merge
9
commits into
mhcomm:master
Choose a base branch
from
klausfmh:dvl/klaus/webadm_readable_msg
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
d63ef37
enable readable messages in admin web interface
klausfmh 05f1e6e
first shot at readable messages should be working
klausfmh 36754ca
handle feedback on PR
efbd2f3
forgotten commit with more features for encoding
581e75c
typo
87160bb
feedback for PR (fix a comment)
klausfmh 867d1e3
add B64PickleEncoder (flake8)
9e923b3
add encoder param to message
6de03e9
add tests for serializers
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
""" | ||
serializers, that might be helpful for pypeman projects | ||
""" | ||
|
||
import json | ||
import logging | ||
import pickle | ||
|
||
from base64 import b64decode | ||
from base64 import b64encode | ||
|
||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
||
# attempt to create serializers allowing to encode objects for non | ||
# Python applications. | ||
class JsonableEncoder: | ||
""" Initial simple encoder/decoder for objects. | ||
returns: a dict with type and value | ||
type: 'asis' or 'b64', 'repr' | ||
value: | ||
- the object itself if it can be json encoded (basetype or | ||
struct of base types) | ||
- a b64 encoded string if obj is of type bytes and not ASCII | ||
(- a slightly transformed object, that can be json encoded | ||
(To be implemented)) | ||
- a repr string of the object (which can normally not be decoded) | ||
""" | ||
def encode(self, obj): | ||
logger.debug("try to encode type %s (%r)", type(obj), obj) | ||
result = { | ||
'type': 'asis', | ||
'value': obj, | ||
'repr': '', | ||
} | ||
if isinstance(obj, bytes): | ||
try: | ||
result['type'] = "bytes" | ||
result['value'] = obj.decode('ascii') # OR UTF-8? | ||
return result | ||
except UnicodeDecodeError: | ||
pass | ||
try: | ||
result['type'] = "utf8bytes" | ||
# TODO could merge asis and utf8 | ||
result['value'] = obj.decode('utf-8') | ||
return result | ||
except UnicodeDecodeError: | ||
result['type'] = "b64" | ||
result['value'] = b64encode(obj).decode() | ||
result['repr'] = obj.decode('utf-8', 'ignore') | ||
return result | ||
try: | ||
# logger.info("try to dump %s", repr(obj)) | ||
json_str = json.dumps(obj) # noqa this line will except if not jsonable | ||
# logger.info("can be dumped as json %s", json_str) | ||
return result | ||
except TypeError: | ||
pass | ||
result['type'] = "repr" | ||
# TODO: might add a base64 pickle for the repr case | ||
result['value'] = None | ||
result['repr'] = repr(obj) | ||
return result | ||
|
||
def decode(self, encoded_obj): | ||
enc_type = encoded_obj['type'] | ||
data = encoded_obj['value'] | ||
if enc_type == 'asis': | ||
return data | ||
elif enc_type in ('bytes', 'utf8bytes'): | ||
return data.encode('utf-8') | ||
elif enc_type == 'b64': | ||
return b64decode(data.encode('ascii')) | ||
|
||
|
||
class B64PickleEncoder: | ||
""" Initial simple encoder for objects. | ||
objects are encoded, such, that they can be stored / transfered as | ||
an ASCII string. | ||
|
||
drawback: the encoded object can only be decoded if the | ||
application handles pickle. This limits this decoding mostly | ||
to python3 applications. | ||
""" | ||
|
||
def encode(self, obj): | ||
""" the encoding function | ||
:param obj: the object to be encoded | ||
returns: a b64 encoded string of the pickled bytes of the passed | ||
object | ||
""" | ||
return b64encode(pickle.dumps(obj)).decode('ascii') | ||
|
||
def decode(self, encoded_obj): | ||
""" the decoding function | ||
:param encoded_obj: the encoded_object to be decoded | ||
""" | ||
return pickle.loads(b64decode(encoded_obj.encode('ascii'))) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -141,19 +141,34 @@ async def stop_channel(self, channel): | |
'status': channels.BaseChannel.status_id_to_str(chan.status) | ||
} | ||
|
||
async def list_msg(self, channel, start=0, count=10, order_by='timestamp'): | ||
async def list_msg(self, channel, start=0, count=10, order_by='timestamp', mk_b64pickle=False): | ||
""" | ||
List first `count` messages from message store of specified channel. | ||
|
||
:params channel: The channel name. | ||
:param mk_b64pickle: if True (yield payload / ctx fields as b64 encoded pickles) | ||
if False a a dict with a field type, field value and a field repr | ||
will be created. | ||
Field types depend on the channel's jsonable_msg_info_for_admin() method | ||
typical values are: | ||
"asis": value is the object 'asis', as the object could be pickled | ||
"bytes": bytes just as ASCII string | ||
"utf8bytes": bytes as UTF8 string | ||
"b64": base 64 encoded bytes | ||
"repr": a repr() string of the object is returned | ||
"b64pickle": all fields b64 encoded pickles of the related python object | ||
""" | ||
chan = self.get_channel(channel) | ||
|
||
messages = await chan.message_store.search(start=start, count=count, order_by=order_by) | ||
|
||
for res in messages: | ||
res['timestamp'] = res['message'].timestamp_str() | ||
res['message'] = res['message'].to_json() | ||
if mk_b64pickle: | ||
res['message'] = res['message'].to_json() | ||
else: | ||
msg = res['message'] | ||
res['message'] = chan.jsonable_msg_info_for_admin(msg) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Again, why is this not a message method ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. see comment above |
||
|
||
return {'messages': messages, 'total': await chan.message_store.total()} | ||
|
||
|
@@ -180,7 +195,7 @@ async def push_msg(self, channel, text): | |
Push a message in the channel. | ||
|
||
:params channel: The channel name. | ||
:params msg_ids: The text added to the payload. | ||
:params text: The text added to the payload. | ||
""" | ||
chan = self.get_channel(channel) | ||
msg = message.Message(payload=text) | ||
|
@@ -248,7 +263,8 @@ def stop(self, channel): | |
""" | ||
return self.send_command('stop_channel', [channel]) | ||
|
||
def list_msg(self, channel, start=0, count=10, order_by='timestamp'): | ||
def list_msg(self, channel, start=0, count=10, order_by='timestamp', | ||
mk_b64pickle=True): | ||
""" | ||
List first 10 messages on specified channel from remote instance. | ||
|
||
|
@@ -258,7 +274,8 @@ def list_msg(self, channel, start=0, count=10, order_by='timestamp'): | |
:params order_by: Message order. only 'timestamp' and '-timestamp' handled for now. | ||
:returns: list of message with status. | ||
""" | ||
result = self.send_command('list_msg', [channel, start, count, order_by]) | ||
result = self.send_command('list_msg', [channel, start, count, | ||
order_by, mk_b64pickle]) | ||
|
||
for m in result['messages']: | ||
m['message'] = message.Message.from_json(m['message']) | ||
|
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this is not a
Message
method ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each channel handles different kinds of messages.
I imagine the usecase where a channel wants to have a different representation of a message.
Let's say an MLLP channel would prefer to have perhaps a partially preparsed representation for the web interface,
A file watcher channel might not display the contents of the file, but more stats about the file.
An alternative would of course be to put this method into the channel and let certain endpoints / channels subclass the Message object. (Though this might imply more data copies)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems fair.
New proposal : you add the method to the
Message
class but you allow to redefine default payload_encoder with an argument. Then you add a method to the channel that call theMessage
method with the appropriate encoder. Later (i.e. in another PR) we can add a way to custom encoder by a channel argument.What do you think ?
I think you can use current
.to_dict()
method and add the correct argument to achieve that ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After more thinking, more details :
b64picklerEncoder
andJsonableEncoder
,payload_encoder
option toMessage.to_json()
method withb64picklerEncoder
as default encoder (to keep compatibility) and propagate the option to.to_dict()
method,message_payload_encoder
property to channel with b64 default encoder with default value to b64encoder,.list_message()
while calling.to_json()
to get your formatted message.Do I miss something ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should the param for message_payload_encoder be
a class, an instance (which must have the method encode or just a function that encodes).
Will make a first start with an instance, but I am open.
and the more I think the less I wonder why not just passing a function, except we want to group the encode and decode function in the same office.
In that case we should perhaps call the param
payload_codec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad. pushed only to local gitlab server and not to github
Just check klausfmh@297b219
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In fact best to recheck all the deltas of this MR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder to continue review :-)
I think I handled all the FB. pls correct me if wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes that's what i think for message but part :
is not yet done ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
jsonable_msg_info_for_admin
channel method should not exists anymore at the end.