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

Move Message classes to class variables #631

Closed
tpazderka opened this issue Mar 19, 2019 · 3 comments
Closed

Move Message classes to class variables #631

tpazderka opened this issue Mar 19, 2019 · 3 comments

Comments

@tpazderka
Copy link
Collaborator

Many methods on Server and Client take Message class as a kwarg. This creates a need to override the method if we need to change the class (especially for Provider).

It might be beneficial to make them a class variable and allow to override them in __init__ perhaps by parsing a dictionary in order to not clutter the kwargs on the init method.

@tpazderka
Copy link
Collaborator Author

@schlenk Any objections or thoughts?

@schlenk
Copy link
Collaborator

schlenk commented Mar 22, 2019

Sounds halfway reasonable.

I see mostly pairs of request/response classes there, sometimes the response classes are hidden from sight one layer deeper. Most end up in parse_response() or construct_request() one way or the other, so this smells a bit like a pair of factories that could be extracted into their own classes.

A bit like have a factory class, register the message pairs (request/response) under some symbolic name and use those in client/server where needed. Then pass in the factory into the constructor somewhere.

@tpazderka
Copy link
Collaborator Author

The coupling of request/response cls makes sense. My idea was to have a dictionary with mapping of message type to class and pass that as a single kwarg to provider/server/client.

It might not necessarily be a dictionary, but an object with attributes. The value in the mapping can be a tuple with request/response as elements.

@tpazderka tpazderka self-assigned this Mar 23, 2019
tpazderka added a commit that referenced this issue Mar 30, 2019
tpazderka added a commit that referenced this issue Apr 2, 2019
andrewkrug pushed a commit to mozilla-iam/pyoidc that referenced this issue Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants