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

[IMP] use python3.8 for pre-commit-vauxoo #204

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

antonag32
Copy link
Contributor

@antonag32 antonag32 commented Aug 4, 2023

Closes #203.

pre-commit-vauxoo requires Python 3.7 or greater to run. Some containers
in Odoo 12 and below are using Python 3.6, as a workaround, if the
Python version detected is not suitable, Python 3.8 will be installed
and used for running pre-commit-vauxoo.

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (72c2c18) 81.98% compared to head (93ed00a) 81.98%.
Report is 2 commits behind head on main.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #204   +/-   ##
=======================================
  Coverage   81.98%   81.98%           
=======================================
  Files           7        7           
  Lines         594      594           
  Branches      111      111           
=======================================
  Hits          487      487           
  Misses         77       77           
  Partials       30       30           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@antonag32
Copy link
Contributor Author

@moylop260 @luisg123v

Can you review? I did not understand the comment about having to create a new image, I think we can simply install python3.8 and it can coexist with the default interpreter.

I tested this on absa:

[odoo@cd314ba669fb]~/instance/extra_addons/absa$
 (12.0)$ python3 --version
Python 3.6.9
[odoo@cd314ba669fb]~/instance/extra_addons/absa$
 (12.0)$ which pre-commit-vauxoo
/usr/local/bin/pre-commit-vauxoo
[odoo@cd314ba669fb]~/instance/extra_addons/absa$
 (12.0)$ cat /usr/local/bin/pre-commit-vauxoo
#!/usr/bin/python3.8
# -*- coding: utf-8 -*-
import re
import sys
from pre_commit_vauxoo.cli import main
if __name__ == '__main__':
    sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])
    sys.exit(main())

@moylop260
Copy link
Collaborator

We have 2 matters to fix here

  1. The CI runs pre-commit-vauxoo from quay.io/vauxoo/dockerv docker image using py3.8
  2. t2d runs pre-commit-vauxoo from the quay.io/vauxoo/{customer-image} using the python version installed in the ubuntu version, we have configured manually this value from python3.X configured from variables.sh

So, currently, running the CI it could have different results since it is using py3.8 but the project itself is developed from another py version

So, IMHO the lints from the CI should run from the same python version of the image

e.g. for newer py version the lint could raises use current_threading but for old ones use currentThreading so it depends on the environment where it will be ran but currently all the CI results run only for py3.8 so it could raises lint errors not compatible with py real version used

I don't know if I was clear

@moylop260
Copy link
Collaborator

I mean, we need to fix the point 1) instead of force old py version for 2)

@antonag32
Copy link
Contributor Author

Makes sense. It seems like the script should only install python3.8 when the interpreter is lower than that, otherwise just use the default one. Do you agree @moylop260 @luisg123v

@antonag32 antonag32 force-pushed the pcv-in3.8-anton branch 2 times, most recently from 0b8c661 to 93ed00a Compare August 22, 2023 17:15
Closes Vauxoo#203.

pre-commit-vauxoo requires Python 3.7 or greater to run. Some containers
in Odoo 12 and below are using Python 3.6, as a workaround, if the
Python version detected is not suitable, Python 3.8 will be installed
and used for running pre-commit-vauxoo.
@antonag32
Copy link
Contributor Author

I mean, we need to fix the point 1) instead of force old py version for 2)

I updated the PR, it will only install it if the Python version is lower than 3.7, this is thinking on Odoo 12 containers (runs on Python 3.6). But re-reading your comment, does this mean each PM should just change their variables.sh and use a supported version?

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.

Run pre-commit-vauxoo in python 3.8
3 participants