-
Notifications
You must be signed in to change notification settings - Fork 9
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
check for message attribute #81
Conversation
@neelasha23 I'm guessing this is because of: https://github.com/ploomber/cli/issues/7 ? is it because ClickException has |
yes
Yes it doesn't seem to be accessing the have created a test PR in JupySQL to ensure the change doesn't affect the JupySQL tests. |
7988484
to
c047cc7
Compare
@edublancas Ready for review. |
@@ -87,6 +87,8 @@ def _add_community_link(e): | |||
elif COMMUNITY not in e.args[0]: | |||
message = e.args[0] + COMMUNITY | |||
e.args = (message,) | |||
if hasattr(e, "message"): |
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.
just add a short comment explaining why we have this edge case
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.
I've released this in 0.2.19: 538e7e7 should be on PyPI in a few minutes, please update your other PR and pin |
Describe your changes
Added check for
message
attribute when displaying Slack link. This is required for exceptions which havemessage
as their first parameter and notargs
.Issue number
Closes #X
Checklist before requesting a review
pkgmt format
📚 Documentation preview 📚: https://ploomber-core--81.org.readthedocs.build/en/81/