-
Notifications
You must be signed in to change notification settings - Fork 486
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
Add flow API support #2094
Add flow API support #2094
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.
I gave this a first pass, but I need more time to carefully review json_walker.go. It feels like there's more opportunities for it to reuse logic from river/internal/value and I want to take the time to find them so we don't have to spend too much time maintaining the json conversion.
Rewrote / changed pretty much the whole thing, its more code but I think much clearer on what is happening. |
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.
Nice! This is looking much better.
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.
Getting much closer! I think this is my final round of comments.
(The CI is failing because of changes to the Makefile; you'll need to rebase on top of latest main) |
… isValid to hasValue.
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 after final comments. Nice work!
Co-authored-by: Robert Fratto <robertfratto@gmail.com>
Co-authored-by: Robert Fratto <robertfratto@gmail.com>
Co-authored-by: Robert Fratto <robertfratto@gmail.com>
Co-authored-by: Robert Fratto <robertfratto@gmail.com>
PR Description
This adds the initial support for the flow API, necessary for the UI portion.
Fixes #2007.