-
Notifications
You must be signed in to change notification settings - Fork 124
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
Dynamic load
#291
Comments
@kentonv Thanks for the explanation here: capnproto/capnproto#1532 I know this is a different sub project to the C++ one, but as a first step would it be reasonable to provide a PR which prevents the file write / read hack above and simply allows a schema string to be passed in. The second step would be to start investigating any differences between the current code with @haata any thoughts? |
Note that I would NOT recommend using the same |
Okay, so if I understand correctly, the current implementation in Allowing dynamic loading can be implemented as a Also, would they be namespaced somehow because of using a different Assuming all of that is correct, would it be okay to add a PR? |
To be clear I only know the C++ side, I haven't looked at the Python implementation in about 8 years. @haata will have to comment on the Python side. But I would guess that it wasn't an "intentional" decision to prevent importing from virtual files, I think it was probably just the easiest code to write. I think it would be fine to support importing virtual files, it's just some work that needs to be done. (And there should probably be a warning not to use it for untrusted input.) For untrusted inputs, you need a separate SchemaLoader, yes (and you should only be accepting the schemas in compiled schema.capnp format, not in text |
I'm not opposed to something like this. Just need to make sure it doesn't affect the common use-case too much. |
Okay no worries, I think we will prototype this locally with the above workaround to see if what we are doing makes sense and then provide a PR to discuss in more detail. What we are hoping to do is provide an automatic recursive serde system in our python code base which uses capnp under the hood and then for instances where we need performance and zero copy semantics we will implement manually constructed capnp serde logic. The primary reason being we had protobuf for this but protobuf maxing at 2GB and not supporting zero copy became a bottleneck for us. Right now we have both protobuf and capnp in our code base and we are going to remove protobuf and use capnp exclusively for our bytes format between systems (hopefully including js). Ill report back when we have some more meaningful examples to show. |
With respect to dynamic schema loading discussed in this issue, a colleague of mine should be in touch in the coming weeks about a PR for potential inclusion (after some extension and clean up our end) based off tinkering done in this branch: master...Zentren:pycapnp:dynamic_schema_loading. We need to load binary schemas sent over the wire to interpret arbitrary structs able to appear in specific RPCs and had to make library changes to support this use-case. |
I want to be able to define the capnp definitions at run time.
It seems that
.load
won't accept anything but a real file due to this exception:I can work around this by saving a file to
/tmp
and loading it which seems silly.My question is other than only having the same
module
name once in the module tree is there any reason why this shouldn't be allowed? I am happy to open apr
which allows this work unless there is some reason why it should never be done.I imagine that allowing a network user to define an arbitrary capnp definition and having that initialized miiiight be a concern? But I guess the actual serde code where the
traversal_limit_in_words
is set is the main concern?The text was updated successfully, but these errors were encountered: