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
[BEAM-10545] Added the inspector of PCollections and pipelines #12626
Conversation
c714895
to
a52a2fa
Compare
R: @prodonjs PTAL! |
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.
Sorry for the delay. I started on Friday and forgot I didn't submit. Just a few small nits but feel free to submit as-is if you like.
} | ||
|
||
render(): React.ReactNode { | ||
const pcollListItems = Object.entries(this.props.pcolls).map( |
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.
Just a nit, but you could probably move this and the onClick function out to private methods to make the logical separation a bit more readable. Or, since there is no state to this component, you could write it as a pure functional component that accepts props and returns the React node.
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.
Thanks! I've moved the logic out into private functions.
} | ||
|
||
renderOptions(): React.ReactNode { | ||
if (this.props.model.inspectableType === 'pcollection') { |
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.
nit you can reduce the nesting level for the pcollection
case if you put put the inverse condition first and short-circuit the return for the empty span.
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.
Inverted the condition and the return statements.
Codecov Report
@@ Coverage Diff @@
## master #12626 +/- ##
==========================================
- Coverage 40.22% 40.22% -0.01%
==========================================
Files 454 454
Lines 53669 53670 +1
==========================================
Hits 21587 21587
- Misses 32082 32083 +1
Continue to review full report at Codecov.
|
R: @pabloem PTAL, thx! Do we have a way to re-trigger |
Run Python PreCommit |
Run Python2_PVR_Flink PreCommit |
1. Added the InteractiveInspector component and its sub components. The hierarchy can be found in the code documentation. 2. The component allows listing all user defined pipelines and PCollections in the kernel of the current notebook session, displaying pipeline graphs, and visualizing PCollection data.
18b1dc5
to
dc3144d
Compare
Rebased to head to resolve failures in Python tests. |
Run Typescript PreCommit |
hierarchy can be found in the code documentation.
PCollections in the kernel of the current notebook session,
displaying pipeline graphs, and visualizing PCollection data.
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
R: @username
).[BEAM-XXX] Fixes bug in ApproximateQuantiles
, where you replaceBEAM-XXX
with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.CHANGES.md
with noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
Post-Commit Tests Status (on master branch)
Pre-Commit Tests Status (on master branch)
See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI.