Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork258
Removing AuthenticatedClient in favor of Client.with_authorization_header and Generics#1287
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
-
Hi, I've been exploring this package and am very excited about the output it generates. Especially the extensibility due to the jinja2 templating is great! Thank you for maintaining it. 🙇 After generating a client I noticed that there's a lot of duplication between AuthenticatedClient and Client. From what I can tell the usage of AuthenticatedClient is:
Setting authentication headersCurrently we solve for this usecase: client=Client()# will make unauthenticated requestsauth_client=AuthenticatedClient(token='my-token')# will make authenticated requests However, when the token needs to change (f.e. b/c different users use different tokens) then we'd have to completely change the client. Leading to the auth_client1=AuthenticatedClient(token='user1-token')# will make authenticated requests for user1auth_client2=AuthenticatedClient(token='user2-token')# will make authenticated requests for user2 It might be nicer to instead have methods on client=Client()client.with_authorization_token(token='user1-token')client.request()# will make auth requests for user1client.with_authorization_token(token='user2-token') The added benefit here is that we can solve for#823 by adding Type hintingTo make clear when an endpoint requires authentication headers set we can add a phantom type parameter. frombase64importb64encodefromtypingimportDict,Generic,TypeVar,castfromattrsimportdefine,evolve,fieldclassUnauthenticated: ...classAuthenticated: ..._State=TypeVar("_State")@defineclass_Client(Generic[_State]):_headers:Dict[str,str]=field(factory=dict,kw_only=True)defwith_headers(self,headers:Dict[str,str])->"Client":returnevolve(self,_headers={**self._headers,**headers})defwith_authorization_token(self:"Client",token:str,prefix:str="Bearer", )->"AuthClient":returncast("AuthClient",self.with_headers({"Authorization":f"{prefix}{token}"}))defwith_authorization_basic(self:"Client",username:str,password:str,prefix:str="Basic", )->"AuthClient":encoded_credentials=b64encode(f"{username}:{password}".encode()).decode()returncast("AuthClient",self.with_headers({"Authorization":f"{prefix}{encoded_credentials}"}))defremove_headers(self:"AuthClient")->"Client":returnevolve(self,_headers={})Client=_Client[Unauthenticated]AuthClient=_Client[Authenticated]defmy_func(client:AuthClient)->None:# only accepts authenticated clientsprint("got headers:",client._headers)# --- usage demos ---c0=Client()# Unauthenticatedmy_func(c0)# static type errorc1=c0.with_authorization_token("my-token")# Authenticatedmy_func(c1)# okc2=c1.remove_headers()# back to Unauthenticatedmy_func(c2)# static type errorc1=c0.with_authorization_basic("my-username","my-password")# Authenticatedmy_func(c1)# ok Let me know what you think. I'm happy to suggest the PR 🙇 and thanks again! |
BetaWas this translation helpful?Give feedback.
All reactions
Replies: 2 comments
-
Oh, and this would be a breaking change as |
BetaWas this translation helpful?Give feedback.
All reactions
-
BetaWas this translation helpful?Give feedback.