Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e97eeaa877
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Wake the model in case it's scaled to zero | ||
| wake_url = f"{model_hostname}/development/wake" | ||
| api_key = remote_provider._auth_service.authenticate().value |
There was a problem hiding this comment.
Normalize model hostname before building keepalive URLs
If resolved_model["hostname"] is a bare host (e.g. the repo’s tests and API mocks use values like host.baseten.co/hostname without a scheme), then f"{model_hostname}/development/wake" (and the later keepalive URL) becomes an invalid URL and requests raises MissingSchema. That means --no-sleep will never successfully wake/keepalive and will eventually exit after 20 failures. Consider normalizing the hostname (e.g. prefix https:// when missing) or reusing a URL builder that guarantees a full scheme.
Useful? React with 👍 / 👎.
| sys.exit(1) | ||
|
|
||
| # Wake the model in case it's scaled to zero | ||
| wake_url = f"{model_hostname}/development/wake" |
There was a problem hiding this comment.
nit: Should we log here too? Something along the lines of 'Model currently inactive, waking.'
| "[bold green]Waiting for development model to be ready..." | ||
| ) as status: | ||
| while True: | ||
| time.sleep(1) |
There was a problem hiding this comment.
nit: Thoughts about making this slightly less aggressive? Agreed it's a balance between appearance of quickness to user and risk to our servers
There was a problem hiding this comment.
current --wait functionality also uses a 1s sleep for fetching for status but yes I'm ok with increasing this value
81f8417 to
ecef162
Compare
🚀 What
Add new
truss watch --no-sleepwhich starts up a keep-alive thread in the background that pings the development model's readiness endpoint to prevent it from scaling down💻 How
Local E2E testing + unit tests
🔬 Testing