Skip to content
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

Avoid use of deprecated function in distro module. #248

Merged
merged 2 commits into from
Feb 24, 2022

Conversation

nuclearsandwich
Copy link
Contributor

@nuclearsandwich nuclearsandwich commented Feb 24, 2022

Recent versions of the distro module have deprecated the distro.linux_distribution() which is a compatibility shim for the
platform.linux_distribution() function removed in python 3.8.

All versions of the distro library going back to at least 1.0.11 packaged in Ubuntu Bionic2 provide the id, version, and codename methods which are recommended instead.

distro.id() is used instead of distro.name() corresponding with the full_distribution_name=0 argument to linux_distribution.

When updating the tests to support this change I noticed that the mocks were being added in such a way that the dist conditional branch would never actually be tested. I didn't actually look at how far back we'd have to
go to find a platform module which did not support linux_distribution but it made sense to at least make testing that branch possible.

We'll have to take care that the codename member of the tuple continues to match what our scripts expect downstream since the tests mock the distro return values we don't have full coverage of the actual results on contemporary platforms.

Closes #241

@nuclearsandwich
Copy link
Contributor Author

nuclearsandwich commented Feb 24, 2022

Temporarily cherry-picking #245 to get CI results back.

Edit: rebased now that #245 is merged. (Thanks Scott!)

Recent versions of the distro module have deprecated the
`distro.linux_distribution()` which is a compatibility shim for the
`platform.linux_distribution()` function removed in python 3.8.

All versions of the distro library going back to at least 1.0.1[1]
packaged in Ubuntu Bionic[2] provide the `id`, `version`, and `codename`
methods which are recommended instead.

`distro.id()` is used instead of `distro.name()` corresponding with the
full_distribution_name=0 argument to `linux_distribution`.

When updating the tests to support this change I noticed that the mocks
were being added in such a way that the `dist` conditional branch would never
actually be tested. I didn't actually look at how far back we'd have to
go to find a platform module which did not support `linux_distribution`
but it made sense to at least make testing that branch possible.

We'll have to take care that the `codename` member of the tuple
continues to match what our scripts expect downstream since the tests
mock the distro return values we don't have full coverage of the actual
results on contemporary platforms.

[1]: https://github.com/python-distro/distro/blob/v1.0.1/
[2]: https://packages.ubuntu.com/bionic/python3-distro
@nuclearsandwich nuclearsandwich force-pushed the nuclearsandwich/distro-deprecations branch from 319441f to 8375657 Compare February 24, 2022 20:42
@nuclearsandwich nuclearsandwich marked this pull request as ready for review February 24, 2022 21:48
@nuclearsandwich
Copy link
Contributor Author

I had to do some work in #241 (comment) to convince myself that aside from getting us away from a deprecated function this also fixes that issue. At least in all known situations it resolves that issue as well.

Copy link
Member

@cottsay cottsay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for spearheading the development and testing on this one.

@nuclearsandwich
Copy link
Contributor Author

At @cottsay's prompting I went a little further back and found that there are some cases in older versions of distro where codename isn't what we want.

However, across all tested versions of the distro module the distro.codename() function returns what we want and so the patch in #248 should resolve our issues on all currently supported platforms.

I ran an extra test using the packaged version of distro in Ubuntu Bionic, our oldest supported distro and the distro.codename() returns "Bionic Beaver" which means that I need to update that to say that across all recent versions of distro...

I'm going to update this PR to require distro>=1.4.0 which is when codename first started returning the symbolic name rather than the full nickname.

The `distro.codename()` function in prior versions returned the "full
nickname / codename" on Ubuntu. For example "Bionic Beaver" instead of
"bionic".

All supported distributions which package python3 >= 3.8 also package
python3-distro >= 1.4.0 so this should not actually change anything and
is added as a precaution.
@nuclearsandwich nuclearsandwich merged commit fd75b64 into master Feb 24, 2022
@nuclearsandwich nuclearsandwich deleted the nuclearsandwich/distro-deprecations branch February 24, 2022 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ubuntu detection broken with 'distro>=1.7.0'
3 participants