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

Post-mortem for broken web3 4.7.0 release. #1053

Closed
dylanjw opened this issue Sep 11, 2018 · 9 comments
Closed

Post-mortem for broken web3 4.7.0 release. #1053

dylanjw opened this issue Sep 11, 2018 · 9 comments

Comments

@dylanjw
Copy link
Contributor

dylanjw commented Sep 11, 2018

See #1052

It was discovered that web3 4.7.0 would attempt to install pypiwin32 on all
platforms when installed from pypi with pip whereas a local install with setup.py or
the built tar.gz package did not.

Ref:

setuptools had a bug where environment markers get stripped from the wheel's
MANIFEST.

I had started out building with the broken package with pip version 9.0.3 and
setuptools 28.8.0. (doh!)

$ python setup.py sdist bdist_wheel
$ grep pypiwin32 web3.egg-info/requires.txt
pypiwin32>=223;platform_system=='Windows'

$ mkdir web3_whl; cd web3_whl
$ unzip ../dist/web3-4.7.0-py3-none-any.whl
$ grep pypiwin32 web3-4.7.0.dist-info/METADATA
Requires-Dist: pypiwin32 (>=223)

Upgrading pip was not sufficient to fix the issue:

$ pip install pip --upgrade
$ pip show pip
Name: pip
Version: 18.0
Summary: The PyPA recommended tool for installing Python packages.
Home-page: https://pip.pypa.io/
Author: The pip developers
Author-email: [email protected]
License: MIT
Location: /home/dwilson/Develop/Projects/ethereum/web3.py/venv-test/lib/python3.6/site-packages
Requires: 
Required-by: 
$ pip show setuptools
Name: setuptools
Version: 28.8.0
Summary: Easily download, build, install, upgrade, and uninstall Python packages
Home-page: https://github.com/pypa/setuptools
Author: Python Packaging Authority
Author-email: [email protected]
License: UNKNOWN
Location: /home/dwilson/Develop/Projects/ethereum/web3.py/venv-test/lib/python3.6/site-packages
Requires: 
Required-by: 

$ python setup.py sdist bdist_wheel
$ grep pypiwin32 web3.egg-info/requires.txt
pypiwin32>=223;platform_system=='Windows'
$ grep pypiwin32 web3-4.7.0.dist-info/METADATA
Requires-Dist: pypiwin32 (>=223)

After upgrading setuptools the MANIFEST file included in the wheel contains the
environment markers.

$ pip install setuptools --upgrade
Collecting setuptools
  Using cached https://files.pythonhosted.org/packages/66/e8/570bb5ca88a8bcd2a1db9c6246bb66615750663ffaaeada95b04ffe74e12/setuptools-40.2.0-py2.py3-none-any.whl
Installing collected packages: setuptools
  Found existing installation: setuptools 28.8.0
    Uninstalling setuptools-28.8.0:
      Successfully uninstalled setuptools-28.8.0
Successfully installed setuptools-40.2.0
$ grep pypiwin32 web3-4.7.0.dist-info/METADATA
Requires-Dist: pypiwin32 (>=223); platform_system == "Windows"
@dylanjw
Copy link
Contributor Author

dylanjw commented Sep 11, 2018

To prevent this in the future, a requirement for setuptools 36.2.0 or greater could be added to setup.py:

e.g.

import setuptools.__version__


if StrictVersion(setuptools.__version__) < StrictVersion('36.2.0'):
    print('your setuptools version does not support PEP 508. Upgrade setuptools and repeat the installation.')
    sys.exit(1)

@voith
Copy link
Contributor

voith commented Sep 11, 2018

@dylanjw Mirroring what I said on gitter.
Can we have an automated script that deploys to pypi. This script should take the credentials and the version number as command line arguments. After deploying to pypi the script downloads the released version and runs tests against them.

Bonus: Also run tests against other ethereum projects that use web3. eg. py-evm, populus...
This way we can be sure that a release has not broken anything!

@dylanjw
Copy link
Contributor Author

dylanjw commented Sep 11, 2018

It would bee cool if the wheel could be tested before it gets uploaded to pypi.

Maybe the test script you are describing could be broken into 2. A script the detects pypi updates and runs tests across different projects to check for broken dependency issues. Another set of tests that run as part of the build script before uploading the wheel to pypi.

@voith
Copy link
Contributor

voith commented Sep 11, 2018

yeah, you're right. Tests need to run pre-relase with the newly created wheel and post-release across repositories.

A script the detects pypi updates and runs tests across different projects to check for broken dependency issues.

If pypi could provide such a hook then it would be great!

@dylanjw
Copy link
Contributor Author

dylanjw commented Sep 11, 2018

If pypi could provide such a hook then it would be great!

Spent some time looking, and havent found a convenient post-release hook from pypi. The closest I have found is libraries.io that will send you an email when a dependency has a new version.

@pipermerriam
Copy link
Member

I think it would be sufficient to add setuptools>=36.0.0 to the [dev] requirements section of extras_require.

Regarding @voith suggesting about testing the builds:

I don't think the complexity justifies the protection it would offer. Complexity is likely to be a non-trivial as it will have to work across OSX/Linux, as well as across python versions. It could easily pass locally but fail for others due to differences in python version and platforms. To prevent some of these false positives, our testing scripts could test across multiple python versions, etc, but this is why I don't think the complexity is justified by the protection it offers.

dylanjw added a commit to dylanjw/web3.py that referenced this issue Sep 11, 2018
@carver
Copy link
Collaborator

carver commented Sep 12, 2018

I don't think the complexity justifies the protection it would offer.

I agree, especially about the post-deployment hook. I've seen something like that implemented, and it's really hard to tell the difference between a downstream failure and a web3 failure, which adds a lot of noise to deployment.

I do think there might be value in a smoke test in a local pre-deployment install from the built wheel.

@voith
Copy link
Contributor

voith commented Sep 12, 2018

Well, l I didn't mean to run an entire integration test-suite.
Instead, we could just test pip install project. If not automated then at least the release manager should check that the installation works fine as part of the basic sanity check.
The recent eth-hash version that broke installation of eth-tester could have easily been detected if there was a test that checked if pip install eth-tester worked fine after eth-hash was deployed.

@voith
Copy link
Contributor

voith commented Sep 16, 2018

ethereum/eth-tester#127: Yet another instance of dependency hell.

ankitchiplunkar pushed a commit to analyseether/web3.py that referenced this issue Sep 19, 2018
see ethereum#1053

added feature for inputing block_identifier in eth_estimateGas command

variable inputs in estimateGas and tests

formatter for block_identifier and passing tests

added feature for inputing block_identifier in eth_estimateGas command

variable inputs in estimateGas and tests

passing lint
ankitchiplunkar pushed a commit to analyseether/web3.py that referenced this issue Sep 19, 2018
see ethereum#1053

added feature for inputing block_identifier in eth_estimateGas command

variable inputs in estimateGas and tests

formatter for block_identifier and passing tests

added feature for inputing block_identifier in eth_estimateGas command

variable inputs in estimateGas and tests

passing lint
kclowes pushed a commit to pjryan93/web3.py that referenced this issue Apr 25, 2019
kclowes pushed a commit to pjryan93/web3.py that referenced this issue Apr 25, 2019
see ethereum#1053

added feature for inputing block_identifier in eth_estimateGas command

variable inputs in estimateGas and tests

formatter for block_identifier and passing tests

added feature for inputing block_identifier in eth_estimateGas command

variable inputs in estimateGas and tests

passing lint
@fselmo fselmo closed this as completed Jul 7, 2022
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

No branches or pull requests

5 participants