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

Drop Python < 3.8 and replace pkg_resources with importlib #37

Merged
merged 3 commits into from
Dec 9, 2024

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Sep 12, 2023

No description provided.

Copy link
Member

@evgeni evgeni left a comment

Choose a reason for hiding this comment

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

This would kill Python 2 compatibility, no?

@ehelms ehelms requested a review from evgeni September 12, 2023 14:15
@ekohl
Copy link
Member Author

ekohl commented Sep 12, 2023

Version 3.3.1 still had Python 2.7 support: https://pypi.org/project/importlib-resources/3.3.1/

@evgeni
Copy link
Member

evgeni commented Sep 13, 2023

Version 3.3.1 still had Python 2.7 support: https://pypi.org/project/importlib-resources/3.3.1/

let's find out by fixing CI on 2.7: #38

@ekohl ekohl force-pushed the replace-pkg_resources branch from 3a810e3 to c87d44d Compare September 13, 2023 09:03
@evgeni
Copy link
Member

evgeni commented Sep 13, 2023

Thanks! Looks good then.

One oddity I still can offer. https://github.com/theforeman/jenkins-jobs/blob/master/theforeman.org/pipelines/lib/obal.groovy does not pip install anything, but just checks out obsah and obal and mangles PYTHONPATH. That means it'll break with this change unless we somehow ensure importlib things are installed.

(pip install probably won't do as we don't use a venv right now? need to investigate)

setup.py Outdated
Comment on lines 71 to 72
'importlib_resources; python_version < "3.7"',
'importlib_metadata; python_version < "3.8"',
Copy link
Member

Choose a reason for hiding this comment

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

given on EL8 we use py3.11 now, this is safe to merge once we turn of the el7 builders.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we then refactor this in a "drop Python < 3.8 compatibility" PR?

Copy link
Member

Choose a reason for hiding this comment

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

I think downstream it's still py3.6 (with whatever ansible is compatible with that)

Copy link
Member Author

Choose a reason for hiding this comment

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

Do we still need to care about this or can we assume Python 3.8+ by now?

@ekohl ekohl force-pushed the replace-pkg_resources branch from c87d44d to 9b23b91 Compare October 2, 2023 09:51
@ekohl ekohl changed the title Replace pkg_resources with importlib Drop Python < 3.8 and replace pkg_resources with importlib Oct 2, 2023
@ekohl ekohl force-pushed the replace-pkg_resources branch from 9b23b91 to 71160af Compare October 2, 2023 09:54
@ekohl ekohl force-pushed the replace-pkg_resources branch from 71160af to f2f8933 Compare December 6, 2024 18:03
@ekohl ekohl force-pushed the replace-pkg_resources branch from f2f8933 to c13603d Compare December 6, 2024 19:41
@ekohl ekohl merged commit c13603d into theforeman:master Dec 9, 2024
6 checks passed
@ekohl ekohl deleted the replace-pkg_resources branch December 9, 2024 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants