Add browser-based auth and improved error handling for model loading#798
Add browser-based auth and improved error handling for model loading#798brendan-priorlabs wants to merge 8 commits intomainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
Code Review
This pull request introduces a browser-based authentication flow for license acceptance, which is a significant feature addition. The implementation is robust, covering various user scenarios and including a fallback to manual token entry. The changes are well-tested with a new suite of unit tests for the authentication module. I've added a couple of suggestions: one to correct an environment variable name in the user testing documentation, and another to improve debuggability by logging a swallowed exception in the new authentication server thread. Overall, this is a high-quality contribution.
USER_TESTING.md
Outdated
|
|
||
| ```bash | ||
| # simulate gating removal by setting a token | ||
| export HF_TOKEN=... |
| except Exception: | ||
| break |
There was a problem hiding this comment.
The except Exception block in the server loop swallows exceptions without logging. This can make it difficult to debug issues with the callback server. It's good practice to log the exception before breaking the loop.
| except Exception: | |
| break | |
| except Exception: | |
| logger.debug("Callback server loop error", exc_info=True) | |
| break |
No description provided.