feat(rotation): manual account selection #93
Conversation
|
I'm starting my review of the manual account selection feature. Diving into the new rotation logic and TUI changes now. |
![mirrobot-agent[bot]](https://avatars.githubusercontent.com/in/2140342?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overall Assessment
This is a solid implementation of manual account selection. The feature is well-integrated into the TUI and respects existing usage/cooldown constraints. The addition of the /v1/force-credential endpoint is a clean way to handle this override.
Architectural Feedback
The choice to handle the force logic at the beginning of acquire_key is correct as it allows you to reuse all the existing cooldown and concurrency checks. Falling back to normal rotation is a pragmatic choice to maintain proxy uptime, though it might surprise a user who expected a "hard lock".
Key Suggestions
- Path Handling: Use
pathlibinmain.pyfor cleaner display name extraction. - Matching Logic: Be aware of potential ambiguity when matching by basename if multiple files share a name.
- TUI Efficiency: Consider persisting the
httpx.ClientinQuotaViewer.
Questions for the Author
- Was there a specific scenario where you found basename matching necessary over using the full path?
- Should the "forced" state be persisted across proxy restarts, or is it intentionally transient?
This review was generated by an AI assistant.
| if credential: | ||
| # Extract a friendly display name from the credential path | ||
| if "/" in credential or "\\" in credential: | ||
| display_name = credential.split("/")[-1].split("\\")[-1] |
There was a problem hiding this comment.
This path splitting logic is a bit brittle. Consider using Path(credential).name from pathlib for a more robust cross-platform solution.
| display_name = credential.split("/")[-1].split("\\")[-1] | |
| display_name = Path(credential).name |
| # Try matching by filename (basename) | ||
| for key in available_keys: | ||
| if key.endswith(forced_cred) or Path(key).name == forced_cred: | ||
| matched_cred = key | ||
| break |
There was a problem hiding this comment.
Matching by basename (filename) could be ambiguous if multiple credentials have the same filename in different directories. While probably rare, it might be safer to prioritize exact matches (which you already do) and perhaps log a warning if multiple basename matches are found.
| with httpx.Client(timeout=10.0) as http_client: | ||
| response = http_client.post( | ||
| url, | ||
| headers=self._get_headers(), | ||
| json=payload | ||
| ) |
There was a problem hiding this comment.
Creating a new httpx.Client for every request in a loop is slightly inefficient. For a TUI it likely doesn't matter, but using a single client instance for the QuotaViewer session might be cleaner for the long term.
| lib_logger.warning( | ||
| f"Forced credential {mask_credential(matched_cred)} is at max concurrency " | ||
| f"({current_count}/{max_concurrent}), falling back to normal rotation" | ||
| ) | ||
| else: | ||
| lib_logger.warning( | ||
| f"Forced credential {mask_credential(matched_cred)} is on cooldown, " | ||
| f"falling back to normal rotation" | ||
| ) | ||
| else: | ||
| lib_logger.warning( | ||
| f"Forced credential {mask_credential(forced_cred)} not found in available credentials, " | ||
| f"falling back to normal rotation" | ||
| ) |
There was a problem hiding this comment.
The fallback to normal rotation when a forced credential is on cooldown or at max concurrency is a safe choice for availability. However, if a user forces a credential, they might prefer a clear error if it can't be used. Since the TUI mentions "(if available)", this behavior is at least documented, but it's worth considering if a stricter 'force' is needed.
1 participant
probably slop, but tested with antigrav accounts and worked
Important
Adds manual account selection feature to proxy, allowing users to force specific credentials for requests via a new endpoint and TUI updates.
/v1/force-credentialendpoint inmain.pyto force a specific credential for all requests, overriding normal rotation logic.show_provider_detail_screen()inquota_viewer.pyto allow forcing and clearing credentials via numeric selection.set_forced_credential()andget_forced_credential()inusage_manager.pyto manage forced credential state.acquire_key()inusage_manager.pyto check and use forced credentials if set.This description was created by
for fa2e987. You can customize this summary. It will automatically update as commits are pushed.