-
Notifications
You must be signed in to change notification settings - Fork 85
feat(shell-api): add ability to drill into stream processor attributes STREAMS-1136 #2578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enhances the Stream Processor API to allow users to drill into individual stream processor attributes. Previously, stream processors only exposed their name; now they expose all attributes (id, pipeline, state, tier, errorMsg, lastModified, lastStateChange) as individual properties, enabling easier variable assignment and debugging in the shell.
Key changes:
- Modified
StreamProcessorconstructor to accept a data object instead of just a name string - Updated
getProcessor()to pass complete processor data instead of only the name - Changed
[asPrintable]()to return a complete data object rather than a simple string
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/shell-api/src/streams.ts | Updated getProcessor() signature to accept StreamProcessorData object; modified calls to pass complete processor data including pipeline and options |
| packages/shell-api/src/streams.spec.ts | Added comprehensive test coverage for listStreamProcessors() including filter handling, error cases, and property access verification |
| packages/shell-api/src/stream-processor.ts | Refactored constructor to accept data object and expose all processor attributes as public properties; updated [asPrintable]() to return complete processor data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| this.tier = data.tier; | ||
| this.errorMsg = data.errorMsg; | ||
| this.lastModified = data.lastModified; | ||
| this.lastStateChange = data.lastStateChange; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having this list means we'll need to keep it up to date with the source of truth, which I guess is the server – would there be a major downside to assining all properties of data to this object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah... I kind of hated it. I tried to get this working at one point but was having issues. I tried to stuff everything under a this.data attribute with the type of StreamProcessorData (Document & { name: string }) but I couldn't figure out how to dynamically expose top level getters for all of the attributes (so that users don't have to go through .data. I guess I could just iterate through the keys in the document and assign those values to this 🤔 will get that going :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I could just iterate through the keys in the document and assign those values to
this🤔 will get that going :)
Yeah, that's more or less what I would have done – there's obviously a small potential for conflicts, but I think that's okay here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me know what you think. Seems fairly straightforward to me. But easy to say as the author :)
|
The evergreen failures appear to be unrelated |
14899c5 to
13ff008
Compare
13ff008 to
a556a8c
Compare
|
@addaleax Should be good to merge now. The evergreen failures do not appear to be related to my PR |
addaleax
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you!
Jira: https://jira.mongodb.org/browse/STREAMS-1136
Users ask for the ability to assign stream processor pipelines to variables so they can more easily reuse and debug them in the shell. While I was in there I figured I would expose all individual attributes on the stream processor.