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

Fix bug in RHEL6 detection by distributor id. #148

Conversation

tgamblin
Copy link
Contributor

@tgamblin tgamblin commented Nov 6, 2016

This wasn't working on our RHEL6 clusters. It seems to figure out that our RHEL7 ones should be called rhel by some other means, but on RHEL6 it got down to the distributor ID. I updated the translation table to include the server release.



- There are two types of RHEL: server and workstation.

- Distributor ID table only had a string for workstation.

- Added a string for server.

- There are two types of RHEL: server and workstation.
- Distributor ID table only had a string for workstation.
- Added a string for server.
@tgamblin
Copy link
Contributor Author

tgamblin commented Nov 6, 2016

Does this need a test? It looks like the coverage is the same as for the base branch.

@nir0s
Copy link
Collaborator

nir0s commented Nov 22, 2016

In principle, any distribution we can get our hands on should have a test. Coverage unfortunately doesn't mean you really cover every possible scenario. Can you add a test for this distro? You can see the contributions.md file for info. Thanks

@adamjstewart
Copy link
Contributor

I can help add a test case. I'm the one who originally reported the problem. It occurred for me on RHEL 5 with the following redhat-release:

Red Hat Enterprise Linux Server release 5.11 (Tikanga)

and:

$ lsb_release -a
LSB Version:	:core-4.0-amd64:core-4.0-ia32:core-4.0-noarch:graphics-4.0-amd64:graphics-4.0-ia32:graphics-4.0-noarch:printing-4.0-amd64:printing-4.0-ia32:printing-4.0-noarch
Distributor ID:	RedHatEnterpriseServer
Description:	Red Hat Enterprise Linux Server release 5.11 (Tikanga)
Release:	5.11
Codename:	Tikanga

Do we just need to add the /etc/redhat-release file to tests/resources/distros/rhel5/etc/redhat-release and add test_rhel5_os_release to TestOSRelease in tests/test_distro.py? @tgamblin Can I push to this PR branch?

@adamjstewart
Copy link
Contributor

Or would it be add test_rhel5_release to TestOverall? That seems to match better.

@adamjstewart
Copy link
Contributor

Ok, added the following test in addition to my /etc/redhat-release:

def test_rhel5_release(self):
        desired_outcome = {
            'id': 'rhel',
            'name': 'Red Hat Enterprise Linux Server',
            'pretty_name': 'Red Hat Enterprise Linux Server release 5.11 (Tikanga)',
            'version': '5.11',
            'pretty_version': '5.11 (Tikanga)',
            'best_version': '5.11',
            'codename': 'Tikanga',
            'major_version': '5',
            'minor_version': '11'
        }
        self._test_outcome(desired_outcome)

        desired_info = {
            'id': 'redhat',
            'name': 'Red Hat Enterprise Linux Server',
            'version_id': '5.11',
            'codename': 'Tikanga'
        }
        self._test_release_file_info('redhat-release', desired_info)

@nir0s How do I run the test locally to make sure it works?

@tgamblin I wasn't able to push to your branch.

@nir0s
Copy link
Collaborator

nir0s commented Nov 22, 2016

Just run tox locally. Instructions are in the readme

@tgamblin
Copy link
Contributor Author

@adamjstewart: gave you access -- can you try again?

@adamjstewart
Copy link
Contributor

Hmm, seems to be some strange error messages in Travis. Don't think those are my fault.

@tgamblin tgamblin closed this Nov 27, 2016
@tgamblin tgamblin reopened this Nov 27, 2016
@nir0s
Copy link
Collaborator

nir0s commented Jan 12, 2017

It certainly isn't your fault. Travis are providing pypy3 with python 3.2 which isn't supported by pip9. Can you please rebase on top of master?

Thanks.

@nir0s nir0s closed this Jan 12, 2017
@nir0s nir0s reopened this Jan 12, 2017
@nir0s
Copy link
Collaborator

nir0s commented Feb 8, 2017

#165 fixed code coverage and rebased on top of master.

@nir0s nir0s closed this Feb 8, 2017
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.

3 participants