Added API Token object and logging in as a different user#2
Added API Token object and logging in as a different user#2
Conversation
…ith different accounts
There was a problem hiding this comment.
Pull request overview
Adds support for interacting with Hashtopolis APIv2 using explicit credentials (for “log in as a different user”) and introduces an ApiToken API object.
Changes:
- Added
HashtopolisConfig.with_credentials()to construct configs without reading a config file. - Threaded optional
authcredentials throughQuerySet→ connectorfilter()calls. - Added
ApiTokenmodel and exported it from the package.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
hashtopolis/hashtopolis.py |
Adds credential-based config construction, optional auth handling for filtering, QuerySet.authenticate(), and the new ApiToken model. |
hashtopolis/__init__.py |
Exports ApiToken and adds Helper to the base import list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def authenticate(self, auth=None): | ||
| if auth is not None: | ||
| logger.info("Start authentication with provided credentials") | ||
| auth_uri = self._api_endpoint + '/auth/token' | ||
| auth = (self.config.username, self.config.password) | ||
| r = requests.post(auth_uri, auth=auth) | ||
| self.validate_status_code(r, [201], "Authentication failed") | ||
|
|
||
| r_json = self.resp_to_json(r) | ||
| HashtopolisConnector.token[self._api_endpoint] = r_json['token'] | ||
| HashtopolisConnector.token_expires[self._api_endpoint] = r_json['token'] | ||
|
|
||
| self._token = HashtopolisConnector.token[self._api_endpoint] | ||
| self._token_expires = HashtopolisConnector.token_expires[self._api_endpoint] | ||
| self._token = r_json['token'] | ||
| self._token_expires = r_json['token'] |
There was a problem hiding this comment.
HashtopolisConnector.authenticate(auth=...) bypasses the class-level token cache and will POST to /auth/token on every call when auth is provided. This can cause unnecessary authentication traffic and makes behavior inconsistent with the no-arg path. Consider caching tokens per (api_endpoint, username) (or per full auth identity) or alternatively accepting a bearer token directly so repeated requests don't re-authenticate.
| def filter(self, include, ordering, filter, start_offset, auth=None): | ||
| self.authenticate(auth=auth) | ||
| headers = self._headers |
There was a problem hiding this comment.
Auth support is only threaded through HashtopolisConnector.filter(..., auth=...). Other connector methods invoked by the ORM-style API (get_single_page, get_one, create, delete, etc.) still call authenticate() without a way to supply alternate credentials, so "log in as a different user" will be incomplete depending on which QuerySet/Manager method is used. Consider consistently plumbing an auth (or token) parameter through connector methods, or storing the per-request auth context on the connector/QuerySet and using it across all request types.
| def authenticate(self, auth): | ||
| self.auth = auth | ||
| return self | ||
|
|
There was a problem hiding this comment.
QuerySet.authenticate() doesn't actually perform authentication; it only sets self.auth for later use. This name can be misleading for callers (it reads like a network operation). Consider renaming it to something that reflects intent (e.g., with_auth/using_credentials) and/or documenting the expected auth type (requests auth tuple).
| def authenticate(self, auth): | |
| self.auth = auth | |
| return self | |
| def with_auth(self, auth): | |
| """Set authentication credentials for subsequent requests. | |
| This only stores the value on the QuerySet so it can be passed to | |
| later HTTP requests. It does not perform authentication immediately. | |
| Args: | |
| auth: Authentication object understood by requests, typically a | |
| ``(username, password)`` tuple. | |
| """ | |
| self.auth = auth | |
| return self | |
| def authenticate(self, auth): | |
| """Backward-compatible alias for :meth:`with_auth`.""" | |
| return self.with_auth(auth) |
| ModelBase, | ||
| Model | ||
| Model, | ||
| Helper | ||
| ) |
There was a problem hiding this comment.
Helper is now imported in the "base stuff" import list, but it is also imported again later in the "action utilities" import list. Consider removing one of these imports to avoid redundancy and potential confusion during maintenance.
There was a problem hiding this comment.
As it's mentioned by Copilot, the helper class is already imported below.
| ModelBase, | ||
| Model | ||
| Model, | ||
| Helper | ||
| ) |
There was a problem hiding this comment.
As it's mentioned by Copilot, the helper class is already imported below.
No description provided.