OAuth for CLI#12
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
adivate2021
force-pushed
the
aaryan/oauth-cli
branch
from
0513c92 to
a934416
Compare
| ) | ||
| authorize_url = f"{auth_url.rstrip('/')}/oauth/authorize?{authorize_query}" | ||
|
|
||
| thread = Thread(target=server.serve_forever, daemon=True) |
There was a problem hiding this comment.
This needs to be killed right? This should not be a daemon either. The resource cleanup on this is imporant.
There was a problem hiding this comment.
I see the server is killed but what happens to the thread
There was a problem hiding this comment.
should be fixed I think (not daemonized and proper cleanup)
| api_key: str | ||
|
|
||
|
|
||
| def _config_dir() -> Path: |
There was a problem hiding this comment.
i feel like there are so many 1 line functions that just call other functions that are invoked only one time like this.
There was a problem hiding this comment.
removed this func
| return AccessToken(self._api_key) | ||
|
|
||
| def refresh(self, *, force: bool = False) -> bool: | ||
| return False |
There was a problem hiding this comment.
shouldnt refresh be true?
There was a problem hiding this comment.
With API key auth there is nothing to refresh so its false
There was a problem hiding this comment.
what does the return value of refresh mean? Doesnt saying false indicates it failed to refresh?
|
|
||
|
|
||
| class Credential(Protocol): | ||
| def get_token(self) -> AccessToken: ... |
There was a problem hiding this comment.
why is both get_token and apply needed?
I think shouldnt create an object each time calling get_token
There was a problem hiding this comment.
Removed get_token
| auth_url: str, | ||
| access_token: str, | ||
| refresh_token: str, | ||
| expires_at: int | None = None, |
There was a problem hiding this comment.
what is this token_updater used for now?
| "state": query.get("state", [""])[0], | ||
| "error": query.get("error", [""])[0], | ||
| } | ||
| self.server.result_queue.put(result) # type: ignore[attr-defined] |
There was a problem hiding this comment.
we dont do any other validation here?
There was a problem hiding this comment.
added some more validation
2 participants
Uh oh!
There was an error while loading. Please reload this page.