Skip to content
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

eth-abi 1.2.2 has requirement eth-typing<2, but you'll have eth-typing 2.0.0 which is incompatible.but you'll ha… #1538

Merged
merged 4 commits into from
Dec 7, 2018

Conversation

javimaravillas
Copy link

@javimaravillas javimaravillas commented Dec 1, 2018

#1484

What was wrong?

  1. pytest 3.6.4 has requirement pluggy<0.8,>=0.5, but you'll have pluggy 0.8.0 which is incompatible.
  2. eth-abi 1.2.2 has requirement eth-typing<2, but you'll have eth-typing 2.0.0 which is incompatible.
  3. requests 2.20.1 has requirement idna<2.8,>=2.5, but you'll have idna 2.8 which is incompatible.

How was it fixed?

  1. tox was installing the latest version of pluggy 0.8 that is incompatible with pytest so pytest was not working. Fixing a version of pluggy minor to 0.8 makes pytest work without an issue for tox

  2. Releasing a new version of eth-abi using eth-typing 2.0.0 (waiting for a PR in eth-abi to be ready)

  3. Fixing idna to use 2.7 is fixing the issue

Cute Animal Picture

put a cute animal picture link inside the parentheses

@javimaravillas javimaravillas changed the title 1484 fix pytest 3.6.4 has requirement pluggy<0.8,>=0.5, but you'll ha… [WIP] 1484 fix pytest 3.6.4 has requirement pluggy<0.8,>=0.5, but you'll ha… Dec 1, 2018
@@ -40,7 +40,7 @@
"async-generator==1.10",
"bloom-filter==1.3",
"cachetools>=2.1.0,<3.0.0",
"coincurve>=8.0.0,<9.0.0",
"coincurve>=10.0.0,<11.0.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

@javimaravillas what's the reason for bumping these?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, ok I just read it was suggested here as lower versions have issues with FreeBSD. Sounds fair.

@@ -83,6 +83,7 @@
"bumpversion>=0.5.3,<1",
"wheel",
"setuptools>=36.2.0",
"pluggy==0.7.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if we add this as a standalone dependency to workaround the issue, we should put a comment above that line as an explainer. Something like:

Pinning pluggy version to workaround incompatibility issues between certain pytest and tox versions.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@javimaravillas
Copy link
Author

This PR is waiting for ethereum/eth-abi#110

@javimaravillas javimaravillas changed the title [WIP] 1484 fix pytest 3.6.4 has requirement pluggy<0.8,>=0.5, but you'll ha… 1484 fix pytest 3.6.4 has requirement pluggy<0.8,>=0.5, but you'll ha… Dec 6, 2018
@javimaravillas javimaravillas changed the title 1484 fix pytest 3.6.4 has requirement pluggy<0.8,>=0.5, but you'll ha… eth-abi 1.2.2 has requirement eth-typing<2, but you'll have eth-typing 2.0.0 which is incompatible.but you'll ha… Dec 6, 2018
@javimaravillas
Copy link
Author

This PR is complete to review and merge

Copy link
Contributor

@carver carver left a comment

Choose a reason for hiding this comment

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

LGTM!

#Fixing this dependency due to: pytest 3.6.4 has requirement pluggy<0.8,>=0.5, but you'll have pluggy 0.8.0 which is incompatible.
"pluggy==0.7.1",
#Fixing this dependency due to: requests 2.20.1 has requirement idna<2.8,>=2.5, but you'll have idna 2.8 which is incompatible.
"idna==2.7",
Copy link
Contributor

Choose a reason for hiding this comment

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

Just as an FYI, I think it would be ideal to do >=2.5,<2.8 here, but since it's only a dev dependency, I'm okay pinning it.

@carver carver merged commit b10f1ed into ethereum:master Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants