-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Explicitly call out dependency installation issues as unrecoverable #28107
Conversation
Codecov Report
@@ Coverage Diff @@
## master #28107 +/- ##
=======================================
Coverage 72.30% 72.31%
=======================================
Files 678 678
Lines 99771 99783 +12
=======================================
+ Hits 72140 72155 +15
+ Misses 26068 26065 -3
Partials 1563 1563
Flags with carried forward coverage won't be shown. Click here to find out more. see 9 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
PyDoc failure is as a result of #28104 |
Assigning reviewers. If you would like to opt out of this review, comment R: @damccorm for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Run PythonDocs PreCommit |
} | ||
// Install the Dataflow Python SDK and worker packages. | ||
// We install the extra requirements in case of using the beam sdk. These are ignored by pip | ||
// if the user is using an SDK that does not provide these. | ||
if err := installSdk(files, workDir, sdkSrcFile, acceptableWhlSpecs, false); err != nil { | ||
return fmt.Errorf("failed to install SDK: %v", err) | ||
return fmt.Errorf("failed to install SDK (this may be unrecoverable): %v", err) |
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.
Could we add more context here? Either a reason of why each of these errors are unrecoverable, or even better a link to a doc explaining it?
As a user, it wouldn't really be obvious to me what to do with this (many errors in a pipeline "may be unrecoverable" if you don't have more context about them)
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.
This will also be paired with logged output from PIP at error saying that a dependency failed to be installed once the buffered logger is implemented so that context will be there. A doc explaining what we mean by "unrecoverable" would be good though, something like a "Common Unrecoverable Errors" FAQ
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, something like https://cloud.google.com/dataflow/docs/guides/common-errors (but probably a Beam page)
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.
It can be super brief
Reminder, please take a look at this pr: @damccorm |
Adds a call-out to Python dependency installation failures that indicates that they may be unrecoverable. Also makes the error in the case that beam is not installed in the runtime environment Golang style compliant.
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.md
with 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.