Use modern tomllib/tomli modules for reading TOML files

[mgorny]

Replace the unmaintained toml/pytoml dependencies with the modern alternatives: the built-in tomllib module in Python 3.11, and tomli in older Python versions. Preserving backwards compatibility does not seem necessary, as podman-py no longer supports Python versions older than 3.6.

Signed-off-by: Michał Górny mgorny@gentoo.org

[mgorny]

I don't think that pylint failure is my fault.

[jwhonce]

@cdoern @lsm5 Please verify those libraries are available across all supported OS version as packages.

[jwhonce]

@mgorny I would guess the linter has been updated on the image a caught this issue.

@cdoern FYI I was not able to reproduce the pylint issue with the following env:

pylint --version
pylint 2.14.4
astroid 2.12.10
Python 3.11.0rc2 (main, Sep 13 2022, 00:00:00) [GCC 12.2.1 20220819 (Red Hat 12.2.1-1)]

[cdoern]

@mgorny would you mind re pushing? also try a rebase just in case. If it fails again, I suggest following the error message and moving the podman import up

[mgorny]

@mgorny would you mind re pushing? also try a rebase just in case. If it fails again, I suggest following the error message and moving the podman import up

Re-pushed. There was nothing to rebase on, i.e. it was already at the latest.

[mgorny]

@mgorny I would guess the linter has been updated on the image a caught this issue.

@cdoern FYI I was not able to reproduce the pylint issue with the following env:

pylint --version
pylint 2.14.4
astroid 2.12.10
Python 3.11.0rc2 (main, Sep 13 2022, 00:00:00) [GCC 12.2.1 20220819 (Red Hat 12.2.1-1)]

I actually see it with tox -e pylint. It's using pylint==2.15.4 apparently.

[mgorny]

Moving the import fixes one error but leaves the two others:

************* Module /tmp/podman-py/.pylintrc
.pylintrc:1:0: E0015: Unrecognized option found: no-space-check, argument-name-hint, variable-name-hint (unrecognized-option)
************* Module podman.api.__init__
podman/api/__init__.py:1:0: R0401: Cyclic import (podman.api -> podman.api.client) (cyclic-import)

[lsm5]

I can't find any tomli packages for rhel 8 and 9 so I suspect this would break those distros. Can we please keep toml as a fallback if tomli isn't available?

[mgorny]

I can't find any tomli packages for rhel 8 and 9 so I suspect this would break those distros. Can we please keep toml as a fallback if tomli isn't available?

I suppose I can make the code compatible with it but I don't see how to make Python-level requires happy with either package. That said, I don't understand why it's listed as build system dependency in the first place.

[mgorny]

I can't find any tomli packages for rhel 8 and 9 so I suspect this would break those distros. Can we please keep toml as a fallback if tomli isn't available?

I suppose I can make the code compatible with it but I don't see how to make Python-level requires happy with either package. That said, I don't understand why it's listed as build system dependency in the first place.

Done that.

[lsm5]

let me try building this on copr for centos before we merge.

[lsm5]

So, if epel repos aren't enabled, centos8 will have trouble finding python3-toml and centos9 will have trouble finding python3-tomli. This would mean rhel8 and rhel9 builds would also have trouble since we can't enable epel repos there. See: https://copr.fedorainfracloud.org/coprs/rhcontainerbot/podman-next/build/4954089/ for details.

@cdoern @umohnani8 how do you wanna go forward with this? Are we ok with having a separate rhel branch or should we make sure main branch itself builds for all rhel8 and rhel9 ?

[lsm5]

@cdoern @umohnani8 PTAL.

[rhatdan]

What would it take to get tomllib/tomli into RHEL? Is this something we can just make available for podman-py? How do we handle this with other python tools?

[lsm5]

What would it take to get tomllib/tomli into RHEL?

I guess the typical process to get a package included in RHEL.

Is this something we can just make available for podman-py?

You mean like bundling tomllib in the podman-py RPM for rhel? No idea how ugly that might get.

How do we handle this with other python tools?

I guess other deps have been available on RHEL so far. Don't quite remember bundling a python dependency in another python-based RPM

[rhatdan]

Bottom line, I don't believe we should block Upstream based on Downstream packaging. We can work to get this packaged in RHEL in the future, but by the same token, we should be careful not to bring in huge libraries, which add little value.

[lsm5]

Bottom line, I don't believe we should block Upstream based on Downstream packaging. We can work to get this packaged in RHEL in the future, but by the same token, we should be careful not to bring in huge libraries, which add little value.

Since we're keeping podman-py versions in sync with podman, RHEL packaging would have to be dealt with rather sooner. If we don't wanna block upstream on RHEL packaging, then having a separate rhel branch seems to be the way to go.

[umohnani8]

How much effort would it be to maintain a separate rhel branch on github? We would have to make sure we push any new commits to both the main and rhel branch right?
I am fine with a separate rhel branch as long as there is an easy way to maintain it. One more thing to consider is we may run into dep issues with new code that works fine with the modern tomlib/tomli modules and not the old ones we are using for rhel, how would we handle that then?

[rhatdan]

I think is it a mistake to branch them. We are doubling our work rather then just getting the new libraries into RHEL.

[lsm5]

I think is it a mistake to branch them. We are doubling our work rather then just getting the new libraries into RHEL.

I'm fine with that. I guess we can continue RHEL discussion elsewhere (cc: @TomSweeneyRedHat @jnovy)

[lsm5]

Builds fine on Fedora and CentOS Stream. So LGTM from me.

@cdoern @umohnani8 PTAL for final review and merge.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lsm5, mgorny

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:

• OWNERS [lsm5]

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

[rhatdan]

@mgorny needs a rebase.

[mgorny]

Rebased.

[umohnani8]

@mgorny there is still a conflict, can you please rebase again.

[mgorny]

Rebased again.

[rhatdan]

LGTM

[mgorny]

Thanks!