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

Cyclic imports might be better avoided? #24

Open
rogpeppe opened this issue Jun 2, 2017 · 2 comments
Open

Cyclic imports might be better avoided? #24

rogpeppe opened this issue Jun 2, 2017 · 2 comments
Milestone

Comments

@rogpeppe
Copy link
Contributor

rogpeppe commented Jun 2, 2017

There's a cyclic dependency between pymacaroons and serializers.
ISTM like this might be better avoided. Here are a few possible
ways of doing so.

  • include serializers in the base pymacaroons module
  • make serializer a mandatory argument to the serialize method
  • define a canonical dict-based format and make serializers
    serialize to and from that format
  • pass Macaroon and Caveat constructors as arguments to the
    deserialize method.
@ecordell
Copy link
Owner

ecordell commented Jun 2, 2017

Good points; I considered it a usability tradeoff (for simple cases you get your de/serialization for free).

I've been thinking of doing something more go-like and just throwing Marshal/Unmarshal and MarshalJSON/UnmarshalJSON on the macaroon object as the standard formts. If you want additional serializations (like the protobuf test I have in a branch here) you can just extend the Macaroon object with additional methods.

@rogpeppe
Copy link
Contributor Author

rogpeppe commented Jun 2, 2017

That seems good to me as long as it's documented how to get access to the data inside the macaroon so it can be serialised. Tbh I'm not sure that I'd extend the object - an external function could work ok, but I'm probably betraying my Go tendencies there.

It's definitely nice to have some serialisation available without having an extra import.

@ecordell ecordell added this to the 1.0.0 milestone Aug 25, 2017
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

No branches or pull requests

2 participants