-
Notifications
You must be signed in to change notification settings - Fork 49
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
chore: bump python version #449
chore: bump python version #449
Conversation
Signed-off-by: Dushyant Behl <dushyantbehl@in.ibm.com>
Thanks for making a pull request! 😃 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the quick fix and testing Dushyant! Apologies for the lack of understanding but could you clarify what prevents us from keeping a minimum version of 3.9 for users who may want to use it?
@willmj Anything less than v3.12 has a FIPS security compliance issue which stems from underlying python version...if security is not a concern we can keep multiple versions one internal to the product where we go with the compliant 3.12+ python else we keep it open to 3.9 users like we had already. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, thanks! This change seems good to me for now, but let's wait for @anhuong's approval before merging.
.github/workflows/test.yaml
Outdated
- setup: "3.9" | ||
tox: "py39" | ||
- setup: "3.10" | ||
tox: "py310" | ||
- setup: "3.11" | ||
tox: "py311" | ||
- setup: "3.12" | ||
tox: "py312" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should leave all versions to test unit tests on, we can still support python 3.9-3.12
@@ -17,7 +17,7 @@ | |||
ARG BASE_UBI_IMAGE_TAG=latest | |||
ARG USER=tuning | |||
ARG USER_UID=1000 | |||
ARG PYTHON_VERSION=3.11 | |||
ARG PYTHON_VERSION=3.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even though our minimum version is set to 3.9, we have been building our image with 3.11. I don’t see an issue with moving this up to 3.12 except for the additional tests we should run to ensure everything works as expected. We can still have our minimum version be 3.9 but just has our automation running with 3.12. Currently our unit tests are the only thing that tests the suite of python version whereas our e2e tests focus solely on testing the image.
1d7d2a3
to
39dc6cf
Compare
Testing results on python 3.12 version:
for comparison, this is what we saw on python 3.11
|
Signed-off-by: Anh Uong <anh.uong@ibm.com>
Co-authored-by: Will Johnson <mwjohnson728@gmail.com> Signed-off-by: Anh Uong <anhuong4444@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating python to 3.12 for fips compliance, image and library will be built with python 3.12 but support will remain for python 3.9-3.12
chore: bump python version Signed-off-by: Angel Luu <angel.luu@us.ibm.com>
Thanks everyone for the quick turnaround on this! |
Description of the change
Related issue number
How to verify the PR
Was the PR tested