[#29803] Fix nil pointer access in logRuntimeDependencies#29804
[#29803] Fix nil pointer access in logRuntimeDependencies#29804jrmccluskey merged 1 commit intoapache:masterfrom
Conversation
|
R: @damccorm @riteshghorse |
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #29804 +/- ##
==========================================
- Coverage 38.23% 38.23% -0.01%
==========================================
Files 696 696
Lines 101879 101879
==========================================
- Hits 38956 38949 -7
- Misses 61307 61312 +5
- Partials 1616 1618 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| @@ -409,7 +409,7 @@ func installSetupPackages(ctx context.Context, logger *tools.Logger, files []str | |||
| if err := pipInstallPackage(ctx, logger, files, workDir, workflowFile, false, true, nil); err != nil { | |||
There was a problem hiding this comment.
Do we want to do the same for installExtraPackages and pipInstallPackages?
There was a problem hiding this comment.
We can clean this up after the fact to a cleaner pattern, IIRC an issue similar to the one this fixes popped up when I was implementing the buffered logger originally which led to needing to handle the nil logger in the setup-only mode case.
There was a problem hiding this comment.
Thanks, @jrmccluskey ! Could you please file an issue to follow up? thank you!
|
It is an odd pattern that at present:
I'd expect us to pass around the buffered logger type from the top, instead of arbitrarily trying to re-construct it. Stuff to clean up when it's not a release blocker. |
Corrects a nil pointer usage when Python containers are built through
processArtifactsInSetupOnlyMode()Fixes #29803
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.