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

replace pkg_resources with importlib_metadata and packaging #5790

Merged
merged 2 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 44 additions & 33 deletions .github/workflows/test_fast.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
runs-on: ${{ matrix.os }}
timeout-minutes: 20
strategy:
fail-fast: false # Don't let a failed MacOS run stop the Ubuntu runs
fail-fast: false # don't stop on first failure
matrix:
os: ['ubuntu-latest']
python-version: ['3.7', '3.8', '3.9', '3.10', '3.11']
Expand All @@ -39,7 +39,7 @@ jobs:
if: startsWith(matrix.os, 'ubuntu')
run: |
sudo apt-get update
sudo apt-get install -y shellcheck sqlite3
sudo apt-get install -y sqlite3

- name: Install
run: |
Expand All @@ -48,37 +48,10 @@ jobs:
- name: Configure git # Needed by the odd test
uses: cylc/release-actions/configure-git@v1

- name: Check changelog
if: startsWith(matrix.os, 'ubuntu')
run: towncrier build --draft

- name: Style
if: startsWith(matrix.os, 'ubuntu')
run: |
flake8
etc/bin/shellchecker

# note: exclude python 3.10+ from mypy checks as these produce false
# positives in installed libraries for python 3.7
- name: Typing
if: startsWith(matrix.os, 'ubuntu') && startsWith(matrix.python-version, 3.9)
run: mypy

- name: Doctests
timeout-minutes: 4
run: |
pytest cylc/flow

- name: Unit Tests
timeout-minutes: 4
run: |
pytest tests/unit

- name: Bandit
if: ${{ matrix.python-version == '3.7' }}
# https://github.com/PyCQA/bandit/issues/658
timeout-minutes: 5
run: |
bandit -r --ini .bandit cylc/flow
pytest cylc/flow tests/unit
Copy link
Member

@MetRonnie MetRonnie Oct 30, 2023

Choose a reason for hiding this comment

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

If you're consolidating these into one step, it would be good to set -q for quiet in PYTEST_ADDOPTS, to prevent us having to scroll thousands of log lines to get to the test failures. Note that when CI=true Pytest always outputs full diffs in assertion failures regardless of the verbosity option.

Also -l for displaying the value of locals in traceback might be handy.

Copy link
Member Author

@oliver-sanders oliver-sanders Oct 31, 2023

Choose a reason for hiding this comment

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

I don't see combining these sections as a reason for reducing verbosity, better to have the required information than to hide it?

(It didn't really make sense to separate doc and unit tests they are all unit tests and splitting them just slows things down)

Copy link
Member

@MetRonnie MetRonnie Oct 31, 2023

Choose a reason for hiding this comment

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

I'm talking about hiding these (4000!) lines without reducing the verbosity of failed assertion output

image

Copy link
Member Author

@oliver-sanders oliver-sanders Oct 31, 2023

Choose a reason for hiding this comment

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

I don't really see a problem with this and seeing the test output is helpful for debugging timeouts. Unrelated either way.


- name: Integration Tests
timeout-minutes: 6
Expand All @@ -104,9 +77,47 @@ jobs:
path: coverage.xml
retention-days: 7

lint:
runs-on: 'ubuntu-latest'
timeout-minutes: 10
steps:
- name: Apt-Get Install
run: |
sudo apt-get update
sudo apt-get install -y shellcheck

- name: Checkout
uses: actions/checkout@v4

# note: exclude python 3.10+ from mypy checks as these produce false
# positives in installed libraries for python 3.7
- name: Configure Python
uses: actions/setup-python@v4
with:
python-version: 3.9

- name: Install
run: |
pip install -e ."[tests]"
oliver-sanders marked this conversation as resolved.
Show resolved Hide resolved

- name: Flake8
run: flake8

- name: Bandit
run: |
bandit -r --ini .bandit cylc/flow

- name: Shellchecker
run: etc/bin/shellchecker

- name: MyPy
run: mypy

- name: Towncrier
run: towncrier build --draft

- name: Linkcheck
if: startsWith(matrix.python-version, '3.10')
run: pytest -m linkcheck --dist=load tests/unit
run: pytest -m linkcheck --dist=load --color=yes -n 10 tests/unit/test_links.py

codecov:
needs: test
Expand Down
4 changes: 2 additions & 2 deletions conda-environment.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@ dependencies:
# Note: can't pin jinja2 any higher than this until we give up on Cylc 7 back-compat
- jinja2 >=3.0,<3.1
- metomi-isodatetime >=1!3.0.0, <1!3.2.0
- packaging
# Constrain protobuf version for compatible Scheduler-UIS comms across hosts
- protobuf >=4.21.2,<4.22.0
- psutil >=5.6.0
- python
- pyzmq >=22
- setuptools >=49,!=67.*
- importlib_metadata # [py<3.8]
- importlib_metadata >=5.0 # [py<3.12]
- urwid >=2,<3
- tomli >=2 # [py<3.11]

Expand Down
18 changes: 13 additions & 5 deletions cylc/flow/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,19 @@

def iter_entry_points(entry_point_name):
"""Iterate over Cylc entry points."""
import pkg_resources
import sys
if sys.version_info[:2] > (3, 11):
from importlib.metadata import entry_points

Check warning on line 63 in cylc/flow/__init__.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/__init__.py#L63

Added line #L63 was not covered by tests
else:
# BACK COMPAT: importlib_metadata
# importlib.metadata was added in Python 3.8. The required interfaces
# were completed by 3.12. For lower versions we must use the
# importlib_metadata backport.
# FROM: Python 3.7
# TO: Python: 3.12
from importlib_metadata import entry_points
yield from (
entry_point
for entry_point in pkg_resources.iter_entry_points(entry_point_name)
# Filter out the cylc namespace as it should be empty.
# All cylc packages should take the form cylc-<name>
if entry_point.dist.key != 'cylc'
# for entry_point in entry_points()[entry_point_name]
for entry_point in entry_points().select(group=entry_point_name)
)
5 changes: 2 additions & 3 deletions cylc/flow/cfgspec/globalcfg.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
from typing import List, Optional, Tuple, Any, Union

from contextlib import suppress
from pkg_resources import parse_version
from packaging.version import parse as parse_version, Version

from cylc.flow import LOG
from cylc.flow import __version__ as CYLC_VERSION
Expand Down Expand Up @@ -1866,8 +1866,7 @@ def get_version_hierarchy(version: str) -> List[str]:
['', '8', '8.0', '8.0.1', '8.0.1a2', '8.0.1a2.dev']

"""
smart_ver: Any = parse_version(version)
# (No type anno. yet for Version in pkg_resources.extern.packaging.version)
smart_ver: Version = parse_version(version)
base = [str(i) for i in smart_ver.release]
hierarchy = ['']
hierarchy += ['.'.join(base[:i]) for i in range(1, len(base) + 1)]
Expand Down
16 changes: 5 additions & 11 deletions cylc/flow/network/scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,8 @@
import re
from typing import AsyncGenerator, Dict, Iterable, List, Optional, Tuple, Union

from pkg_resources import (
parse_requirements,
parse_version
)
from packaging.version import parse as parse_version
from packaging.specifiers import SpecifierSet

from cylc.flow import LOG
from cylc.flow.async_util import (
Expand Down Expand Up @@ -354,11 +352,7 @@

def parse_requirement(requirement_string):
"""Parse a requirement from a requirement string."""
# we have to give the requirement a name but what we call it doesn't
# actually matter
for req in parse_requirements(f'x {requirement_string}'):
# there should only be one requirement
return (req,), {}
return (SpecifierSet(requirement_string),), {}

Check warning on line 355 in cylc/flow/network/scan.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/network/scan.py#L355

Added line #L355 was not covered by tests


@pipe(preproc=parse_requirement)
Expand All @@ -373,7 +367,7 @@
flow (dict):
Flow information dictionary, provided by scan through the pipe.
requirement (str):
Requirement specifier in pkg_resources format e.g. ``> 8, < 9``
Requirement specifier in PEP 440 format e.g. ``> 8, < 9``

"""
return parse_version(flow[ContactFileFields.VERSION]) in requirement
Expand All @@ -391,7 +385,7 @@
flow (dict):
Flow information dictionary, provided by scan through the pipe.
requirement (str):
Requirement specifier in pkg_resources format e.g. ``> 8, < 9``
Requirement specifier in PEP 440 format e.g. ``> 8, < 9``

"""
return parse_version(flow[ContactFileFields.API]) in requirement
Expand Down
2 changes: 1 addition & 1 deletion cylc/flow/parsec/fileparse.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
get_workflow_source_dir, check_flow_file)

if t.TYPE_CHECKING:
from optparse import Values

Check warning on line 53 in cylc/flow/parsec/fileparse.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/parsec/fileparse.py#L53

Added line #L53 was not covered by tests


# heading/sections can contain commas (namespace name lists) and any
Expand Down Expand Up @@ -277,7 +277,7 @@
try:
# If you want it to work on sourcedirs you need to get the options
# to here.
plugin_result = entry_point.resolve()(
plugin_result = entry_point.load()(

Check warning on line 280 in cylc/flow/parsec/fileparse.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/parsec/fileparse.py#L280

Added line #L280 was not covered by tests
srcdir=fpath, opts=opts
)
except Exception as exc:
Expand Down Expand Up @@ -426,12 +426,12 @@

try:
original_cwd = os.getcwd()
except FileNotFoundError:

Check warning on line 429 in cylc/flow/parsec/fileparse.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/parsec/fileparse.py#L429

Added line #L429 was not covered by tests
# User's current working directory does not actually exist, so we won't
# be able to change back to it later. (Note this might not be enough to
# prevent file parsing commands failing due to missing cwd elsewhere in
# the Python library).
original_cwd = None

Check warning on line 434 in cylc/flow/parsec/fileparse.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/parsec/fileparse.py#L434

Added line #L434 was not covered by tests

# Move to the file location to give the template processor easy access to
# other files in the workflow directory (whether source or installed).
Expand Down
2 changes: 1 addition & 1 deletion cylc/flow/scheduler_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
import sys
from typing import TYPE_CHECKING

from pkg_resources import parse_version
from packaging.version import parse as parse_version

from cylc.flow import LOG, __version__
from cylc.flow.exceptions import (
Expand Down
26 changes: 11 additions & 15 deletions cylc/flow/scripts/completion_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,8 @@
import sys
import typing as t

from pkg_resources import (
parse_requirements,
parse_version
)
from packaging.version import parse as parse_version
from packaging.specifiers import SpecifierSet

from cylc.flow.cfgspec.glbl_cfg import glbl_cfg
from cylc.flow.id import tokenise, IDTokens, Tokens
Expand All @@ -74,7 +72,7 @@
# set the compatibility range for completion scripts with this server
# I.E. if we change the server interface, change this compatibility range.
# User's will be presented with an upgrade notice if this happens.
REQUIRED_SCRIPT_VERSION = 'completion-script >=1.0.0, <2.0.0'
REQUIRED_SCRIPT_VERSION = '>=1.0.0, <2.0.0'

# register the psudo "help" and "version" commands
COMMAND_LIST = list(COMMANDS) + ['help', 'version']
Expand Down Expand Up @@ -181,7 +179,7 @@
return await complete_command()
if length == 1:
return await complete_command(partial)
command: str = ALIASES.get(items[0], items[0]) # resolve command aliases

Check warning on line 182 in cylc/flow/scripts/completion_server.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/scripts/completion_server.py#L182

Added line #L182 was not covered by tests

# special logic for the pseudo "help" and "version" commands
if command == 'help':
Expand Down Expand Up @@ -315,9 +313,9 @@
Note: This provides the long formats of options e.g. `--help` not `-h`.
"""
if command in COMMAND_OPTION_MAP:
return COMMAND_OPTION_MAP[command]

Check warning on line 316 in cylc/flow/scripts/completion_server.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/scripts/completion_server.py#L316

Added line #L316 was not covered by tests
try:
entry_point = COMMANDS[command].resolve()
entry_point = COMMANDS[command].load()

Check warning on line 318 in cylc/flow/scripts/completion_server.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/scripts/completion_server.py#L318

Added line #L318 was not covered by tests
except KeyError:
return []
parser = entry_point.parser_function()
Expand Down Expand Up @@ -637,15 +635,13 @@

# check that the installed completion script is compabile with this
# completion server version
for requirement in parse_requirements(REQUIRED_SCRIPT_VERSION):
# NOTE: there's only one requirement but we have to iterate to get it
if installed_version not in requirement:
is_compatible = False
print(
f'The Cylc {completion_lang} script needs to be updated to'
' work with this version of Cylc.',
file=sys.stderr,
)
if installed_version not in SpecifierSet(REQUIRED_SCRIPT_VERSION):
is_compatible = False
print(

Check warning on line 640 in cylc/flow/scripts/completion_server.py

View check run for this annotation

Codecov / codecov/patch

cylc/flow/scripts/completion_server.py#L639-L640

Added lines #L639 - L640 were not covered by tests
f'The Cylc {completion_lang} script needs to be updated to'
' work with this version of Cylc.',
file=sys.stderr,
)

# check for completion script updates
if installed_version < current_version:
Expand Down
Loading
Loading