Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Extend the list of supported pip versions #91

Merged
merged 3 commits into from
Jun 11, 2020

Conversation

frenzymadness
Copy link
Collaborator

This change extends the list of supported versions of pip in our CI. I know it does not make sense to include all available releases of pip there so I included only the ones I consider important from the (RHEL) maintenance point of view and added comments for our future selfs.

Version 9 is important because it's the default in RHEL 8 (platform-python-pip) and micropipenv will need to work flawlessly with this version when we package it to RHEL.
We updated pip from 19.2.3 to 19.3.1 after we released Python 3.8 module so it would be nice to support both versions for some time.

By the way, micropipenv has install_requires=["pip>=9"] in its setup.py so any incompatibility uncovered by this change should be fixed rather sooner than later. If we decide to not support some specific version of pip for some reason, we should exclude that version in install_requires.

This introduces a breaking change

  • Yes
  • No

@sefkhet-abwy sefkhet-abwy bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Jun 9, 2020
@ghost
Copy link

ghost commented Jun 9, 2020

Build succeeded.

@fridex
Copy link
Collaborator

fridex commented Jun 9, 2020

Very nice! 👍

By the way, micropipenv has install_requires=["pip>=9"] in its setup.py so any incompatibility uncovered by this change should be fixed rather sooner than later. If we decide to not support some specific version of pip for some reason, we should exclude that version in install_requires.

Honestly, I did not check all the versions. I tried older fedora containers (I think down to f27 based on my sh history but not sure if the last worked well though). ubi8 worked well for me, ubi7 was too old IIRC (hence pip>=9).

@fridex
Copy link
Collaborator

fridex commented Jun 10, 2020

Briefly looking at the log, pip-19.2 had some changes in the editable API. It should be fairly doable to add support.

For pip-9.0, case the ValueError: invalid truth value '>=9.0,<10.0' error sounds strange at first look.

@fridex
Copy link
Collaborator

fridex commented Jun 10, 2020

For pip-9.0, case the ValueError: invalid truth value '>=9.0,<10.0' error sounds strange at first look.

BTW it might be due to PIP_VERSION environment variable collision. Are you ok with renaming it to something like MICROPIPENV_TEST_PIP_VERSION?

@frenzymadness
Copy link
Collaborator Author

For pip-9.0, case the ValueError: invalid truth value '>=9.0,<10.0' error sounds strange at first look.

BTW it might be due to PIP_VERSION environment variable collision. Are you ok with renaming it to something like MICROPIPENV_TEST_PIP_VERSION?

You are right, thank you.

$ pip --version
pip 9.0.3 from /home/lbalhar/.virtualenvs/tmp-cd8b4dae64c0d30/lib/python3.6/site-packages (python 3.6)
$ pip search tldr
tldr (1.0.0)         - command line client for tldr
snippy-tldr (0.2.0)  - Snippy plugin to import tldr man pages.
pytest-tldr (0.2.1)  - A pytest plugin that limits the output to just the things you need.
tldr.py (0.8.0)      - A python client for tldr: simplified and community-driven man pages.
You are using pip version 9.0.3, however version 20.1.1 is available.
You should consider upgrading via the 'pip install --upgrade pip' command.
$ PIP_VERSION=19.3.1 pip search tldr
Traceback (most recent call last):
  File "/home/lbalhar/.virtualenvs/tmp-cd8b4dae64c0d30/bin/pip", line 8, in <module>
    sys.exit(main())
  File "/home/lbalhar/.virtualenvs/tmp-cd8b4dae64c0d30/lib/python3.6/site-packages/pip/__init__.py", line 234, in main
    cmd_name, cmd_args = parseopts(args)
  File "/home/lbalhar/.virtualenvs/tmp-cd8b4dae64c0d30/lib/python3.6/site-packages/pip/__init__.py", line 183, in parseopts
    general_options, args_else = parser.parse_args(args)
  File "/usr/lib64/python3.6/optparse.py", line 1371, in parse_args
    values = self.get_default_values()
  File "/home/lbalhar/.virtualenvs/tmp-cd8b4dae64c0d30/lib/python3.6/site-packages/pip/baseparser.py", line 283, in get_default_values
    defaults = self._update_defaults(self.defaults.copy())  # ours
  File "/home/lbalhar/.virtualenvs/tmp-cd8b4dae64c0d30/lib/python3.6/site-packages/pip/baseparser.py", line 230, in _update_defaults
    val = strtobool(val)
  File "/usr/lib64/python3.6/distutils/util.py", line 317, in strtobool
    raise ValueError("invalid truth value %r" % (val,))
ValueError: invalid truth value '19.3.1'

I'll try to rename the variable.

PIP_VERSION env variable causes issues with pip 9.
@sefkhet-abwy sefkhet-abwy bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 10, 2020
@ghost
Copy link

ghost commented Jun 10, 2020

Build succeeded.

@frenzymadness
Copy link
Collaborator Author

The last commit fixed the compatibility with pip 9.

@fridex
Copy link
Collaborator

fridex commented Jun 10, 2020

The last commit fixed the compatibility with pip 9.

Nice! 👏

What about commenting out pip-19.2 tests for now? I'll try to look at the support in a separate PR if you are fine with that.

@frenzymadness
Copy link
Collaborator Author

The last commit fixed the compatibility with pip 9.

Nice! clap

What about commenting out pip-19.2 tests for now? I'll try to look at the support in a separate PR if you are fine with that.

How long do you think it will take to fix it? I am not against disabling it now but a fix would help me with the CI in the python container.

@fridex
Copy link
Collaborator

fridex commented Jun 11, 2020

The last commit fixed the compatibility with pip 9.

Nice! clap
What about commenting out pip-19.2 tests for now? I'll try to look at the support in a separate PR if you are fine with that.

How long do you think it will take to fix it? I am not against disabling it now but a fix would help me with the CI in the python container.

OK, I can have a look at it later today. 👍

@fridex
Copy link
Collaborator

fridex commented Jun 11, 2020

The last commit fixed the compatibility with pip 9.

Nice! clap
What about commenting out pip-19.2 tests for now? I'll try to look at the support in a separate PR if you are fine with that.

How long do you think it will take to fix it? I am not against disabling it now but a fix would help me with the CI in the python container.

OK, I can have a look at it later today.

Done in #92.

@fridex fridex mentioned this pull request Jun 11, 2020
2 tasks
@ghost
Copy link

ghost commented Jun 11, 2020

Build succeeded.

@fridex
Copy link
Collaborator

fridex commented Jun 11, 2020

... it looks like CI is giving this green flag. Let's merge this 👍 Thanks! 👏

Copy link
Collaborator

@fridex fridex left a comment

Choose a reason for hiding this comment

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

👍

@sefkhet-abwy sefkhet-abwy bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 11, 2020
@fridex fridex merged commit 69f7357 into thoth-station:master Jun 11, 2020
@sesheta
Copy link
Member

sesheta commented Jun 11, 2020

Successfully Merged.
Thanks for the contribution.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants