-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[blocker] CLOUDSTACK-9452: use optparse instead of argparse in patchviasocket #1633
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
Conversation
Use optparse that is supported by python 2.3+, instead of argparse that is supported by python 2.7+. On KVM hosts which don't have python-argparse pkg installed, the patchviasocket script will break. This fixes so that patchviasocket would work on CentOS6 KVM hosts, or KVM hosts without python 2.7/argparse in general. The patchviasocket script was rewritten as a Python script from PR apache#1533 Signed-off-by: Rohit Yadav <rohit.yadav@shapeblue.com>
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
@karuturi @jburwell @wido @sverrirab please review, it's a blocker for kvm centos6 hosts without argparse/python2.7 installed. I could run an existing test: (gave same results before and after the change) ..Ran 6 tests in 1.633s OK |
|
Do we seriously want to write code that still works on CentOS 6 or systems without Python 2.7? Looking at the Python website: https://docs.python.org/2/library/optparse.html Deprecated since version 2.7: The optparse module is deprecated and will not be developed further; development will continue with the argparse module. I wouldn't call this a blocker at all. CentOS 6 is pre-historic and we can't go back to using a old module like OptParse. Sorry, -1 for me on this one. |
|
@wido Yes, we want to write code that still works on CentOS6 and systems without Python 2.7 as long as we support them. It is a valid
|
|
The updated script that now depends on python 2.7 (or python 2.6 + argparse I assume) is replacing the perl installation requirement that was there before. Would it not be the correct solution to require python 2.7 to be installed? If the hosts are dedicated KVM hosts that should be relatively low risk? |
|
The reason for my suggestion is that I don't think anyone should be running python 2.6 in production. The last update for it was in 2008 so I would consider it risky at this point. |
|
CentOS6 default/epel repos don't have python 2.7 :) There are large installations running old code, old hardware, old kernels/OSs; they will get upgraded over time, but they need to be supported for the time being. Lastly, I could make the same argument for our usage of Java6/7, why aren't we running Java8. Java7 has eoled, but most of us I know are still using it in production. |
|
Packaging result: ✖centos6 ✔centos7 ✔debian repo: http://packages.shapeblue.com/cloudstack/pr/1633 |
|
@wido @sverrirab will adding python-argparse as a dependency be an acceptable solution to the problem? this package is available on el6, but python 2.7 is not available (without using one of the 3rd party repositories). |
|
adding the python-argparse dependency is fine by me. I have not tested that combination though. |
|
@rhtyd I'm fine with adding it as a dependency. Perception is different here I think. My personal opinion is that nobody should be running CentOS 6 with CloudStack in production just as much as you shouldn't use Ubuntu 12.04 anymore for CloudStack. I'm against writing code which brings in legacy instead of removing it. |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian repo: http://packages.shapeblue.com/cloudstack/pr/1633 |
Use optparse that is supported by python 2.3+, instead of argparse that is
supported by python 2.7+. On KVM hosts which don't have python-argparse pkg
installed, the patchviasocket script will break. This fixes so that patchviasocket
would work on CentOS6 KVM hosts, or KVM hosts without python 2.7/argparse in
general.
The patchviasocket script was rewritten as a Python script from PR #1533
@blueorangutan package