-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Resolve pytest pluggy version conflict #1457
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
Resolve pytest pluggy version conflict #1457
Conversation
Pluggy released a new version today but pytest 3.6.4 explicitly forbade it, so the urllib3 builds started to fail. Pytest 3.8.2 allows pluggy 0.8.0 so they will work with one another.
This isn't strictly necessary, but it looks as if this project prefers to pin its dev dependencies to an exact version (probably to prevent what happened today when a pluggy release caused the urllib3 build to start failing).
Codecov Report
@@ Coverage Diff @@
## master #1457 +/- ##
=======================================
Coverage 82.09% 82.09%
=======================================
Files 22 22
Lines 2312 2312
=======================================
Hits 1898 1898
Misses 414 414 Continue to review full report at Codecov.
|
# https://github.com/GoogleCloudPlatform/python-repo-tools/issues/23 | ||
pylint<2.0;python_version<="2.7" | ||
gcp-devrel-py-tools==0.0.15 | ||
pluggy==0.8.0 |
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.
pluggy==0.8.0 |
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.
Can we get away with not pinning pluggy here?
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.
We can get away with not pinning pluggy, but keep in mind that a new version of pluggy is what broke the build yesterday. I saw the other requirements that were pinned to specific versions and inferred that this project likes to have stable dependencies but the build will succeed without 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.
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.
Arguably that was a bug in pytest 3.6.4, which should have pinned pluggy, just like requests pins urllib3. I think we either want to go the full pip-compile + dependabot route, or just list the dependencies we use, which means we'll get occasional breakage like this. Of course, the maintainers decide. :)
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.
Pytest 3.6.4 did require a range for pluggy (>=0.5,<0.8) but some other module that was loaded sooner caused pluggy 0.8.0 to load so when pytest asked for <0.8 things got crashy. The pain of "DLL hell" is still real.
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.
Oh, right. tox 2.9.1 requires pluggy>=0.3.0,<1.0
and is placed before pytest, which is why pluggy 0.8.0 was installed. Really, the only real way to fix this is to use pip-compile, and possibly dependabot to help with the constant updates.
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 think merging this as-is is fine for now. We'll have to revisit our whole reproducible build strategy collectively to solve this problem completely :)
This resolves issue #1455 .