buildchain, docs, salt, tests: update Python dependencies #2986

NicolasT · 2020-12-16T17:21:15Z

Ran tox -e pip-compile to bring everything up-to-date.

The dependabot-generated PR fails because for some reason, importlib-metadata is (attempted to be) installed but has no entry in requirements.txt (and hence no hash). Turns out this is due to pluggy now using importlib.metadata from the standard library when using Python >= 3.8, which is likely what @dependabot uses.

Hence, pinning the Python version used for pip-compile to Python 3.6, which is what RHEL/CentOS7 comes with (and what we run in CI).

The updated dependencies also bring a new version of pylint (which doesn't work with Python 3.9, hence also pinning to 3.6...). This introduces some new warnings, which are fixed in some related commits as well.

Finally, rename some unfortunately-named variables in a tox.ini script.

Closes: #2897
Closes: #2898
See: pytest-dev/pluggy#222

bert-e · 2020-12-16T17:21:18Z

Hello nicolast,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2020-12-16T17:21:23Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The arrays were called `py3_files` and `py2_files`, even though they're not strictly referring to Python3 or Python2 files: the `sparse_volume_cleanup` script is actually a Python3 script (according to its shebang). The actual split is some files are linted using `mypy`, others are not. Hence renaming into `typed_files` and `untyped_files`. See: https://scality.slack.com/archives/CFXQQUDPT/p1608200682012800

Some future commit updates the version of `pylint` we use to validate Python code, which adds a warning about using `super(Klass, self)` which can be replaced with `super()`.

Some future commit updates the version of `pylint` we use to validate Python code, which adds a warning about using `raise ... from ...` when applicable.

A future commit updates the version of `pylint` we use to validate Python code, which does not work properly with Python 3.9 at the time of writing, which is the default Python on e.g., a Fedora 32 system. Instead, pin the Python version to the (Python 3) version as found on our target system(s), i.e., RHEL/CentOS 7, so Python 3.6. See: pylint-dev/pylint#3882 See: https://scality.slack.com/archives/CFXQQUDPT/p1608201195013900?thread_ts=1608200122.011200&cid=CFXQQUDPT

It appears `pytest-bdd` 4.0.2 introduced changes which are incompatible with our current test-suite. To be investigated.

NicolasT · 2020-12-17T16:24:15Z

/approve

bert-e · 2020-12-17T16:24:21Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

slaperche-scality

LGTM, but a Mac user (@thomasdanan or @ChengYanJin) should run the pip-compile to also update buildchain/requirements-Darwin.txt.

NicolasT · 2020-12-17T16:52:11Z

LGTM, but a Mac user (@thomasdanan or @ChengYanJin) should run the pip-compile to also update buildchain/requirements-Darwin.txt.

@thomasdanan will do. I wonder whether there's a way to unify these two... The only difference appears to be including pyinotify on Linux and macfsevents on Darwin, both dependencies of doit. Could we pin some version of them in some other file with a platform-specific selector, and include that file with the pip install -r invocation?

slaperche-scality · 2020-12-17T17:32:27Z

I wonder whether there's a way to unify these two... The only difference appears to be including pyinotify on Linux and macfsevents on Darwin, both dependencies of doit. Could we pin some version of them in some other file with a platform-specific selector, and include that file with the pip install -r invocation?

I remember trying a lot of things back then, but none of them worked out, but I may have missed something (environment markers sounded like a good option, but didn't work with pip-compile)
The current solution is far from being convenient, clearly :/

NicolasT · 2020-12-18T10:52:38Z

I wonder whether there's a way to unify these two... The only difference appears to be including pyinotify on Linux and macfsevents on Darwin, both dependencies of doit. Could we pin some version of them in some other file with a platform-specific selector, and include that file with the pip install -r invocation?

I remember trying a lot of things back then, but none of them worked out, but I may have missed something (environment markers sounded like a good option, but didn't work with pip-compile)
The current solution is far from being convenient, clearly :/

Here's a suggestion:

We create a buildchain/constraints.txt which constraints both pyinotify and macfsevents to a specific (current) version. From now on these are 'special' packages
We create buildchain/platform-requirements.txt manually which includes both pyinotify and macfsevents at the version specified in the constraint file, including hashes and whatnot, and platform markers
We add -c constraints.txt to buildchain/requirements.in
We add a filter which runs after pip-compile (on buildchain/requirements.in) to remove both pyinotify and macfsevents from the resulting requirements.txt, and add -r platform-requirements.txt to the top

If I'm not mistaken, this then allows to run pip install -r buildchain/requirements.txt and get everything we want. Unless at some point one of the top-level dependencies adds some other platform-specific dependency, but 🤷‍♂️

NicolasT · 2020-12-18T13:19:11Z

For this PR, I'm going to revert the changes in buildchain/requirements-*.txt (and note so in the commit message), so this can go in (without updating the requirements for doit, both Linux and Darwin). Then later we can look into coding the potential solution.

@JBWatenbergScality tried updating on an OSX machine, but it turns out Python3.6 is not supported on Big Sur sigh

Due to changes in `pluggy` (a dependency of `pytest`) using the `importlib.metadata` module from the standard library instead of the `importlib-metadata` package when running on Python >= 3.8, we need to pin the `basepython` used in the `pip-compile` Tox env to the version that's actually used in CI, otherwise `pip` complains during installation of dependencies the `importlib-metadata` package is not listed (with hashes) in some `requirements.txt`. As such, setting `basepython` to `python3.6`, similar to 8a9104b. Also, update the versions of `pip` and `pip-tools` used in the `pip-compile` `tox` target, and use the `--allow-unsafe` flag which is supposedly not really unsafe at all (and will become the default). Finally, since we're currently unable to properly update the MacOS X platform-specific dependencies for the buildchain (in `buildchain/requirements-Darwin.txt`), the updates to `buildchain/requirements-Linux.txt` are manually reverted (after running `tox -e pip-compile` on a Linux host) to ensure both `buildchain/requirements-Linux.txt` and `buildchain/requirements-Darwin.txt` remain in sync (except for `pyinotify` and `macfsevents`, as usual). See: 8a9104b Closes: #2897 Closes: #2898 See: pytest-dev/pluggy#222 See: https://github.com/pytest-dev/pluggy/pull/223/files#diff-60f61ab7a8d1910d86d9fda2261620314edcae5894d5aaa236b821c7256badd7 See: jazzband/pip-tools#806 (comment) See: #2986 (comment)

NicolasT · 2020-12-18T13:37:57Z

@bert-e status

bert-e · 2020-12-18T13:37:59Z

Status

Status report is not available.

The following options are set: approve

bert-e · 2020-12-18T13:38:04Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

NicolasT · 2020-12-18T14:05:45Z

/approve

bert-e · 2020-12-18T15:03:57Z

Build failed

The build for commit did not succeed in branch improvement/update-cryptography.

The following options are set: approve

Instead of having both a `buildchain/requirements-Linux.txt` file and `buildchain/requirements-Darwin.txt`, rendered from `buildchain/requirements.in` and as such being almost identical (modulo the platform-specific `pyinotify` and `macfsevents` modules), making updates to the requirement lock files cumbersome (need to be executed on all platforms), this commit creates a file containing a pinned version of these two dependencies including a (manually added) platform constraint, which is then used as a constraint file for `pip-compile` to do its job (hence the `-c platform-requirements.txt` line in `requirements.in`). After `pip-compile` generates a temporary lockfile (cfr. code in `tox.ini`), we parse out the `pyinotify` and/or `macfsevents` dependency, and append the constraint file to the result. Using this approach we end up with a single `buildchain/requirements.txt` which has all cross-platform `doit` (and other) dependencies from `buildchain/requirements.in`, and (platform-constrained) mentions of `pyinotify` and `macfsevents`. The versions of the latter are pinned in `platform-requirements.txt`, so these won't be upgraded when running `tox -e pip-compile`. If need be, they can be bumped manually. See: #2986 (comment)

bert-e · 2020-12-18T17:37:35Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: approve

gdemonet

LGTM 👌

bert-e · 2020-12-21T09:26:19Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.7

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve

bert-e · 2020-12-21T23:47:22Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.7

The following branches have NOT changed:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6

Please check the status of the associated issue None.

Goodbye nicolast.

NicolasT added topic:tests What's not tested may be broken topic:build Anything related to building steps complexity:easy Something that requires less than a day to fix kind:dependencies Pull requests that update a dependency file python Pull requests that update Python code labels Dec 16, 2020

NicolasT requested a review from a team as a code owner December 16, 2020 17:21

NicolasT force-pushed the improvement/update-cryptography branch from 1d1891e to 0a4bbe1 Compare December 16, 2020 18:34

NicolasT changed the title ~~buildchain, docs, salt, tests: update cryptography~~ buildchain, docs, salt, tests: update Python dependencies Dec 16, 2020

NicolasT force-pushed the improvement/update-cryptography branch from d721417 to 7e87a07 Compare December 16, 2020 20:02

NicolasT added 3 commits December 17, 2020 10:45

salt: use Python3-style super()

4c3b462

Some future commit updates the version of `pylint` we use to validate Python code, which adds a warning about using `super(Klass, self)` which can be replaced with `super()`.

buildchain, salt: use Python3 raise ... from ...

2209929

Some future commit updates the version of `pylint` we use to validate Python code, which adds a warning about using `raise ... from ...` when applicable.

NicolasT force-pushed the improvement/update-cryptography branch 2 times, most recently from 90c9af1 to 05b8c03 Compare December 17, 2020 10:57

NicolasT force-pushed the improvement/update-cryptography branch from 05b8c03 to 47bfe9e Compare December 17, 2020 11:12

tests: pin pytest-bdd to a compatible version

aa16e00

It appears `pytest-bdd` 4.0.2 introduced changes which are incompatible with our current test-suite. To be investigated.

NicolasT force-pushed the improvement/update-cryptography branch from 47bfe9e to 635d0fe Compare December 17, 2020 14:13

slaperche-scality reviewed Dec 17, 2020

View reviewed changes

NicolasT force-pushed the improvement/update-cryptography branch from 635d0fe to 46f1f76 Compare December 18, 2020 13:24

JBWatenbergScality previously approved these changes Dec 18, 2020

View reviewed changes

NicolasT dismissed JBWatenbergScality’s stale review via f1f5f9c December 18, 2020 17:37

gdemonet approved these changes Dec 21, 2020

View reviewed changes

This was referenced Dec 21, 2020

[Snyk] Security upgrade py from 1.8.1 to 1.10.0 #2983

Closed

[Snyk] Security upgrade urllib3 from 1.25.8 to 1.25.9 #2826

Closed

bert-e merged commit ca06c0e into development/2.7 Dec 21, 2020

bert-e deleted the improvement/update-cryptography branch December 21, 2020 23:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

buildchain, docs, salt, tests: update Python dependencies #2986

buildchain, docs, salt, tests: update Python dependencies #2986

NicolasT commented Dec 16, 2020 •

edited

Loading

bert-e commented Dec 16, 2020

bert-e commented Dec 16, 2020

NicolasT commented Dec 17, 2020

bert-e commented Dec 17, 2020

slaperche-scality left a comment

NicolasT commented Dec 17, 2020

slaperche-scality commented Dec 17, 2020 •

edited

Loading

NicolasT commented Dec 18, 2020

NicolasT commented Dec 18, 2020

NicolasT commented Dec 18, 2020

bert-e commented Dec 18, 2020

bert-e commented Dec 18, 2020

NicolasT commented Dec 18, 2020

bert-e commented Dec 18, 2020

bert-e commented Dec 18, 2020

gdemonet left a comment

bert-e commented Dec 21, 2020

bert-e commented Dec 21, 2020

buildchain, docs, salt, tests: update Python dependencies #2986

buildchain, docs, salt, tests: update Python dependencies #2986

Conversation

NicolasT commented Dec 16, 2020 • edited Loading

bert-e commented Dec 16, 2020

Hello nicolast,

bert-e commented Dec 16, 2020

Waiting for approval

NicolasT commented Dec 17, 2020

bert-e commented Dec 17, 2020

Waiting for approval

slaperche-scality left a comment

Choose a reason for hiding this comment

NicolasT commented Dec 17, 2020

slaperche-scality commented Dec 17, 2020 • edited Loading

NicolasT commented Dec 18, 2020

NicolasT commented Dec 18, 2020

NicolasT commented Dec 18, 2020

bert-e commented Dec 18, 2020

Status

bert-e commented Dec 18, 2020

Waiting for approval

NicolasT commented Dec 18, 2020

bert-e commented Dec 18, 2020

Build failed

bert-e commented Dec 18, 2020

Waiting for approval

gdemonet left a comment

Choose a reason for hiding this comment

bert-e commented Dec 21, 2020

In the queue

bert-e commented Dec 21, 2020

NicolasT commented Dec 16, 2020 •

edited

Loading

slaperche-scality commented Dec 17, 2020 •

edited

Loading