[puppet-openstack] stop using the ${pyvers} variable
Hi, Using the ${pyvers} so we can switch be between Python versions made sense 1 or 2 years ago. However, I'm in the opinion that we should stop using that, and switch to using python3 everywhere directly whenever possible. Though Tobias seems to not agree (see [1]), so I'm raising the topic in the list so we can discuss this together. Your thoughts? Cheers, Thomas Goirand (zigo) [1] https://review.opendev.org/c/openstack/puppet-swift/+/777564
Hi Zigo, On Sat, Feb 27, 2021 at 10:24 AM Thomas Goirand <zigo@debian.org> wrote:
Using the ${pyvers} so we can switch be between Python versions made sense 1 or 2 years ago. However, I'm in the opinion that we should stop using that, and switch to using python3 everywhere directly whenever possible. Though Tobias seems to not agree (see [1]), so I'm raising the topic in the list so we can discuss this together.
[1] https://review.opendev.org/c/openstack/puppet-swift/+/777564
I had a quick look and I believe Tobias is not against not using that var but the fact that you renamed the resource from python-ceilometermiddleware to ceilometermiddleware which does not make sense indeed and could be breaking. Kind regards, -yoctozepto
On 2/27/21 11:28 AM, Radosław Piliszek wrote:
Hi Zigo,
On Sat, Feb 27, 2021 at 10:24 AM Thomas Goirand <zigo@debian.org> wrote:
Using the ${pyvers} so we can switch be between Python versions made sense 1 or 2 years ago. However, I'm in the opinion that we should stop using that, and switch to using python3 everywhere directly whenever possible. Though Tobias seems to not agree (see [1]), so I'm raising the topic in the list so we can discuss this together.
[1] https://review.opendev.org/c/openstack/puppet-swift/+/777564
I had a quick look and I believe Tobias is not against not using that var but the fact that you renamed the resource from python-ceilometermiddleware to ceilometermiddleware which does not make sense indeed and could be breaking.
Kind regards,
-yoctozepto
I'll fix that. But the question remain: shall we deprecate the use of ${pyvers} and use python3 instead? Cheers, Thomas Goirand (zigo)
I have posted a comment on the said patch but I prefer using pyvers in that specific patch because; - The change seems to be a backport candidate and using pyvers helps us backport the change to older branches like Train which still supports python 2 IIRC. - We still use pyvers in the other modules so it would make sense to have consistent implementation before we actually remove usage of pyvers from all modules. Personally I prefer keeping pyvers because it doesn't make much effort but helps us just in case we have Python4 in a future (though I don't know when it really comes) or any distro changes package name format from python3- to python- or any different style. On Sat, Feb 27, 2021 at 9:59 PM Thomas Goirand <zigo@debian.org> wrote:
On 2/27/21 11:28 AM, Radosław Piliszek wrote:
Hi Zigo,
On Sat, Feb 27, 2021 at 10:24 AM Thomas Goirand <zigo@debian.org> wrote:
Using the ${pyvers} so we can switch be between Python versions made sense 1 or 2 years ago. However, I'm in the opinion that we should stop using that, and switch to using python3 everywhere directly whenever possible. Though Tobias seems to not agree (see [1]), so I'm raising the topic in the list so we can discuss this together.
[1] https://review.opendev.org/c/openstack/puppet-swift/+/777564
I had a quick look and I believe Tobias is not against not using that var but the fact that you renamed the resource from python-ceilometermiddleware to ceilometermiddleware which does not make sense indeed and could be breaking.
Kind regards,
-yoctozepto
I'll fix that. But the question remain: shall we deprecate the use of ${pyvers} and use python3 instead?
Cheers,
Thomas Goirand (zigo)
Hi, On 2/27/21 3:52 PM, Takashi Kajinami wrote:
I have posted a comment on the said patch but I prefer using pyvers in that specific patch because; - The change seems to be a backport candidate and using pyvers helps us backport the change to older branches like Train which still supports python 2 IIRC.
Even Rocky is already using python3 in Debian/Ubuntu. The last distro using the Python 2 version would be Stretch (which is long EOLed) and Bionic (at this time, people should be moving to Focal, no ?, which IMO are not targets for backports. Therefore, for this specific patch, even if you want to do a backport, it doesn't make sense. Are you planning to do such a backport for the RPM world?
- We still use pyvers in the other modules so it would make sense to have consistent implementation before we actually remove usage of pyvers from all modules.
The fact that we're using pyvers in other modules isn't relevant, because I'm hereby proposing to remove such a usage.
Personally I prefer keeping pyvers because it doesn't make much effort but helps us just in case we have Python4 in a future (though I don't know when it really comes) or any distro changes package name format from python3- to python- or any different style. There will *not* be any python4, ever. Upstream python people clearly expressed themselves about it.
On the Debian/Ubuntu side of things, we don't plan on ever changing our policy, so packages will stay as "python3-<FOO>". It does add some effort to use a ${pyvers} variable, because it makes the code less readable. Cheers, Thomas Goirand (zigo)
On Sat, Feb 27, 2021 at 7:28 PM Thomas Goirand <zigo@debian.org> wrote:
- We still use pyvers in the other modules so it would make sense to have consistent implementation before we actually remove usage of pyvers from all modules.
The fact that we're using pyvers in other modules isn't relevant, because I'm hereby proposing to remove such a usage.
Personally I prefer keeping pyvers because it doesn't make much effort but helps us just in case we have Python4 in a future (though I don't know when it really comes) or any distro changes package name format from python3- to python- or any different style. There will *not* be any python4, ever. Upstream python people clearly expressed themselves about it.
On the Debian/Ubuntu side of things, we don't plan on ever changing our policy, so packages will stay as "python3-<FOO>".
It does add some effort to use a ${pyvers} variable, because it makes the code less readable.
I am with zigo on this one. Going forward, this variable simply does not make sense. -yoctozepto
On Sun, Feb 28, 2021 at 3:32 AM Thomas Goirand <zigo@debian.org> wrote:
Hi,
On 2/27/21 3:52 PM, Takashi Kajinami wrote:
I have posted a comment on the said patch but I prefer using pyvers in that specific patch because; - The change seems to be a backport candidate and using pyvers helps us backport the change to older branches like Train which still supports python 2 IIRC.
Even Rocky is already using python3 in Debian/Ubuntu. The last distro using the Python 2 version would be Stretch (which is long EOLed) and Bionic (at this time, people should be moving to Focal, no ?, which IMO are not targets for backports.
Therefore, for this specific patch, even if you want to do a backport, it doesn't make sense.
Are you planning to do such a backport for the RPM world?
We still have queens open for puppet-openstack modules. IIRC rdo rocky is based on CentOS7 and Python2. Also, I don't really like to see inconsistent implementation caused by backport knowing that we don't support python3 in these branches. Anyway we can consider that when we actually backport the change.
- We still use pyvers in the other modules so it would make sense to have consistent implementation before we actually remove usage of pyvers from all modules.
The fact that we're using pyvers in other modules isn't relevant, because I'm hereby proposing to remove such a usage.
OK. If we can get rid of pyvers from all modules in this cycle then we can go in this direction. Note that when we remove pyvers then we also need to clean up existing implementations for CentOS7 to select package names in unit tests.
Personally I prefer keeping pyvers because it doesn't make much effort but helps us just in case we have Python4 in a future (though I don't know when it really comes) or any distro changes package name format from python3- to python- or any different style. There will *not* be any python4, ever. Upstream python people clearly expressed themselves about it.
On the Debian/Ubuntu side of things, we don't plan on ever changing our policy, so packages will stay as "python3-<FOO>".
It does add some effort to use a ${pyvers} variable, because it makes the code less readable.
Cheers,
Thomas Goirand (zigo)
On 2/28/21 12:10 PM, Takashi Kajinami wrote:
On Sun, Feb 28, 2021 at 3:32 AM Thomas Goirand <zigo@debian.org <mailto:zigo@debian.org>> wrote:
Hi,
On 2/27/21 3:52 PM, Takashi Kajinami wrote: > I have posted a comment on the said patch but I prefer using pyvers in > that specific patch because; > - The change seems to be a backport candidate and using pyvers helps us > backport the change > to older branches like Train which still supports python 2 IIRC.
Even Rocky is already using python3 in Debian/Ubuntu. The last distro using the Python 2 version would be Stretch (which is long EOLed) and Bionic (at this time, people should be moving to Focal, no ?, which IMO are not targets for backports.
Therefore, for this specific patch, even if you want to do a backport, it doesn't make sense.
Are you planning to do such a backport for the RPM world?
We still have queens open for puppet-openstack modules. IIRC rdo rocky is based on CentOS7 and Python2. Also, I don't really like to see inconsistent implementation caused by backport knowing that we don't support python3 in these branches. Anyway we can consider that when we actually backport the change.
I'm a bit surprised that you still care about such an old release as Queens. Is this the release shipped with CentOS 7? In any ways, thanks for letting me know, I have to admit I don't know much about the RPM side of things. In such case, I'm ok to keep the ${pyvers} variable for the CentOS case for a bit longer then, but can we agree when we stop using it? Also IMO, forcing it for Debian/Ubuntu doesn't make sense anymore. Thanks everyone for participating in this thread, Cheers, Thomas Goirand (zigo)
On Sun, Feb 28, 2021 at 8:22 PM Thomas Goirand <zigo@debian.org> wrote:
On 2/28/21 12:10 PM, Takashi Kajinami wrote:
On Sun, Feb 28, 2021 at 3:32 AM Thomas Goirand <zigo@debian.org <mailto:zigo@debian.org>> wrote:
Hi,
On 2/27/21 3:52 PM, Takashi Kajinami wrote: > I have posted a comment on the said patch but I prefer using
pyvers in
> that specific patch because; > - The change seems to be a backport candidate and using pyvers helps us > backport the change > to older branches like Train which still supports python 2 IIRC.
Even Rocky is already using python3 in Debian/Ubuntu. The last distro using the Python 2 version would be Stretch (which is long EOLed) and Bionic (at this time, people should be moving to Focal, no ?, which
IMO
are not targets for backports.
Therefore, for this specific patch, even if you want to do a
backport,
it doesn't make sense.
Are you planning to do such a backport for the RPM world?
We still have queens open for puppet-openstack modules. IIRC rdo rocky is based on CentOS7 and Python2. Also, I don't really like to see inconsistent implementation caused by backport knowing that we don't support python3 in these branches. Anyway we can consider that when we actually backport the change.
I'm a bit surprised that you still care about such an old release as Queens. Is this the release shipped with CentOS 7?
RDO Queens anr RDO Rocky are based on CentOS 7. RDO Train supports both CentOS7 and CentOS8 IIRC. In any ways, thanks for letting me know, I have to admit I don't know
much about the RPM side of things.
In such case, I'm ok to keep the ${pyvers} variable for the CentOS case for a bit longer then, but can we agree when we stop using it? Also IMO, forcing it for Debian/Ubuntu doesn't make sense anymore.
IMO we can think about backport separately(we can make any required changes in backport only) so we can get rid of pyvers in master for both CentOS and Ubuntu/Debian. However I prefer to deal with that removal separately and consistently, so that we won't create inconsistent implementation where some relies on pyvers and the others doesn't rely on pyvers.
Thanks everyone for participating in this thread, Cheers,
Thomas Goirand (zigo)
Hello, I don't really mind removing it if what you are saying about Python 4 is true, I don't know so I will take your word for it. I agree with Takashi, that we can remove it but should do it consistenly and then be done with it, just stopping to use it because it takes literally five seconds more to fix a patch seems like something one can live with until then. Remember that we've for years been working on consistency, cleaning up and removing duplication, I wouldn't want to start introducing then after that. So yes, let's remove it but require everything up until that point to be consistent, maybe a short timespan to squeeze all those changes into Wallaby though. Is this a change you want to drive Thomas? Best regards and have a good start of the week everybody. Tobias ________________________________ From: Takashi Kajinami <tkajinam@redhat.com> Sent: Sunday, February 28, 2021 3:12:42 PM To: Thomas Goirand Cc: OpenStack Discuss Subject: Re: [puppet-openstack] stop using the ${pyvers} variable On Sun, Feb 28, 2021 at 8:22 PM Thomas Goirand <zigo@debian.org<mailto:zigo@debian.org>> wrote: On 2/28/21 12:10 PM, Takashi Kajinami wrote:
On Sun, Feb 28, 2021 at 3:32 AM Thomas Goirand <zigo@debian.org<mailto:zigo@debian.org> <mailto:zigo@debian.org<mailto:zigo@debian.org>>> wrote:
Hi,
On 2/27/21 3:52 PM, Takashi Kajinami wrote: > I have posted a comment on the said patch but I prefer using pyvers in > that specific patch because; > - The change seems to be a backport candidate and using pyvers helps us > backport the change > to older branches like Train which still supports python 2 IIRC.
Even Rocky is already using python3 in Debian/Ubuntu. The last distro using the Python 2 version would be Stretch (which is long EOLed) and Bionic (at this time, people should be moving to Focal, no ?, which IMO are not targets for backports.
Therefore, for this specific patch, even if you want to do a backport, it doesn't make sense.
Are you planning to do such a backport for the RPM world?
We still have queens open for puppet-openstack modules. IIRC rdo rocky is based on CentOS7 and Python2. Also, I don't really like to see inconsistent implementation caused by backport knowing that we don't support python3 in these branches. Anyway we can consider that when we actually backport the change.
I'm a bit surprised that you still care about such an old release as Queens. Is this the release shipped with CentOS 7? RDO Queens anr RDO Rocky are based on CentOS 7. RDO Train supports both CentOS7 and CentOS8 IIRC. In any ways, thanks for letting me know, I have to admit I don't know much about the RPM side of things. In such case, I'm ok to keep the ${pyvers} variable for the CentOS case for a bit longer then, but can we agree when we stop using it? Also IMO, forcing it for Debian/Ubuntu doesn't make sense anymore. IMO we can think about backport separately(we can make any required changes in backport only) so we can get rid of pyvers in master for both CentOS and Ubuntu/Debian. However I prefer to deal with that removal separately and consistently, so that we won't create inconsistent implementation where some relies on pyvers and the others doesn't rely on pyvers. Thanks everyone for participating in this thread, Cheers, Thomas Goirand (zigo)
On Sat, Feb 27, 2021 at 2:29 AM Thomas Goirand <zigo@debian.org> wrote:
Hi,
Using the ${pyvers} so we can switch be between Python versions made sense 1 or 2 years ago. However, I'm in the opinion that we should stop using that, and switch to using python3 everywhere directly whenever possible. Though Tobias seems to not agree (see [1]), so I'm raising the topic in the list so we can discuss this together.
It would only make sense to remove it going forward. I would not support dropping it for any stable branches. It's something we could work on in X unless someone wants to drive it for W.
Your thoughts? Cheers,
Thomas Goirand (zigo)
[1] https://review.opendev.org/c/openstack/puppet-swift/+/777564
participants (5)
-
Alex Schultz
-
Radosław Piliszek
-
Takashi Kajinami
-
Thomas Goirand
-
Tobias Urdin