Skip to content

Support dynamic encodings - #131

Open
carver wants to merge 3 commits into
ApeWorX:mainfrom
carver:test-dynamic-structures
Open

Support dynamic encodings#131
carver wants to merge 3 commits into
ApeWorX:mainfrom
carver:test-dynamic-structures

Conversation

@carver

@carver carver commented Feb 24, 2021

Copy link
Copy Markdown
Contributor

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.
@carver
carver force-pushed the test-dynamic-structures branch from b88b361 to 173bdd8 Compare February 24, 2021 22:41
... and resolve new warnings
@carver
carver force-pushed the test-dynamic-structures branch from 173bdd8 to bf46cfd Compare February 24, 2021 22:43
@carver
carver marked this pull request as ready for review February 24, 2021 22:47
@carver

carver commented Feb 24, 2021

Copy link
Copy Markdown
Contributor Author

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

Comment thread rlp/sedes/dynamic.py
try:
return sedes.deserialize(obj)
except ObjectDeserializationError:
pass

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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.

pacrob pushed a commit to pacrob/pyrlp that referenced this pull request Feb 1, 2025
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