-
Notifications
You must be signed in to change notification settings - Fork 127
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
Fix visibility of pipeline metrics along with reserved/duplicate node name detection #648
Conversation
Ramp metrics should be visible though Signed-off-by: Anup Dhamala <anupdhml+git@gmail.com>
Signed-off-by: Anup Dhamala <anupdhml+git@gmail.com>
Signed-off-by: Anup Dhamala <anupdhml+git@gmail.com>
I think we should definitely throw an error if a user names a node |
Opened #650 to generalize error reporting when a node already exists with the given name. |
… name Fixes #650 Signed-off-by: Anup Dhamala <anupdhml+git@gmail.com>
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.
LGTM in general, I especially like the nice errors.
I just had a tiny question.
Pull request
Fix generation of pipeline metrics (as reported in #647) and improve handing of cases when a pipeline node is created with the same name as an existing one.
Description
Ensured consistent id for metrics node when it's added to the pipeline graph (as
"metrics"
). Added tests to verify pipeline and ramp metrics as well.Also throw error and terminate pipeline creation, if a user happens to use id
metrics
for one of their trickle operators (earlier, this was allowed and we would stop getting metrics too) -- this is solved as part of general handling described in #650.For later, we should rename the
events
measurement to reflect that it's pipeline specific (or merge metrics inramp_events
underevents
.)Related
Checklist