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

Schema loading from the wire #307

Merged
merged 2 commits into from
Jun 19, 2023
Merged

Conversation

DaneSlattery
Copy link

This PR adds the ability to serialize a Schema.Node into a DynamicStruct.Reader, so that schemas can be defined on one end of the channel, and parsed on the other.

The PR also implements the SchemaLoader, which can load from a Schema.Node or a DynamicStruct. This feature is also requested in this issue: #291

Needs cleanup:

  • Refactor unit test capnp schema
  • Move C helper into own file
  • Add factory to create _DynamicStructBuilder from schema
  • Formatting

@DaneSlattery
Copy link
Author

DaneSlattery commented Mar 19, 2023

@haata are you happy with the features this enables and the interface? I'm going to try get this to pass this week.
Todo:

Another MR to expose getAllLoaded will probably be in the pipeline eventually. That will enable pushing a schema onto the wire with all of its dependencies.

@haata
Copy link
Collaborator

haata commented Mar 20, 2023

Yeah, I think once you finish your todo list it will look good.
No complaints about what's in the MR so far.

Make sure to rebase, I fixed some linting errors recently.

@DaneSlattery
Copy link
Author

Could I ask that we tag this as a minor release so that we can utilise it in our project? I have been trying to distribute the builds using cibuildwheels on my local windows machine with NO success. I guess we could use builds from our fork though.

@DaneSlattery
Copy link
Author

DaneSlattery commented Jun 6, 2023

CI seems to be failing on an unrelated test for SSL, but on our github fork we've managed to get this to pass

@haata
Copy link
Collaborator

haata commented Jun 6, 2023

I merged one of the incoming large changes (one that should help a lot with SSL). Unfortunately you'll need to rebase your changes.

Cap'n Proto provides a schema loader, which can be used to dynamically
load schemas during runtime. To port this functionality to pycapnp,
a new class is provided `C_SchemaLoader`, which exposes the Cap'n
Proto C++ interface, and `SchemaLoader`, which is part of the pycapnp
library.

The specific use case for this is when a capnp message contains
a Node.Reader: The schema for a yet unseen message can be loaded
dynamically, allowing the future message to be properly processed.

If the message is a struct containing other structs, all the schemas for
every struct must be loaded to correctly parse the message. See
https://github.com/DaneSlattery/capnp_generic_poc for a
proof-of-concept.

Add docs and cleanup

Add more docs

Reduce changes

Fix flake8 formatting

Fix get datatype
@DaneSlattery
Copy link
Author

Adjusted my tests to be async, given the new changes that remove synchronous RPC

@haata haata merged commit 8f3bfc3 into capnproto:master Jun 19, 2023
@haata
Copy link
Collaborator

haata commented Jun 19, 2023

It's in!

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.

2 participants