-
Notifications
You must be signed in to change notification settings - Fork 33
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
Fix issue related to the installation of Python #236
base: develop
Are you sure you want to change the base?
Conversation
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.
Looks good! Thanks!
@cwlacewe @rolandoquesada is there any risk that this might cause problems for other programs relying on a predictable python version set in $PATH? |
The script file was updated to only "upgrade" the version of Python when the current version is lower than the minimum version needed Also some other script files were updated to call python3 only
Please let me know if you have any other suggestions. Thank you. |
@rolandoquesada I think my bigger concern is that updating Python has the potential to break existing dependencies if we're not careful, unless this is all being done within the docker container or a virtual environment. |
Target CPP Coverage: 63.8504% Target Python Coverage: 98.02% |
Target CPP Coverage: 63.8504% Target Python Coverage: 98.02% |
Hi Ian, thanks for the feedback, @cwlacewe as I was fixing an issue related to the condition for installing
Hi Ian, thanks for the feedback. If that's the case, I think I could remove the lines for updating the path to Python in /usr/bin. However, now I am wondering if we should keep the conditional that checks if we need to install a new version of Python (that condition was before my new changes). @cwlacewe I am wondering what you think about it? |
Target CPP Coverage: 63.8504% Target Python Coverage: 98.02% |
@rolandoquesada I think we should check if 3.12 exists, if so, make it the default python3 version; otherwise install and do the same. |
@cwlacewe I dont have a very concrete concern so much as my own experience that in bare-metal installs updating python can be challenging in unpredictable ways. Sometimes "just works" other times explodes. Seems to depend on vagaries of the host environment. My thought might be that we focus on the automation for container build, and either need a prompt or have just a note that "You need to install python 3.12 for host installation" in the docs. Ill totally admit virtual environments are a mystery to me. Thoughts? |
That would work. We can remove the python installation from script but check whether 3.12+ is installed. If it isn't installed, we can exit script with note advising to install it. If it is installed, we can use the newest version >= 3.12. How does that sound @ifadams @rolandoquesada? |
That works for me. I may be exercising an overabundance of caution, FWIW. |
That works for me as well, I am going to do it in that way. Thank you @ifadams @cwlacewe |
Target CPP Coverage: 64.1637% Target Python Coverage: 98.02% |
Target CPP Coverage: 64.1637% Target Python Coverage: 98.02% |
Target CPP Coverage: 64.1637% Target Python Coverage: 98.02% |
Target CPP Coverage: 64.1637% Target Python Coverage: 98.02% |
Target CPP Coverage: 64.1637% Target Python Coverage: 98.02% |
The current setup_vdms.sh script is not installing the new version of Python due to an issue related to when comparing the current version value to the requested version. Also it is needed to update some of the symbolic links to point to the new Python version directory