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

[wip] Migrate the project to pyproject.toml #422

Closed
wants to merge 6 commits into from

Conversation

inknos
Copy link
Contributor

@inknos inknos commented Aug 19, 2024

This is a WIP pr. before getting merged I expect a lot of changes and reviews. The automation, release process and gating will need to be tested as well before going.

Changes introduced by this PR:

  • package definition and dependencies are incorporated in pyproject.toml.
    package management is now done by hatch as suggested by the python docs.
    • building rpms will be handled via hatch
    • where not possible to use hatch (such as Centos Stream 8) it will use flit
    • things like testing requirements are now handled a bit differently. the testing packages are now specified in a dev environment which is now entirely managed by hatch.
      a big advantage of that is how the requirements are defined. now, it is sufficient to do hatch shell to get to the dev environment and have all the necessary tools in the virtual shell (the ones defined in test-requirements.txt).
      the environment where tests run is also better defined this way. with this PR, only the packages needed for testing are pulled. packages such as linting tools and pre-commit are not included in the testing env.
    • setup.py, setup.cfg, *requirements.txt are no longer needed.
  • pre-commit is now used to run black, isort, pylint. all these checks are incorporated in one tool: ruff. the single pre-commit hook will check linting and formatting in the same environment, limiting the opinionated issues that happen from here and there and alolowing to drop more dependencies that cause issues from time to time.
    • Consequently, cirrus does not require lint checks anymore since they are delegated to a separate check in pre-commit which is run in a gh workflow
  • no more tox. this hurts but the way tox was used is to run lint, black, and coverage steps. as noted above, lint and black are now handled specifically. now, tox is used to handle virtual environments only and to run coverage tests. this is perfectly implemented in hatch, so since we are switching the build backend why don't use it's builtin testing capabilities too? this also have some performances improvement, but I'd need to do some more research on that.
    • Packages that can be dropped
      • setuptools
      • sphinx (not really dropped but more like split in the [docs] package
      • wheel
      • black
      • pylint
      • tox
    • Packages that are added
      • hatch (build backend, replaces setuptools/wheel)
      • pre-commit (already used in many projects and pulled separately online)
      • ruff (pulled by pre-commit, replaces black, isort, pylint)
  • drop python 3.6 and 3.8 from UPSTREAM. reasons:

Important missing parts that I am still figuring out how to wire up:

  • Package RPM upstream
    • requests-mock is limited by a version that is not released in fedora. This forbids from using a dependency check in the specfile. At the present time, this issue has a workaround which removes the minimum requirements. It worked locally but let's see how it behaves on GH
  • Package RPM for CentOS 8

@inknos inknos force-pushed the hatchv2 branch 4 times, most recently from f27dd21 to 374be00 Compare August 19, 2024 13:21
@rhatdan rhatdan changed the title Migrate the project to pyproject.toml [wip] Migrate the project to pyproject.toml Aug 19, 2024
@inknos inknos force-pushed the hatchv2 branch 7 times, most recently from feadcc6 to 848e873 Compare August 20, 2024 14:04
@inknos inknos mentioned this pull request Aug 20, 2024
Copy link
Member

@jwhonce jwhonce left a comment

Choose a reason for hiding this comment

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

With this PR, I think we can also remove api/typing_extensions.py

pyproject.toml Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
Changes introduced by this commit:

Python Packaging
- package definition and dependencies are incorporated in pyproject.toml
- setup.py, setup.cfg, *requirements.txt are no longer needed
- packaging upstream is done via hatch
  see: https://packaging.python.org/en/latest/tutorials/packaging-projects/#choosing-build-backend

Dependencies
- black and pylint are no longer required
- tox is no longer required
- testing deps are no longer part of the dependencies
- docs are split in a separate package

Automation
- cirrus does not require lint checks anymore since they are delegated
  to a separate check in precommit
- pre-commit is used to check black, isort, pylint and it's all

Signed-off-by: Nicola Sella <[email protected]>
Signed-off-by: Nicola Sella <[email protected]>
Signed-off-by: Nicola Sella <[email protected]>
The file is not needed anymore since python<3.8 is dropped

Signed-off-by: Nicola Sella <[email protected]>
1. This commit introduces the [test] subpackage

2. It removes requests-mock <= 1.11 requirement

At the time of writing this commit requests-mock is packaged with the
following versions in Fedora and Epel.

Fedora 42	python-requests-mock-1.10.0-9.fc41
Fedora 41	python-requests-mock-1.10.0-9.fc41
Fedora 40	python-requests-mock-1.10.0-7.fc40
Fedora 39	python-requests-mock-1.10.0-5.fc39
Fedora EPEL 9	python-requests-mock-1.10.0-2.el9
Fedora EPEL 8	python-requests-mock-1.7.0-1.el8

3. It excludes the test subpackage and the check dependencies for test
   dependencies when run for distros which are not Fedora i.e. CentOS
Stream

Signed-off-by: Nicola Sella <[email protected]>
Copy link
Contributor

openshift-ci bot commented Nov 27, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: inknos

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@inknos
Copy link
Contributor Author

inknos commented Nov 28, 2024

Closing in favor of this issue which will track all the steps one by one. #474

Too many steps in a single PR

@inknos inknos closed this Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants