[BEAM-824] Fixing error message when sdk_location not provided #1200
Conversation
| @@ -435,7 +435,13 @@ def _stage_dataflow_sdk_tarball(sdk_remote_location, staged_path, temp_dir): | |||
| _dependency_file_copy(sdk_remote_location, staged_path) | |||
| elif sdk_remote_location == 'pypi': | |||
| logging.info('Staging the SDK tarball from PyPI to %s', staged_path) | |||
| _dependency_file_copy(_download_pypi_sdk_package(temp_dir), staged_path) | |||
| import pkg_resources as pkg | |||
There was a problem hiding this comment.
It would make sense to push down this check to _download_pypi_sdk_package. This will remove the requirement for importing the same package here again, and you can raise the same RuntimeError there.
| try: | ||
| _dependency_file_copy(_download_pypi_sdk_package(temp_dir), staged_path) | ||
| except pkg.DistributionNotFound: | ||
| raise RuntimeError('Unable to stage SDK tarball. ' |
There was a problem hiding this comment.
I am not sure what would be a clear error message. There is no need to mention remote location that is an SDK internal.
Perhaps : 'Please set --sdk_location or install a valid GOOGLE_PACKAGE_NAME distribution.'
|
@aaltay what do you think now? : ) |
| try: | ||
| version = pkg.get_distribution(GOOGLE_PACKAGE_NAME).version | ||
| except pkg.DistributionNotFound: | ||
| raise RuntimeError('Please set --sdk_location or install a valid ' |
There was a problem hiding this comment.
--sdk_location command-line option instead of just --sdk_location. Because this is how we refer to the same thing in a related error in this same file.
|
LGTM. Thank you! |
|
@robertwb can you merge this, please? : ) |
|
@pabloem please manually close this PR (as apache doesn't give us write access to the repo). |
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify. (Even better, enableTravis-CI on your fork and ensure the whole test matrix passes).
<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
r: @aaltay can you take a look?
I tried adding the error strings to
pkg_resources.DistributionNotFound's.argstuple, but it seems to have special formatting for them so I couldn't add extra information on the error message. Let me know how you think we can phrase the error message.P.S. I'll remove the extra space in a new commit soon.