fix: show containerapp endpoints in deployment summary#2130
fix: show containerapp endpoints in deployment summary#2130jordanstephens wants to merge 2 commits into
Conversation
After the CD task finishes, re-fetch service infos from the provider so the deployment summary displays the actual azurecontainerapps.io URLs written by the CD task rather than the pre-deploy defang.app placeholders. Azure delegate domain support is not yet implemented, so this is a stopgap until it is. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAfter log tailing completes, ChangesCompose Service Refresh
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=warning msg="The linter 'gomodguard' is deprecated (since v2.12.0) due to: new major version. Replaced by gomodguard_v2." Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cmd/cli/command/compose.go`:
- Around line 192-194: The code currently assumes resp from
session.Provider.GetServices(ctx, &defangv1.GetServicesRequest{Project:
project.Name}) is non-nil and reads resp.Services, which can panic if
GetServices returns (nil, nil); modify the block around
session.Provider.GetServices so you first check that err == nil AND resp != nil
before accessing resp.Services, only assign updatedServices = resp.Services when
resp != nil and len(resp.Services) > 0, and otherwise leave
deploy.Services/updatedServices unchanged (optionally log a debug message);
reference the call site session.Provider.GetServices and the variables resp,
updatedServices and deploy.Services when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 238995fc-12be-4369-9876-331704b4a520
📒 Files selected for processing (1)
src/cmd/cli/command/compose.go
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/cmd/cli/command/compose.go`:
- Around line 192-195: The log currently uses term.Debugf when calling
session.Provider.GetServices and incorrectly says "failed" even when err==nil
but resp is nil or resp.Services is empty; change this to two distinct slog
debug calls using structured fields: on error (err != nil) call
slog.DebugContext(ctx, "GetServices error after deployment",
slog.String("project", project.Name), slog.Any("err", err")), and when resp==nil
|| len(resp.Services)==0 call slog.DebugContext(ctx, "no services returned after
deployment", slog.String("project", project.Name), slog.Any("resp", resp));
remove the term.Debugf invocation and update any imports to use log/slog.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ecff8c85-18b5-4d4d-9e28-cc3805fd7fc9
📒 Files selected for processing (1)
src/cmd/cli/command/compose.go
| resp, err := session.Provider.GetServices(ctx, &defangv1.GetServicesRequest{Project: project.Name}) | ||
| if err != nil || resp == nil || len(resp.Services) == 0 { | ||
| term.Debugf("GetServices failed after deployment: %v", err) | ||
| } else { |
There was a problem hiding this comment.
wonder if we should check the etag in the fetched response to ensure we're not printing old state
|
See also DefangLabs/pulumi-defang#181 and #2133 |
|
Closing in favour of #2136 |
2 participants
Description
After the CD task finishes, re-fetch service infos from the provider so the deployment summary displays the actual azurecontainerapps.io URLs written by the CD task rather than the pre-deploy defang.app placeholders. Azure delegate domain support is not yet implemented, so this is a stopgap until it is.
Linked Issues
Checklist
Summary by CodeRabbit
compose upfinishes, service info is now refreshed from the deployment target so printed service states and endpoint URLs reflect actual deployed values (Azure endpoints now show real URLs instead of placeholders).