-
Notifications
You must be signed in to change notification settings - Fork 441
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
Upgrade Python version to 3.10 #2057
Upgrade Python version to 3.10 #2057
Conversation
blocked by #2058 |
801e8f9
to
58d63a8
Compare
@@ -1,4 +1,4 @@ | |||
psutil==5.8.0 | |||
psutil==5.9.4 |
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.
To resolve the following error, we must upgrade the psutil version.
...
#0 98.35 C compiler or Python headers are not installed on this system. Try to run:
#0 98.35 sudo apt-get install gcc python3-dev
#0 98.35 error: command 'gcc' failed: No such file or directory
#0 98.35 [end of output]
#0 98.35
#0 98.35 note: This error originates from a subprocess, and is likely not a problem with pip.
#0 98.36 error: legacy-install-failure
#0 98.36
#0 98.36 × Encountered error while trying to install package.
#0 98.36 ╰─> psutil
...
if sys.version_info <= (3, 9): | ||
from pkg.suggestion.v1beta1.chocolate.service import ChocolateService | ||
|
||
import utils | ||
|
||
|
||
@unittest.skipIf(sys.version_info >= (3, 10), reason="Chocolate supports only Python 3.9 or lower") |
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.
We can not work chocolate on Python 3.10.
ref: #2058
cff7b18
to
ff6f913
Compare
if [ "${TARGETARCH}" = "ppc64le" ] || [ "${TARGETARCH}" = "arm64" ]; then \ | ||
apt-get -y install gfortran libopenblas-dev liblapack-dev g++; \ | ||
fi && \ |
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.
To avoid the following error, we must install these packages for all platforms.
...
#0 73.28 gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/usr/local/include/python3.9 -c src/Halton.cpp -o build/temp.linux-x86_64-3.9/src/Halton.o
#0 73.28 error: command 'gcc' failed: No such file or directory
#0 73.28 [end of output]
#0 73.28
#0 73.28 note: This error originates from a subprocess, and is likely not a problem with pip.
#0 73.29 error: legacy-install-failure
#0 73.29
#0 73.29 × Encountered error while trying to install package.
#0 73.29 ╰─> ghalton
...
if sys.version_info < (3, 10): | ||
from pkg.suggestion.v1beta1.chocolate.service import ChocolateService | ||
|
||
import utils | ||
|
||
|
||
@unittest.skipIf(sys.version_info >= (3, 10), reason="Chocolate supports only Python 3.9 or lower") |
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.
We can not work chocolate on Python 3.10.
ref: #2058
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.
Are we removing chocolate in this release itself? Else, removing tests will lead to untested releases
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.
@johnugeorge
IIUC, we can run tests for chocolate in CI since I added Python 3.9 to CI so that we can verify the chocolate suggestion service as shown below:
katib/.github/workflows/test-python.yaml
Lines 29 to 33 in ff6f913
# TODO (tenzen-y): Remove tests for Python 3.9 once we remove the Chocolate Suggestion Service from Katib. | |
strategy: | |
fail-fast: false | |
matrix: | |
python-version: ['3.9', '3.10'] |
Although, I'm ok with removing chocolate in this release.
wdyt?
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 removing chocolate is better, since there is no much reason to support it as you mentioned at #2058
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 agree, we should remove Chocolate first.
We want to reduce number of actions that we run since we have 20 concurrent GitHub Actions jobs limitation.
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.
Ok.
First, I will remove Chocolate Suggestion Service in another PR.
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
looks good to me!
LGTM /assign @andreyvelich |
Thank you for updating the version @tenzen-y! |
It might be better to use our Python SDK in our E2E, as you mentioned in #2024, and use multiple Python versions. |
blocked by #2064 |
I agree with you too @tenzen-y . SDK should care about backward compatibility of python version. And notice to users at release note if supported python version will be changed. |
ff6f913
to
473c633
Compare
Signed-off-by: tenzen-y <yuki.iwai.tz@gmail.com> Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
473c633
to
83afcfc
Compare
@andreyvelich @johnugeorge @anencore94 I have rebased this. PTAL. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: johnugeorge, tenzen-y 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:
Approvers can indicate their approval by writing |
/lgtm |
Signed-off-by: tenzen-y yuki.iwai.tz@gmail.com
What this PR does / why we need it:
I upgraded Python version to 3.10 since Python 3.9 no longer fixes bugs, according to this.
Also, I upgraded pytest version to avoid the error,
E TypeError: required field "lineno" missing from alias
.ref: pytest-dev/pytest#8540
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close the issue(s) when PR gets merged):Fixes #
Checklist: