Skip to content
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

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions sdks/python/container/boot.go
Original file line number Diff line number Diff line change
Expand Up @@ -380,26 +380,26 @@ func installSetupPackages(files []string, workDir string, requirementsFiles []st
pkgName := "apache-beam"
isSdkInstalled := isPackageInstalled(pkgName)
if !isSdkInstalled {
return fmt.Errorf("Apache Beam is not installed in the runtime environment. If you use a custom container image, you must install apache-beam package in the custom image using same version of Beam as in the pipeline submission environment. For more information, see: the https://beam.apache.org/documentation/runtime/environments/.")
return fmt.Errorf("apache-beam is not installed in the runtime environment. If you use a custom container image, you must install apache-beam package in the custom image using same version of Beam as in the pipeline submission environment. For more information, see: the https://beam.apache.org/documentation/runtime/environments/")
}
// 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)
Copy link
Contributor

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)

Copy link
Contributor Author

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

Copy link
Contributor

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)

Copy link
Contributor

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

}
// The staged files will not disappear due to restarts because workDir is a
// folder that is mapped to the host (and therefore survives restarts).
for _, f := range requirementsFiles {
if err := pipInstallRequirements(files, workDir, f); err != nil {
return fmt.Errorf("failed to install requirements: %v", err)
return fmt.Errorf("failed to install requirements (this may be unrecoverable): %v", err)
}
}
if err := installExtraPackages(files, extraPackagesFile, workDir); err != nil {
return fmt.Errorf("failed to install extra packages: %v", err)
return fmt.Errorf("failed to install extra packages (this may be unrecoverable): %v", err)
}
if err := pipInstallPackage(files, workDir, workflowFile, false, true, nil); err != nil {
return fmt.Errorf("failed to install workflow: %v", err)
return fmt.Errorf("failed to install workflow (this may be unrecoverable): %v", err)
}

return nil
Expand Down