feat: pause/resume event streams#442
Conversation
|
Warning Review limit reached
More reviews will be available in 33 minutes and 19 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds ChangesPause/Resume Event Stream Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. 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
🧹 Nitpick comments (1)
cmd/cloudx/eventstreams/flags.go (1)
95-95: ⚡ Quick winClarify the status flag default behavior in the help text.
The help text states
"Defaults to %q."(active), but the actual default is an empty string that gets normalized tonilinValidate(), allowing the server to apply its own default. The help text should reflect that omitting--statusdefers to the server default rather than implying the CLI defaults to"active".📝 Suggested help text revision
- c.Status = f.String("status", "", fmt.Sprintf("The status of the event stream. Supported values are %q and %q. Defaults to %q.", StatusActive, StatusPaused, StatusActive)) + c.Status = f.String("status", "", fmt.Sprintf("The status of the event stream. Supported values are %q and %q. If not provided, the server default applies.", StatusActive, StatusPaused))🤖 Prompt for 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. In `@cmd/cloudx/eventstreams/flags.go` at line 95, The help text for the `c.Status` flag definition using `f.String("status", ...)` currently states "Defaults to %q." with StatusActive as the value, which misleadingly suggests the CLI defaults to active. However, the actual behavior is that the default value is an empty string which normalizes to nil in the Validate() method, allowing the server to apply its own default. Update the help text format string to clarify that when `--status` is omitted, the server default is used, rather than implying the CLI defaults to active. Remove or revise the "Defaults to %q." portion to accurately reflect that the flag is optional and server-determined when not provided.
🤖 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 `@cmd/cloudx/eventstreams/status.go`:
- Around line 38-40: The error handling for ProjectID() is inconsistent across
the codebase: the current code wraps the error with cmdx.PrintOpenAPIError, but
other similar usages return the error directly without wrapping. Since
ProjectID() returns a simple error (ErrProjectNotSet), not an OpenAPI error,
remove the cmdx.PrintOpenAPIError wrapper and return the error from ProjectID()
directly to match the simpler and more consistent pattern used elsewhere.
---
Nitpick comments:
In `@cmd/cloudx/eventstreams/flags.go`:
- Line 95: The help text for the `c.Status` flag definition using
`f.String("status", ...)` currently states "Defaults to %q." with StatusActive
as the value, which misleadingly suggests the CLI defaults to active. However,
the actual behavior is that the default value is an empty string which
normalizes to nil in the Validate() method, allowing the server to apply its own
default. Update the help text format string to clarify that when `--status` is
omitted, the server default is used, rather than implying the CLI defaults to
active. Remove or revise the "Defaults to %q." portion to accurately reflect
that the flag is optional and server-determined when not provided.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fc519111-260b-4393-bbc0-d123644fe5ab
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (9)
cmd/cloudx/eventstreams/flags.gocmd/cloudx/eventstreams/flags_test.gocmd/cloudx/eventstreams/output.gocmd/cloudx/eventstreams/status.gocmd/cloudx/eventstreams/update.gocmd/cloudx/pause.gocmd/cloudx/resume.gocmd/root.gogo.mod
ProjectID() returns a plain ErrProjectNotSet, not an OpenAPI error, so wrapping it with cmdx.PrintOpenAPIError was a no-op and semantically misleading. Match the pattern used in create.go and update.go. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
The --status help text said it defaults to "active", which is accurate for create but misleading for update: omitting --status does NOT reset a paused stream to active. The behavior is already a partial update (an unset status is normalized to nil and omitted from the request), so only the wording needed fixing. Reword to state that an unset status keeps an existing stream's current status. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2 participants
Docs: ory/docs#2626
Summary by CodeRabbit
Summary by CodeRabbit
New Features
pauseandresumecommands to update event stream status.--statusflag support for creating and updating event streams (with validation for allowed values).STATUScolumn.Bug Fixes
--statusvalues are normalized to preserve default/server behavior.Tests
Chores