fix: wait for deployment readiness before reporting success#296
Conversation
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
|
Addressed review feedback: removed the trivial constant test in 9904798. Also updated the PR description to match the template. |
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
internal/cli/agent/deploy.go
Outdated
| ns = "(default)" | ||
| } | ||
|
|
||
| if noWait { |
There was a problem hiding this comment.
just noticed that this message here is duplicated with the one we return at the end of the function.
Perhaps reverse the check for noWait. Also, thinking about this now, might be better if the flag is called wait and it's set to true by default.
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
1 similar comment
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
|
Both issues addressed in 49b6170:
|
|
@optimus-fulcria there are some test failures |
|
All feedback addressed in 49b6170:
|
When deploying to non-local providers (e.g. kubernetes), the CLI now polls the deployment status until it reaches a terminal state (deployed, failed, or cancelled) instead of returning immediately. Add --no-wait flag to skip readiness polling and return immediately. Fixes agentregistry-dev#230 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address review feedback: - Rename --no-wait flag to --wait with default true per reviewer suggestion - Remove duplicated success message by using single print after optional wait - Remove trivial TestWaitConstants that just asserts constant values Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update deploy_test.go to reference the renamed --wait flag (default true) instead of --no-wait. Also rename deployNoWait to deployWait in MCP deploy.go for consistency. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
|
Thanks for the review! Both comments have been addressed:
Ready for re-review when you get a chance. |
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
1 similar comment
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
|
Addressed all review feedback and rebased on latest main:
|
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
1 similar comment
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
|
Thanks for the review @peterj! Both items addressed:
The two messages in |
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
|
Addressed remaining feedback in 1547e6e:
No other files changed. |
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
|
All review feedback addressed:
All CI checks pass. Ready for re-review. |
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
1 similar comment
|
You already have 3 pull requests open. Please consider working on getting the existing ones merged before opening new ones. Thanks! |
9ea004e to
44b01a3
Compare
Description
--waitflag (defaulttrue) to bothagent deployandmcp deploycommandsWaitForDeploymentReadyhelper ininternal/cli/common/wait.gothat polls deployment status until terminal state or 5-minute timeoutdeploy_test.gowith flag registration testsTestWaitConstantstest per review feedbackChange Type
/kind feature
Changelog
Additional Notes
--waitflag defaults totrueper review feedback (reversed from original--no-waitapproach).