Revert "Revert "truss push: Make published deployments the default""#2202
Revert "Revert "truss push: Make published deployments the default""#2202
truss push: Make published deployments the default""#2202Conversation
|
Issue reopened: BT-15696 Making |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 61cc13539a
ℹ️ 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".
| if tail: | ||
| raise click.UsageError("Cannot use --watch with --tail.") | ||
| # Development deployment for watch mode | ||
| publish = False |
There was a problem hiding this comment.
When --watch is used, the CLI unconditionally forces publish = False and does not validate that the user also passed --publish. This means truss push --watch --publish silently creates a development deployment even though the caller explicitly requested published behavior. Because --watch is new and --publish is still accepted, this is a user-facing regression that can lead to deploying the wrong model type; it should raise a UsageError (similar to the promote/environment/tail checks) or explicitly reject --publish with --watch.
Useful? React with 👍 / 👎.
daanielww
force-pushed
the
revert-2201-revert-2178-danielwang/bt-15696-making-publish-the-default-in-truss
branch
from
e9dc21e to
55e0a3e
Compare
| help=( | ||
| "Deploy the chain as a published deployment to the specified environment." | ||
| "If specified, --publish is implied and the supplied value of --promote will be ignored." | ||
| "If specified, publish is implied and the supplied value of --promote will be ignored." |
There was a problem hiding this comment.
super nit: given that we say published above, I bet we can simplify this to If specified, the supplied value ...
| "Cannot use --watch with --environment. Watch mode runs a development deployment." | ||
| ) | ||
| if tail: | ||
| raise click.UsageError("Cannot use --watch with --tail.") |
There was a problem hiding this comment.
Makes sense for now, but I do think this is going to be an ask soon. Basically stream logs, but still have watch indicate what it's doing when needed
I'm not sure the best DX for this yet
There was a problem hiding this comment.
yeah agreed, but I think we can handle that separately
There was a problem hiding this comment.
yes agreed! not for this PR
|
|
||
| # Handle --watch flag: deploys as development and then watches | ||
| if watch_after_push: | ||
| if promote: |
There was a problem hiding this comment.
Should we error if you provided both --publish and --watch explicitly?
Otherwise I think people might miss the warning above and be slightly surprised
daanielww
force-pushed
the
revert-2201-revert-2178-danielwang/bt-15696-making-publish-the-default-in-truss
branch
from
510307d to
29ce828
Compare
2 participants
Reverts #2201