linux_distribution() return on CentOS differs from the README

[terminalmage]

From the README

>>> import distro
>>> distro.linux_distribution(full_distribution_name=False)
('centos', '7.1.1503', 'Core')

Actual behavior

>>> import distro
>>> distro.linux_distribution(full_distribution_name=False)
('centos', '7', 'Core')

The same goes for CentOS 8:

>>> import distro
>>> distro.linux_distribution(full_distribution_name=False)
('centos', '8', 'Core')

Possible solutions

1. Use best=True when getting the version in linux_distribution()
2. Add a best_version argument to linux_distribution() to allow this to be passed through.

I do think that adding a best_version argument would be a good idea. However, I'm not sure defaulting to best=True helps, since it adds the maintenance releases to Ubuntu LTS releases.

So, perhaps this current behavior is best, since it makes the behavior of linux_distribution more congruent with other Linux distros. If that is indeed the position of the project maintainers, and no changes need to be made to the code, I can open a PR to update the README.

[danie-dejager]

If on CentOS, then better to use centos-release if you want to see the full version:

# cat /etc/centos-release
CentOS Linux release 7.8.2003 (Core)

[terminalmage]

That is not the point. I know how to write code that parses /etc/centos-release. The point is that a) the documentation is incorrect, and b) distro is already capable of retrieving the full version, but that functionality is not exposed to distro.linux_distribution().

[danie-dejager]

It looks like a bug to me. Distro should should get the full version from centos-release as the full version details is not in /etc/os-release as with other distros like Debian.

[terminalmage]

I think that doing something special for CentOS would be ugly.

Here's what I came up with: #267

However, I overlooked the last line of the contributing docs and need to make changes to the tests I added. (EDIT: maybe not? Looks like maybe a test case in TestOverall is only required for new distros.)

[danie-dejager]

FYI

I logged an issue with CentOS. Will hopefully get them to put the full version in /etc/os-release and in doing so behave more like other Distros.

https://bugs.centos.org/view.php?id=17792

[brejoc]

I just learned that Centos Stream will only have a major version number and redhat-release will be gone: https://git.centos.org/rpms/centos-stream-release/blob/c8s/f/SPECS/centos-stream-release.spec#_67-83

[nir0s]

So, perhaps this current behavior is best since it makes the behavior of linux_distribution more congruent with other Linux distros. If that is indeed the position of the project maintainers, and no changes need to be made to the code, I can open a PR to update the README.

To be clear, when I initially started this project, I meant for linux_distribution() to implement what py27's platform.linux_distribution() implemented. AFAIC, distro.linux_distribution() shouldn't even be used, and was simply implemented so that there's a drop-in replacement for anyone using py26 up to py37.

What I then intended is for people to start using distro.id(), distro.name(), and distro.version() instead, which do provide some flexibility.

I wouldn't make any breaking change to linux_distribution(), and furthermore wouldn't add best_version to it, as it's simply easily retrievable by using distro.version(best=True). If anyone wanted to modify their code to use linux_distribution(best_version=True), they may as well not use linux_distribution() at all.

Thoughts?

[terminalmage]

To be clear, when I initially started this project, I meant for linux_distribution() to implement what py27's platform.linux_distribution() implemented.

OK, well even so.... it doesn't. platform.linux_distribution produces the return from the documentation. distro.linux_distribution does not.

>>> import platform
>>> platform.linux_distribution(full_distribution_name=False)
('centos', '7.8.2003', 'Core')

(The behavior of distro, as well as the documentation, is referenced in the OP)

AFAIC, distro.linux_distribution() shouldn't even be used, and was simply implemented so that there's a drop-in replacement for anyone using py26 up to py37.

That's fair. I don't have any code that relies on distro behaving exactly like platform.linux_distribution. I was merely trying to reconcile the fact that its actual behavior doesn't match your own documentation. If you're not invested in making it match platform's implementation, I don't really care, but then the documentation should probably be updated, or you're going to have more confused people like myself coming here to open bug reports.

[nir0s]

Indeed it's not. The platform module was extremely inconsistent in what it returned, hence.. distro.

However, I agree with you that what's important here is to prevent confusion. A PR explaining the parity will be most welcome.

[jdufresne]

AFAIC, distro.linux_distribution() shouldn't even be used

@nir0s Would it make sense to emit a deprecation warning when calling linux_distribution then? The warning could direct the user to the more appropriate functions to use. If you agree, I'm happy to open a PR to achieve the result.

[nir0s]

Funny, I haven't even thought about it until you asked. That would be great. I indeed think that it's completely redundant.

…

On Sun, Jul 4, 2021, 20:21 Jon Dufresne ***@***.***> wrote: AFAIC, distro.linux_distribution() shouldn't even be used @nir0s <https://github.com/nir0s> Would it make sense to emit a deprecation warning <https://docs.python.org/3/library/exceptions.html#DeprecationWarning> when calling linux_distribution then? The warning could direct the user to the more appropriate functions to use. If you agree, I'm happy to open a PR to achieve the result. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#263 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAPJBBFMQHJHCJ3OLGDRW2TTWCKCBANCNFSM4MQ5B6CA> .

[jdufresne]

👍 Done in #296.

[HorlogeSkynet]

Can we close here @terminalmage ?