From 84759b3b18c9d13724c95312d9284630b7bd9c6c Mon Sep 17 00:00:00 2001 From: Douglas Cerna Date: Mon, 30 Oct 2023 08:56:37 -0600 Subject: [PATCH] Drop unsupported Python versions * Update package setup * Update CI configuration * Address test issues * Mock timezone in test_create_dip_success --- .github/workflows/test.yml | 49 +++++++------------ .pre-commit-config.yaml | 19 +++---- aips/create_dip.py | 3 +- aips/create_dips_job.py | 3 +- dips/copy_to_netx.py | 1 - .../local.txt => requirements-dev.txt | 2 +- requirements.txt | 6 ++- requirements/base.txt | 5 -- requirements/production.txt | 2 - tests/test_aips_models.py | 2 +- tests/test_create_dip.py | 1 + tests/test_reingest.py | 1 - tests/test_transfer_model.py | 2 +- tox.ini | 33 +++++++++---- transfers/examples/split_transfer.py | 4 +- transfers/reingest.py | 14 +++--- transfers/transfer.py | 2 +- 17 files changed, 72 insertions(+), 77 deletions(-) rename requirements/local.txt => requirements-dev.txt (69%) delete mode 100644 requirements/base.txt delete mode 100644 requirements/production.txt diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9c9630a3..92f7a48d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -7,44 +7,33 @@ on: - "master" jobs: tox: - name: "Test ${{ matrix.toxenv }}" - runs-on: "ubuntu-20.04" + name: "Test Python ${{ matrix.python-version }}" + runs-on: "ubuntu-22.04" strategy: matrix: - include: - - python-version: "3.6" - toxenv: "py36" - - python-version: "3.7" - toxenv: "py37" - - python-version: "3.8" - toxenv: "py38" - - python-version: "3.9" - toxenv: "py39" + python-version: [ + "3.8", + "3.9", + "3.10", + "3.11", + "3.12", + ] steps: - name: "Check out repository" - uses: "actions/checkout@v3" + uses: "actions/checkout@v4" - name: "Set up Python ${{ matrix.python-version }}" uses: "actions/setup-python@v4" with: python-version: "${{ matrix.python-version }}" - - name: "Get pip cache dir" - id: "pip-cache" - run: | - echo "dir=$(pip cache dir)" >> $GITHUB_OUTPUT - - name: "Cache pip packages" - uses: "actions/cache@v3" - with: - path: "${{ steps.pip-cache.outputs.dir }}" - key: "${{ runner.os }}-pip-${{ hashFiles('**/base.txt', '**/local.txt', '**/production.txt') }}" - restore-keys: | - ${{ runner.os }}-pip- + cache: "pip" + cache-dependency-path: | + requirements.txt + requirements-dev.txt - name: "Install tox" run: | python -m pip install --upgrade pip - pip install tox + pip install tox tox-gh-actions - name: "Run tox" - env: - TOXENV: ${{ matrix.toxenv }} run: | tox -- --cov --cov-config .coveragerc --cov-report xml:coverage.xml - name: "Upload coverage report" @@ -52,20 +41,18 @@ jobs: uses: "codecov/codecov-action@v3" with: files: ./coverage.xml - fail_ci_if_error: true + fail_ci_if_error: false verbose: true - name: ${{ matrix.toxenv }} - flags: ${{ matrix.toxenv }} lint: name: "Lint" runs-on: "ubuntu-22.04" steps: - name: "Check out repository" - uses: "actions/checkout@v3" + uses: "actions/checkout@v4" - name: "Set up Python" uses: "actions/setup-python@v4" with: - python-version: "3.8" + python-version: "3.12" - name: "Install tox" run: | python -m pip install --upgrade pip diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index db6b5718..57f867c4 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,22 +1,23 @@ repos: - repo: https://github.com/asottile/pyupgrade - rev: v2.31.0 + rev: v3.10.1 hooks: - id: pyupgrade - args: [--py3-plus, --py36-plus] + args: [--py38-plus] - repo: https://github.com/asottile/reorder_python_imports - rev: v2.6.0 + rev: v3.10.0 hooks: - id: reorder-python-imports - args: [--py3-plus, --py36-plus] -- repo: https://github.com/ambv/black - rev: 22.8.0 + args: [--py38-plus] +- repo: https://github.com/psf/black + rev: "23.7.0" hooks: - id: black args: [--safe, --quiet] - language_version: python3 - repo: https://github.com/pycqa/flake8 - rev: 5.0.4 + rev: "6.1.0" hooks: - id: flake8 - language_version: python3 + additional_dependencies: + - flake8-bugbear==23.9.16 + - flake8-comprehensions==3.14.0 diff --git a/aips/create_dip.py b/aips/create_dip.py index 0456d833..92f578c6 100644 --- a/aips/create_dip.py +++ b/aips/create_dip.py @@ -444,7 +444,6 @@ def get_original_relpath(original_name): if __name__ == "__main__": - parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter ) @@ -544,7 +543,7 @@ def get_original_relpath(original_name): # The main function returns the DIP's path on success # or an int higher than 0 if it fails. The scrip will # always exit with an int, 0 on success. - if type(ret) != int: + if not isinstance(ret, int): ret = 0 sys.exit(ret) diff --git a/aips/create_dips_job.py b/aips/create_dips_job.py index cff54257..fe56881f 100644 --- a/aips/create_dips_job.py +++ b/aips/create_dips_job.py @@ -133,7 +133,7 @@ def main( ) # Do not try upload on creation error - if type(dip_path) == int: + if isinstance(dip_path, int): LOGGER.error("Could not create DIP from AIP: %s", uuid) continue @@ -200,7 +200,6 @@ def filter_aips(aips, location_uuid, origin_pipeline_uuid): if __name__ == "__main__": - parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter ) diff --git a/dips/copy_to_netx.py b/dips/copy_to_netx.py index 29f4545f..30837ea6 100755 --- a/dips/copy_to_netx.py +++ b/dips/copy_to_netx.py @@ -231,7 +231,6 @@ def main( if __name__ == "__main__": - parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter ) diff --git a/requirements/local.txt b/requirements-dev.txt similarity index 69% rename from requirements/local.txt rename to requirements-dev.txt index cae2dce0..c391c3fe 100644 --- a/requirements/local.txt +++ b/requirements-dev.txt @@ -1,4 +1,4 @@ --r base.txt +-r requirements.txt pytest==7.0.1 pytest-cov==4.0.0 vcrpy==4.1.1 diff --git a/requirements.txt b/requirements.txt index ea77c2d8..751e34c0 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1 +1,5 @@ --r requirements/production.txt +amclient==1.2.3 +metsrw==0.4.0 +requests==2.31.0 +sqlalchemy==1.4.49 +urllib3==1.26.18 diff --git a/requirements/base.txt b/requirements/base.txt deleted file mode 100644 index c1802651..00000000 --- a/requirements/base.txt +++ /dev/null @@ -1,5 +0,0 @@ -amclient==1.2.3 -metsrw==0.3.23 -requests==2.27.1 -sqlalchemy==1.4.48 -urllib3==1.26.17 diff --git a/requirements/production.txt b/requirements/production.txt deleted file mode 100644 index 42b2763f..00000000 --- a/requirements/production.txt +++ /dev/null @@ -1,2 +0,0 @@ -# There should be no dependency in production that isn't in development. --r base.txt diff --git a/tests/test_aips_models.py b/tests/test_aips_models.py index 4669196d..3b6f8026 100644 --- a/tests/test_aips_models.py +++ b/tests/test_aips_models.py @@ -20,7 +20,7 @@ def test_init_success(self): assert os.path.isfile(DATABASE_FILE) assert "aip" in models.Base.metadata.tables assert hasattr(session, "add") - assert callable(getattr(session, "add")) + assert callable(session.add) def test_init_fail(self): """Test that the database can't be created in a wrong path.""" diff --git a/tests/test_create_dip.py b/tests/test_create_dip.py index 5c6e3655..6219ed39 100644 --- a/tests/test_create_dip.py +++ b/tests/test_create_dip.py @@ -50,6 +50,7 @@ def test_extract_aip_fail(self): @vcr.use_cassette( "fixtures/vcr_cassettes/test_create_dip_download_aip_success.yaml" ) + @unittest.mock.patch.dict(os.environ, {"TZ": "UTC"}) def test_create_dip_success(self): """ Test full DIP creation: diff --git a/tests/test_reingest.py b/tests/test_reingest.py index 1fb7e184..122ddf51 100644 --- a/tests/test_reingest.py +++ b/tests/test_reingest.py @@ -8,7 +8,6 @@ class TestReingestClass: - dbpath = "fixtures/reingest_test.db" @pytest.fixture(autouse=True) diff --git a/tests/test_transfer_model.py b/tests/test_transfer_model.py index 6e65f046..0e3de115 100644 --- a/tests/test_transfer_model.py +++ b/tests/test_transfer_model.py @@ -20,7 +20,7 @@ def setup_session(): try: models.transfer_session.query(models.Unit).one() except MultipleResultsFound: - assert False + raise AssertionError() except NoResultFound: assert True diff --git a/tox.ini b/tox.ini index f4908b93..c66e0119 100644 --- a/tox.ini +++ b/tox.ini @@ -1,9 +1,17 @@ [tox] -envlist = py{36,37,38,39}, linting +envlist = py{38,39,310,311,312}, linting skipsdist = True +[gh-actions] +python = + 3.8: py38 + 3.9: py39 + 3.10: py310 + 3.11: py311 + 3.12: py312 + [testenv] -deps = -rrequirements/local.txt +deps = -rrequirements-dev.txt skip_install = True commands = pytest {posargs} @@ -14,12 +22,17 @@ commands = pre-commit run --all-files --show-diff-on-failure [flake8] exclude = .git, .tox, __pycache__, old, build, dist -application-import-names = flake8 -select = C, E, F, W, B, B950 +# Error codes: +# - https://flake8.pycqa.org/en/latest/user/error-codes.html +# - https://pycodestyle.pycqa.org/en/latest/intro.html#error-codes +# - https://github.com/PyCQA/flake8-bugbear#list-of-warnings +# +# E203: whitespace before ‘,’, ‘;’, or ‘:’ +# E402: module level import not at top of file +# E501: line too long +# W503: line break before binary operator ignore = - E203 # Whitespace before `:` - E501 # Lines are too long - E402 # Module level imports not at top of file - W503 # Line break before binary operator -import-order-style = pep8 - + E203, + E402, + E501, + W503 diff --git a/transfers/examples/split_transfer.py b/transfers/examples/split_transfer.py index 40995b19..01d68750 100755 --- a/transfers/examples/split_transfer.py +++ b/transfers/examples/split_transfer.py @@ -27,7 +27,7 @@ def __init__(self, source_sip, csv_delimiter): self.index_csv() def index_csv(self): - self.index = dict() + self.index = {} with open(self.csv_file) as csvf: csvr = csv.reader(csvf, delimiter=self.csv_delimiter) for i, row in enumerate(csvr): @@ -84,7 +84,7 @@ def main(source_sip, target_dir, csv_delimiter, prefix=None, metadata_only=False if len(objects_dirs) < 1: print(f"Object directory is empty: {objects_dir}") - for i, item in enumerate(objects_dirs): + for item in objects_dirs: print(f"- {item}") src = os.path.join(objects_dir, item, "") diff --git a/transfers/reingest.py b/transfers/reingest.py index 891e50fd..7bcb5df7 100644 --- a/transfers/reingest.py +++ b/transfers/reingest.py @@ -61,13 +61,13 @@ def setup_amclient(amclient): this block. The `setattr(...)` calls in this block are ordered alphabetically. """ - setattr(amclient, "aip_uuid", None) - setattr(amclient, "package_uuid", None) - setattr(amclient, "pipeline_uuid", None) - setattr(amclient, "processing_config", None) - setattr(amclient, "sip_uuid", None) - setattr(amclient, "transfer_directory", None) - setattr(amclient, "transfer_uuid", None) + amclient.aip_uuid = None + amclient.package_uuid = None + amclient.pipeline_uuid = None + amclient.processing_config = None + amclient.sip_uuid = None + amclient.transfer_directory = None + amclient.transfer_uuid = None return amclient diff --git a/transfers/transfer.py b/transfers/transfer.py index de7d31d9..a5fc2fa1 100755 --- a/transfers/transfer.py +++ b/transfers/transfer.py @@ -330,7 +330,7 @@ def get_next_transfer( LOGGER.debug("New transfer candidates: %s", entries) LOGGER.info("Unprocessed entries to choose from: %s", len(entries)) # Sort, take the first - entries = sorted(list(entries)) + entries = sorted(entries) if not entries: LOGGER.info("All potential transfers in %s have been created.", path_prefix) return None