-
Notifications
You must be signed in to change notification settings - Fork 55
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
Snow 1703629 log pip error message #1701
Conversation
f"pip wheel finished with error code {result.returncode}. Please re-run with --verbose or --debug for more details." | ||
) | ||
else: | ||
log.info("Pip wheel command executed successfully") |
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.
Do we want spam this 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.
I kept that as I liked the log consistency - "running pip" -> "pip error" or "running pip" -> "pip success", no strong opinion though
f"pip wheel finished with error code {result.returncode}. Please re-run with --verbose or --debug for more details." | ||
) | ||
else: | ||
log.info("Pip wheel command executed successfully") |
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.
Please decide on one version 😄 pip wheel
vs Pip wheel
.
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.
pip wheel
, fixed
tests_integration/test_package.py
Outdated
@pytest.mark.integration | ||
def test_incorrect_input(self, runner): | ||
# TODO: refactor snowpark, so pip error is always thrown |
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.
For who is this TODO? Do we want to create jira ticket for it?
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.
Ah, it was for me from the morning, changes are already applied. Removed :D
* Add test * implement fix * move test to integration * refactor * Fix tests * update release notes * review fixes
* Add test * implement fix * move test to integration * refactor * Fix tests * update release notes * review fixes
* Add test * implement fix * move test to integration * refactor * Fix tests * update release notes * review fixes
Pre-review checklist
Changes description
Add pip error details to logs.
Small refactor of
download_unavailable_packages
(raise exception earlier, as all usages were immediately raising in casesucceded==False
)