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

Support dynamic encodings #131

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

carver
Copy link
Contributor

@carver carver commented Feb 24, 2021

Demonstrate how to use dynamic structures in a test, and expose a DynamicSerializable.

I suppose this can fix #112 if it gets merged. It's so small that I'm not sure it's necessary. Then again, I didn't realize it would be such a small addition until I wrote it.

TODO:

  • raise exception if none of the encodings/decodings work
  • add new exception for the dynamic serialization type, to clarify that the failure was against all the possible options

The setup is slow, so skipping it in normal pytest ways is ineffective.
Now it's easier to run something like `pytest tests/core -k mytest`
without incurring the slow benchmark setup.
... and resolve new warnings
@carver carver marked this pull request as ready for review February 24, 2021 22:47
@carver
Copy link
Contributor Author

carver commented Feb 24, 2021

This PR has a bit of housekeeping. The relevant bits are in the 2nd commit: 97d4771

try:
return sedes.deserialize(obj)
except ObjectDeserializationError:
pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if all of the sedes options result in ObjectDeserializationError. Won't that result in this returning None?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sorry I "marked for review" a bit early. That's one of the TODOs listed above.

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

Successfully merging this pull request may close these issues.

Cannot (de)serialize objects with optional fields
2 participants