Skip to content

Comments

Add states and events url to Deployment protobuf#1761

Merged
edwardrf merged 1 commit intomainfrom
edw/deployment-add-states-and-events-url
Jan 8, 2026
Merged

Add states and events url to Deployment protobuf#1761
edwardrf merged 1 commit intomainfrom
edw/deployment-add-states-and-events-url

Conversation

@edwardrf
Copy link
Contributor

@edwardrf edwardrf commented Jan 8, 2026

Description

To allow deployment to capture states and events.

Linked Issues

https://github.com/DefangLabs/defang-mvp/issues/2562

Checklist

  • I have performed a self-review of my code
  • I have added appropriate tests
  • I have updated the Defang CLI docs and/or README to reflect my changes, if necessary

Summary by CodeRabbit

  • New Features
    • Deployment objects now include URLs for accessing state and event information.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 8, 2026

📝 Walkthrough

Walkthrough

Added two new string fields (states_url and events_url) to the Deployment message in the protobuf schema. These fields extend the Deployment message definition without modifying or removing existing fields.

Changes

Cohort / File(s) Change Summary
Protobuf Schema Extension
src/protos/io/defang/v1/fabric.proto
Added two new string fields to Deployment message: states_url (field 14) and events_url (field 15) after the status field

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related PRs

Suggested reviewers

  • jordanstephens
  • lionello

Poem

🐰 Two URLs hop into the Deployment,
States and events now have a settlement,
Field fourteen and fifteen align,
The schema grows, oh how it shines!
Proto-magic, perfectly benign! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding two URL fields (states_url and events_url) to the Deployment protobuf message.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In @src/protos/io/defang/v1/fabric.proto:
- Around line 472-473: Deployment message fields states_url and events_url are
declared but never set or used; update the server-side ListDeployments handler
(where Deployment objects are built) to populate Deployment.states_url and
Deployment.events_url with the correct endpoints/URLs (e.g., per-deployment
state and event stream paths), and update any client code/tests that construct
or consume Deployment to use these fields instead of empty values;
alternatively, if these URLs are not needed, remove states_url and events_url
from the Deployment proto and all server/client/test references to avoid dead
contract surface. Ensure the change is applied consistently: set the fields in
the ListDeployments response builder (and any Create/Update responses), adjust
clients to read and use Deployment.states_url/Deployment.events_url, and
update/augment unit tests to assert the presence and correctness of these fields
(or remove proto fields and tests that reference them).
🧹 Nitpick comments (1)
src/protos/io/defang/v1/fabric.proto (1)

472-473: Consider adding inline documentation for the new URL fields.

Adding comments would clarify the purpose and expected format of these URLs (e.g., what do the states and events URLs point to, are they absolute URLs, do they require authentication).

📝 Example documentation
   DeploymentStatus status = 13;
-  string states_url = 14;
-  string events_url = 15;
+  string states_url = 14; // URL to access deployment state information
+  string events_url = 15; // URL to access deployment event logs
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dc57109 and 6a5fb2e.

⛔ Files ignored due to path filters (1)
  • src/protos/io/defang/v1/fabric.pb.go is excluded by !**/*.pb.go
📒 Files selected for processing (1)
  • src/protos/io/defang/v1/fabric.proto
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Analyze (go)
  • GitHub Check: go-test
🔇 Additional comments (1)
src/protos/io/defang/v1/fabric.proto (1)

472-473: LGTM! Field additions are syntactically correct and backward compatible.

The new URL fields follow proper protobuf conventions with sequential field numbering and snake_case naming. The additions are backward compatible since proto3 fields are optional by default.

@edwardrf edwardrf merged commit 02dd22c into main Jan 8, 2026
14 checks passed
@edwardrf edwardrf deleted the edw/deployment-add-states-and-events-url branch January 8, 2026 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants