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

platform.linux_distribution() is deprecated; stop using it #1001

Merged
merged 4 commits into from
Aug 8, 2018

Conversation

smithfarm
Copy link
Contributor

Fixes: http://tracker.ceph.com/issues/18141
Signed-off-by: Nathan Cutler ncutler@suse.com

@smithfarm
Copy link
Contributor Author

@zmc Ping

Copy link
Member

@zmc zmc left a comment

Choose a reason for hiding this comment

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

Where is platform.linux_distribution() deprecated?

This would need to be tested with all OSes to ensure we wouldn't regress.

@smithfarm
Copy link
Contributor Author

platform.linux_distribution() is deprecated since python 3.5, and will be removed in python 3.7 (see https://docs.python.org/3.6/library/platform.html#platform.linux_distribution).

@smithfarm
Copy link
Contributor Author

The implementation in Python 2.7 returns (,,) on newer distros like openSUSE Tumbleweed (which dropped the old /etc/SuSE-release now that /etc/os-release is the new standard). I think this (,,) return value happens in newer Fedoras as well, but I haven't tried it.

@smithfarm
Copy link
Contributor Author

@zmc When you say "all OSes" which OSes do you mean? Are there any that don't have /etc/os-release ?

@ktdreyer
Copy link
Member

ktdreyer commented Feb 8, 2017

FYI mock recently switched to the distro lib, now that platform.dist and platform.linux_distribution are going away in py37.

@smithfarm
Copy link
Contributor Author

@ktdreyer Do we need an elaborate python module when a glance at /etc/os-release gives us the information we need?

@ktdreyer
Copy link
Member

Teuthology does a lot already - it was just a suggestion :)

@smithfarm
Copy link
Contributor Author

In ceph/ceph#23111 I have unit tests that cover "all" OSes. I will add a commit to this PR implementing these unit tests in teuthology so we can be confident that platform.linux_distribution is no longer needed.

Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Nathan Cutler <ncutler@suse.com>
Signed-off-by: Nathan Cutler <ncutler@suse.com>
@smithfarm
Copy link
Contributor Author

Added unit tests and support for new distros. Pinging again for review.

@gregsfortytwo
Copy link
Member

This looks good to me. @zmc, is there other testing we can do besides rolling it out in the lab? If not I think we should just go for it...

@smithfarm
Copy link
Contributor Author

I will run some ad-hoc tests.

@smithfarm
Copy link
Contributor Author

@zmc Ping

Copy link
Member

@gregsfortytwo gregsfortytwo left a comment

Choose a reason for hiding this comment

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

Well, I didn't hear any immediate complaints and it's been a couple weeks.

@gregsfortytwo gregsfortytwo merged commit e44082b into ceph:master Aug 8, 2018
@smithfarm smithfarm deleted the wip-18141 branch August 13, 2018 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants