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

WIP: Platform nir0s distro #35985

Open
wants to merge 5 commits into
base: devel
from

Conversation

Projects
None yet
6 participants
@alikins
Contributor

alikins commented Feb 9, 2018

SUMMARY

WIP fixes #35312

Fixes #30489

Opening PR now to get CI results for some of the tests.

ISSUE TYPE
  • Feature Pull Request
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/module_utils/distro.py

ANSIBLE VERSION
ansible 2.6.0 (platform_nir0s_distro 5483c6d09e) last updated 2018/02/09 16:38:49 (GMT -400)
  config file = /home/adrian/src/ansible/ansible.cfg
  configured module search path = [u'/home/adrian/ansible/my-modules']
  ansible python module location = /home/adrian/src/ansible/lib/ansible
  executable location = /home/adrian/src/ansible/bin/ansible
  python version = 2.7.14 (default, Jan 17 2018, 14:28:32) [GCC 7.2.1 20170915 (Red Hat 7.2.1-2)]

ADDITIONAL INFORMATION

@ansibot

This comment has been minimized.

Contributor

ansibot commented Feb 9, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/module_utils/distro.py:549:32: ansible-format-automatic-specification Format string contains automatic field numbering specification

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/module_utils/distro.py:37:0: ImportError: No module named argparse

The test ansible-test sanity --test no-assert [explain] failed with the error:

Command "test/sanity/code-smell/no-assert.py" returned exit status 1.
>>> Standard Output
Use of assert in production code is not recommended.
Python will remove all assert statements if run with optimizations
Alternatives:
    if not isinstance(value, dict):
        raise AssertionError("Expected a dict for value")
lib/ansible/module_utils/distro.py:549:9:         assert obj is not None, 'call {} on an instance'.format(self._fname)

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/module_utils/distro.py:721:24: E126 continuation line over-indented for hanging indent

click here for bot help

@alikins alikins force-pushed the alikins:platform_nir0s_distro branch Feb 12, 2018

@sivel sivel removed the needs_triage label Feb 12, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Feb 12, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/module_utils/distro/__init__.py:555:32: ansible-format-automatic-specification Format string contains automatic field numbering specification

The test ansible-test sanity --test import --python 2.6 [explain] failed with 1 error:

lib/ansible/module_utils/distro/__init__.py:43:0: ImportError: No module named argparse

The test ansible-test sanity --test empty-init [explain] failed with the error:

Command "test/sanity/code-smell/empty-init.sh" returned exit status 1.
>>> Standard Output
lib/ansible/module_utils/distro/__init__.py
One or more __init__.py file(s) listed above are non-empty.

The test ansible-test sanity --test no-assert [explain] failed with the error:

Command "test/sanity/code-smell/no-assert.py" returned exit status 1.
>>> Standard Output
Use of assert in production code is not recommended.
Python will remove all assert statements if run with optimizations
Alternatives:
    if not isinstance(value, dict):
        raise AssertionError("Expected a dict for value")
lib/ansible/module_utils/distro/__init__.py:555:9:         assert obj is not None, 'call {} on an instance'.format(self._fname)

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/module_utils/distro/__init__.py:727:24: E126 continuation line over-indented for hanging indent

click here for bot help

@ansibot ansibot added the stale_ci label Feb 20, 2018

@alikins

This comment has been minimized.

Contributor

alikins commented Feb 21, 2018

I failed to link this to #30489 previously.

@alikins alikins force-pushed the alikins:platform_nir0s_distro branch Feb 21, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Feb 21, 2018

@ansibot ansibot removed the stale_ci label Feb 21, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Feb 21, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/module_utils/distro/__init__.py:556:28: ansible-format-automatic-specification Format string contains automatic field numbering specification

click here for bot help

lib/ansible/module_utils/distro/LICENSE Outdated
@@ -0,0 +1,202 @@
Apache License

This comment has been minimized.

@mattclay

mattclay Feb 21, 2018

Member

Can the license go in the licenses directory and be referenced there instead?

@alikins alikins force-pushed the alikins:platform_nir0s_distro branch to 531d292 Feb 22, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Feb 22, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/module_utils/distro/__init__.py:556:28: ansible-format-automatic-specification Format string contains automatic field numbering specification

The test ansible-test sanity --test empty-init [explain] failed with 1 error:

lib/ansible/module_utils/distro/__init__.py:0:0: empty __init__.py required

click here for bot help

@ansibot ansibot added feature and removed feature_pull_request labels Mar 2, 2018

@ansibot ansibot added the stale_ci label Mar 10, 2018

@ansibot ansibot added the affects_2.6 label May 23, 2018

marcosps added a commit to marcosps/ansible that referenced this pull request Oct 14, 2018

hostname.py: Fix openSUSE distribution name
Currently there is a problem to discover the distribution name by
python's platform.linux_distribution method used by Ansible's hostname modules
when running a playblook in openSUSE[1]. This method was deprecated
in python  3.5, and was already removed in the current developer version (3.8)[2]

There is a patch[3] being reviewed that would fix this problem by using
the distro package, which would return correctly the openSUSE string,
also avoiding the use of the deprecated python method in Ansible.

Using the distro package, the returned distribution string in a openSUSE system would
be 'openSUSE Leap', but currently Ansible is checking for 'Opensuse ' in hostname.py,
so this patch change the string to check for the correct value, making possible to use
the hostname module in a playbook for example.

[1]: https://bugzilla.suse.com/show_bug.cgi?id=997614
[2]: https://docs.python.org/3.7/library/platform.html#platform.linux_distribution
[2]: ansible#35985

Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>

marcosps added a commit to marcosps/ansible that referenced this pull request Oct 14, 2018

hostname.py: Fix openSUSE distribution name
Currently there is a problem to discover the distribution name by
python's platform.linux_distribution method used by Ansible's hostname modules
when running a playblook in openSUSE[1]. This method was deprecated
in python  3.5, and was already removed in the current developer version (3.8)[2]

There is a patch[3] being reviewed that would fix this problem by using
the distro package, which would return correctly the openSUSE string,
also avoiding the use of the deprecated python method in Ansible.

Using the distro package, the returned distribution string in a openSUSE system would
be 'openSUSE Leap', but currently Ansible is checking for 'Opensuse ' in hostname.py,
so this patch change the string to check for the correct value, making possible to use
the hostname module in a playbook for example.

1: https://bugzilla.suse.com/show_bug.cgi?id=997614
2: https://docs.python.org/3.7/library/platform.html#platform.linux_distribution
3: ansible#35985

Signed-off-by: Marcos Paulo de Souza <marcos.souza.org@gmail.com>

@abadger abadger force-pushed the alikins:platform_nir0s_distro branch from 531d292 to 771a77a Oct 18, 2018

@ansibot ansibot removed the stale_ci label Oct 18, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Oct 18, 2018

The test ansible-test sanity --test pylint [explain] failed with 1 error:

lib/ansible/module_utils/distro/__init__.py:556:28: ansible-format-automatic-specification Format string contains automatic field numbering specification

The test ansible-test sanity --test empty-init [explain] failed with 1 error:

lib/ansible/module_utils/distro/__init__.py:0:0: empty __init__.py required

click here for bot help

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 11, 2018

The test ansible-test sanity --test pylint [explain] failed with 3 errors:

lib/ansible/module_utils/common/sys_info.py:19:11: undefined-variable Undefined variable 'platform'
lib/ansible/module_utils/common/sys_info.py:29:7: undefined-variable Undefined variable 'platform'
lib/ansible/module_utils/common/sys_info.py:45:7: undefined-variable Undefined variable 'platform'

The test ansible-test sanity --test import --python 2.6 [explain] failed with 2 errors:

test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:19:0: NameError: global name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:45:0: NameError: global name 'platform' is not defined

The test ansible-test sanity --test import --python 2.7 [explain] failed with 4 errors:

lib/ansible/modules/system/filesystem.py:284:0: NameError: global name 'platform' is not defined
lib/ansible/modules/system/hostname.py:570:0: NameError: global name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:19:0: NameError: global name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:45:0: NameError: global name 'platform' is not defined

The test ansible-test sanity --test import --python 3.5 [explain] failed with 4 errors:

lib/ansible/modules/system/filesystem.py:284:0: NameError: name 'platform' is not defined
lib/ansible/modules/system/hostname.py:570:0: NameError: name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:19:0: NameError: name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:45:0: NameError: name 'platform' is not defined

The test ansible-test sanity --test import --python 3.6 [explain] failed with 4 errors:

lib/ansible/modules/system/filesystem.py:284:0: NameError: name 'platform' is not defined
lib/ansible/modules/system/hostname.py:570:0: NameError: name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:19:0: NameError: name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:45:0: NameError: name 'platform' is not defined

The test ansible-test sanity --test import --python 3.7 [explain] failed with 4 errors:

lib/ansible/modules/system/filesystem.py:284:0: NameError: name 'platform' is not defined
lib/ansible/modules/system/hostname.py:570:0: NameError: name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:19:0: NameError: name 'platform' is not defined
test/runner/.tox/import/lib/ansible/module_utils/common/sys_info.py:45:0: NameError: name 'platform' is not defined

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

hacking/tests/gen_distribution_version_testcase.py:6:161: E501 line too long (166 > 160 characters)

The test ansible-test sanity --test validate-modules [explain] failed with 2 errors:

lib/ansible/modules/system/filesystem.py:0:0: E321 Exception attempting to import module for argument_spec introspection, 'name 'platform' is not defined'
lib/ansible/modules/system/hostname.py:0:0: E321 Exception attempting to import module for argument_spec introspection, 'name 'platform' is not defined'

click here for bot help

@abadger abadger force-pushed the alikins:platform_nir0s_distro branch 2 times, most recently from eaffff4 to 76eaf91 Dec 11, 2018

@ansibot

This comment has been minimized.

Contributor

ansibot commented Dec 11, 2018

The test ansible-test sanity --test pylint [explain] failed with 2 errors:

test/units/module_utils/basic/test_platform_distribution.py:49:15: singleton-comparison Comparison to None should be 'expr is None'
test/units/module_utils/basic/test_platform_distribution.py:78:15: singleton-comparison Comparison to None should be 'expr is None'

The test ansible-test sanity --test pep8 [explain] failed with 6 errors:

hacking/tests/gen_distribution_version_testcase.py:6:161: E501 line too long (166 > 160 characters)
test/units/module_utils/basic/test_platform_distribution.py:28:1: E302 expected 2 blank lines, found 1
test/units/module_utils/basic/test_platform_distribution.py:49:35: E711 comparison to None should be 'if cond is None:'
test/units/module_utils/basic/test_platform_distribution.py:75:1: E302 expected 2 blank lines, found 1
test/units/module_utils/basic/test_platform_distribution.py:78:43: E711 comparison to None should be 'if cond is None:'
test/units/module_utils/basic/test_platform_distribution.py:90:1: E302 expected 2 blank lines, found 1

click here for bot help

@abadger abadger force-pushed the alikins:platform_nir0s_distro branch 18 times, most recently from 44a2cd7 to 9ed002e Dec 11, 2018

alikins and others added some commits Feb 21, 2018

Changes to bundled distro to be Python-2.6 compatible
* Port bundled distro to use optparse instead of argparse (py2.6)
* Use an absolute import to satisfy the current import testing harness
* Port from subprocess.check_output to subprocess.Popen.communicate() (py2.6)
* Add license location

The changes have been proposed upstream here:
nir0s/distro#232

Upstream is contemplating a branch where everyone wanting python-2.6
support can collaborate without it becoming part of the regularly
supported releases.
WIP: add tests to compare to platform.*dist
Can be removed once everything is ported.
Skip sanity tests that don't apply to bundled code
* add distro to pep8 skip tests
* Skip no-assert test for distro
* Add bundled distro to the empty-init skip list

@abadger abadger force-pushed the alikins:platform_nir0s_distro branch from 9ed002e to 29d4a40 Dec 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment