Skip to content
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

Address issues related to DataTransmitMessage's data field type and handling of binary data #486

Closed
aaraney opened this issue Jan 23, 2024 · 2 comments
Assignees
Labels
maas MaaS Workstream

Comments

@aaraney
Copy link
Member

aaraney commented Jan 23, 2024

The original title for this issue was Change DataTransmitMessage's data field type to accommodate binary data. See discussion here and here.

It relates to #424 and may still have implications on #432.

However, as noted below, we cannot simply change the data field from str to bytes, as this breaks the implementation for serialization. That said, this leaves in question whether/how we can support anything beyond unicode data. This issue should track further discussion and work on addressing this.

@aaraney aaraney added the Urgent This needs to be addressed as soon as possible label Jan 23, 2024
@robertbartel
Copy link
Contributor

robertbartel commented Jan 23, 2024

As noted over in the other issue comments, it looks like this would break JSON serialization for DataTransmitMessage.

@robertbartel robertbartel changed the title Change DataTransmitMessage's data field type to accommodate binary data Address issues related to DataTransmitMessage's data field type and handling of binary data Jan 24, 2024
@robertbartel robertbartel added maas MaaS Workstream and removed Urgent This needs to be addressed as soon as possible labels Jan 24, 2024
@aaraney
Copy link
Member Author

aaraney commented Jan 24, 2024

Yeah you are right, @robertbartel. I was not using all of my brain when I was writing this up 😅. There does seem to be a larger issue that we cannot upload binary data without first encoding it in something like base64 or base85, but we should address that in a separate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maas MaaS Workstream
Projects
None yet
Development

No branches or pull requests

2 participants