Skip to content

Improve support for pip on CentOS7/EPEL #347

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Dec 12, 2016
Merged

Improve support for pip on CentOS7/EPEL #347

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c1aaf34
Improve support for pip on CentOS7/EPEL
ju5t Dec 10, 2016
f08a885
Remove the unneeded dependency on EPEL
ju5t Dec 12, 2016
11577b3
Added pip package tests on RHEL 5 and 6
ju5t Dec 12, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions manifests/install.pp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,9 @@
if $::python::version =~ /^3/ {
$pip_category = undef
$pip_package = 'python3-pip'
} elsif ($::osfamily == 'RedHat') and (versioncmp($::operatingsystemmajrelease, '7') >= 0) and ($python::use_epel == true) {
Copy link

Choose a reason for hiding this comment

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

I disabled use_epel since I have manage it elsewhere, but still would like this case to apply, which is why I like #349 better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I probably misinterpreted $use_epel when writing this. I'll remove it.

As far as #349 is concerned: it doesn't do any spec tests and uses a regex on a fact that can be done cheaper and more effectively with versioncmp() and $::operatingsystemmajrelease.

$pip_category = undef
$pip_package = 'python2-pip'
} elsif $::osfamily == 'Gentoo' {
$pip_category = 'dev-python'
$pip_package = 'pip'
Expand Down
16 changes: 16 additions & 0 deletions spec/classes/python_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,22 @@
end
end

context "on a Redhat 7 OS" do
Copy link
Member

Choose a reason for hiding this comment

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

awesome work, thank you!

Would you mind adding a case for RedHat 6, that shows the same thing as this, but with the correct pip package name and update the existing RedHat 5 part with the name attribute. That would give us much better testing for all use cases with EL.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've pushed a change that adds tests for 6 and tests the name on 5 too.

I would suggest rewriting the spec tests at some point so there isn't that much divergence between the different operating systems. E.g.

require 'spec_helper'
describe 'python', :type => :class do
  context 'supported operating systems' do
    on_supported_os.each do |os, facts|
      context "on #{os}" do

Something for the future.

let :facts do
{
:id => 'root',
:kernel => 'Linux',
:osfamily => 'RedHat',
:operatingsystem => 'RedHat',
:operatingsystemmajrelease => '7',
:concat_basedir => '/dne',
:path => '/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin',
}
end
it { is_expected.to contain_class("python::install") }
it { is_expected.to contain_package("pip").with_name('python2-pip') }
end

context "on a SLES 11 SP3" do
let :facts do
{
Expand Down