Convert to python module (pyproject.toml)

[VannTen]

possible followup to #658

[sesheta]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please assign harshad16 for approval by writing /assign @harshad16 in a comment. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/thoth-station/package-releases-job/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment

[VannTen]

On Wed, Jan 18, 2023 at 02:26:34PM -0800, Harshad Reddy Nalla wrote:

@harshad16 requested changes on this pull request.

On reviewing, its seems, i missed earlier that this component is not module. we don't need the project in pyproject.toml and can keep the version.py for this component. as it would be released as container image.

+[project] +name = "package-releases-job" +description = "Check packages updates on Python package indexes" +readme = "README.rst" +requires-python = ">=3.8" +license = {file = "LICENSE"} +dynamic = ["version"] + +[project.scripts] +package-releases-job = "producer:main" + +[build-system] +requires = ["setuptools", "wheel", "setuptools_scm"] + +[tool.setuptools_scm]

This might not be needed for this component, as this is not released as module, just as a container image on release.

I don't disagree ; I still think we could get some benefits from converting to pyproject.toml, mainly standardizing all of thoth repos on a common model so we can treat them the same in the CI.

[harshad16]

I don't disagree ; I still think we could get some benefits from converting to pyproject.toml, mainly standardizing all of thoth repos on a common model so we can treat them the same in the CI.

we are using pyproject.toml. is there a PEP standard for having a version on pyproject.toml, it seems project which use poetry use that? as this project is just a component, it feel version.py file is more easy and convenient. WDYT ?

[VannTen]

Actually we can have both if having version defined inside a file is desirable

setuptools support something like this in pyproject.toml

[project] dynamic = ["version"]

[tool.setuptools.dynamic] version = {attr: "version.version"}

The main point is to avoid repeating the version twice and then forgetting to update it twice later on.

Would that work ?

[harshad16]

Actually we can have both if having version defined inside a file is desirable setuptools support something like this in pyproject.toml [project] dynamic = ["version"] [tool.setuptools.dynamic] version = {attr: "version.version"} The main point is to avoid repeating the version twice and then forgetting to update it twice later on. Would that work ?

yes, we can do this. side question: what is the reason to have version in pyproject.toml in projects which are not python modules ?

[VannTen]

Well, handling them as python modules allows to handle them in a more general way. For example, using the repo as a pre-commit hook (like we're currently doing in adviser) rely on it being a python module.

Some of the tooling (mypy in particular) seems to be a bit lost when it can't identify a module.

Regarding scripts / job specifically, relying on [project.scripts] instead of manually invoking the python file guarantee we'll have the correct dependency in scope by just doing a pip install . of the repo.

All of the above is my opinion but TL;DR: easier integration with the Python ecosystem and it's tooling.

[harshad16]

Thank you for the explanation, i appreciate it.

For this PR, can we bring back the version.py and introduce the dynamic version in pyproject.toml

[VannTen]

Do you think we can have the __version__ directly in producer.py ? Having one single file for that seems a bit strange.

(Or maybe that's related to kebechet ?)

[harshad16]

Do you think we can have the __version__ directly in producer.py ? Having one single file for that seems a bit strange. (Or maybe that's related to kebechet ?)

yeah that is related to kebechet maybe can plan this future