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

Make openscap-report compatible with Python 3.6 #226

Merged
merged 9 commits into from
Apr 23, 2024
Merged

Conversation

jan-cerny
Copy link
Member

OpenSCAP Report currently supports Python 3.8 and newer. However, the default version in RHEL 8 is Python 3.6. This PR introduces changes necessary to make our code compatible with Python 3.6.

The most important changes are:

  • using bundled dataclasses if not available
  • do not use contextlib.nullcontext
  • do not use importlib.metadata

Python 3.6, which is the default Python in RHEL 8, doesn't provide
the `dataclasses` module, this module has been introduced in Python
3.8. To make the class work, we will bundle this module.

Imported from: https://github.com/ericvsmith/dataclasses/blob/master/dataclasses.py
@pep8speaks
Copy link

pep8speaks commented Apr 22, 2024

Hello @jan-cerny! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-04-22 15:51:47 UTC

openscap_report/dataclasses.py Fixed Show fixed Hide fixed
openscap_report/dataclasses.py Fixed Show fixed Hide fixed
This commit introduces changes necessary to make our code
compatbile with Python 3.6, the default Python version
shipped in RHEL 8.

The most important changes are:
- using bundled dataclasses if not available
- do not use contextlib.nullcontext
- do not use importlib.metadata
This change should make the test environment similar to RHEL 8
where the Python 3.6 is used as default.
Build with the default Python 3 (Python 3.6).
Builiding with Python 3.8 is possible in EPEL but not for CS 8
where the default module must be used.
Copy link
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

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

It looks good but I have a few comments.

Copy link
Member

Choose a reason for hiding this comment

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

I recommend placing this file in a new dataclasses directory with the file __init__.py, where the import of internal dataclasses or those provided by Python will be processed. Then you will be able to simplify imports in other files and reduce code duplication.

Please try to solve the Linters problem. I recommend formatting the code with black and sorting the imports with isort.

This helps simplify the import statements in individual files.
@jan-cerny jan-cerny marked this pull request as ready for review April 22, 2024 15:05
Copy link
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

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

I checked the liners via tox test suite and the pylint has some issues. I recommend setting ignore for dataclasses.py file and fix other issues.

Copy link
Member

@Honny1 Honny1 left a comment

Choose a reason for hiding this comment

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

LGTM

@Honny1 Honny1 merged commit f176ea8 into OpenSCAP:main Apr 23, 2024
19 checks passed
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.

None yet

3 participants