Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Add a better error message when deps not found #39

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

Merged
merged 2 commits into from
Nov 15, 2017
Merged

Add a better error message when deps not found #39

merged 2 commits into from
Nov 15, 2017

Conversation

hwright
Copy link
Contributor

@hwright hwright commented Nov 13, 2017

Use the skylark fail function instead of letting Python throw a
key-not-found exception.

(There may be a prettier way of doing this: I'm happy to be shown
differently, or to bikeshed the prose.)

Use the skylark `fail` function instead of letting Python throw a
key-not-found exception.
@bazel-io
Copy link

Can one of the admins verify this patch?

@mattmoor
Copy link
Contributor

Please test this

@mattmoor mattmoor self-requested a review November 15, 2017 14:30
return _requirements[name]
name_key = name.replace("-", "_").lower()
if name_key not in _requirements:
fail("Could not find pip-provided dependency: '%s'" % name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would be interesting to include what's available?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but since that list is unbounded, I'm concerned that it would be too much for this error string.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is Python, not Node.js ;-)

SGTM

@mattmoor
Copy link
Contributor

Test this please

@mattmoor mattmoor merged commit 1a1a539 into bazel-contrib:master Nov 15, 2017
alexeagle pushed a commit to alexeagle/rules_python that referenced this pull request Aug 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.