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

Update unarchive.py - Further clarify extra_opts #58102

Merged
merged 8 commits into from
Oct 14, 2019

Conversation

jagibson
Copy link
Contributor

@jagibson jagibson commented Jun 19, 2019

SUMMARY

Update the description in the options to help assist with what the module is looking for in the extra_opts. #31873 alone is not enough since if you are not familiar with the option given in the example it may not be obvious that both elements are part of the same option instead of being two different options.

ISSUE TYPE
  • Docs Pull Request

+label: docsite_pr

SUMMARY

Further clarifies #31873.

Also the Ansible output is not clear what the error is if you format the extra_opts incorrectly (see below)

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

unarchive.py module

ADDITIONAL INFORMATION
Failed to find handler for \"/var/tmp/blah.tar.gz\". Make sure the required command to extract the file is installed. Command \"/bin/gtar\" could not handle archive. Command \"/usr/bin/unzip\" could not handle archive.

##### SUMMARY
Update the description in the options to help assist with what the module is looking for in the extra_opts.  ansible#31873 alone is not enough since if you are not familiar with the option given in the example it may not be obvious that both elements are part of the same option instead of being two different options.


##### ISSUE TYPE
- Docs Pull Request

+label: docsite_pr
@ansibot
Copy link
Contributor

ansibot commented Jun 19, 2019

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

lib/ansible/modules/files/unarchive.py:74:161: E501 line too long (168 > 160 characters)

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Jun 19, 2019

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 ci_verified Changes made in this PR are causing tests to fail. docs This issue/PR relates to or includes documentation. files Files category m:unarchive This issue/PR relates to the unarchive module. module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. needs_triage Needs a first human triage before being processed. small_patch support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jun 19, 2019
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Jun 20, 2019
@ansibot
Copy link
Contributor

ansibot commented Jun 20, 2019

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

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test ansible-doc --python 3.8 [explain] failed with 1 error:

lib/ansible/modules/files/unarchive.py:0:0: has a documentation error formatting or is missing documentation.

The test ansible-test sanity --test docs-build [explain] failed with the error:

Command "/usr/bin/python3.6 test/sanity/code-smell/docs-build.py" returned exit status 1.
>>> Standard Error
Command 'make singlehtmldocs' failed with status code: 2
--> Standard Output
PYTHONPATH=../../lib ../bin/dump_config.py --template-file=../templates/config.rst.j2 --output-dir=rst/reference_appendices/ -d ../../lib/ansible/config/base.yml
mkdir -p rst/cli
PYTHONPATH=../../lib ../bin/generate_man.py --template-file=../templates/cli_rst.j2 --output-dir=rst/cli/ --output-format rst ../../lib/ansible/cli/*.py
PYTHONPATH=../../lib ../bin/dump_keywords.py --template-dir=../templates --output-dir=rst/reference_appendices/ -d ./keyword_desc.yml
PYTHONPATH=../../lib ../bin/plugin_formatter.py -t rst --template-dir=../templates --module-dir=../../lib/ansible/modules -o rst/modules/ 
Evaluating module files...
Makefile:93: recipe for target 'modules' failed
--> Standard Error
Traceback (most recent call last):
  File "../bin/plugin_formatter.py", line 815, in <module>
    main()
  File "../bin/plugin_formatter.py", line 770, in main
    plugin_info, categories = get_plugin_info(options.module_dir, limit_to=options.limit_to, verbose=(options.verbosity > 0))
  File "../bin/plugin_formatter.py", line 300, in get_plugin_info
    doc, examples, returndocs, metadata = plugin_docs.get_docstring(module_path, fragment_loader, verbose=verbose)
  File "/root/ansible/lib/ansible/utils/plugin_docs.py", line 111, in get_docstring
    data = read_docstring(filename, verbose=verbose, ignore_errors=ignore_errors)
  File "/root/ansible/lib/ansible/parsing/plugin_docs.py", line 59, in read_docstring
    data[varkey] = AnsibleLoader(child.value.s, file_name=filename).get_single_data()
  File "/usr/local/lib/python3.6/dist-packages/yaml/constructor.py", line 41, in get_single_data
    node = self.get_single_node()
  File "ext/_yaml.pyx", line 707, in _yaml.CParser.get_single_node
  File "ext/_yaml.pyx", line 725, in _yaml.CParser._compose_document
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 774, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 853, in _yaml.CParser._compose_sequence_node
  File "ext/_yaml.pyx", line 905, in _yaml.CParser._parse_next_event
yaml.scanner.ScannerError: while scanning a simple key
  in "<unicode string>", line 58, column 7
could not find expected ':'
  in "<unicode string>", line 59, column 5
make: *** [modules] Error 1

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

lib/ansible/modules/files/unarchive.py:0:0: E324 Argument 'validate_certs' in argument_spec defines default as (True) but documentation defines default as (False)
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'creates' in argument_spec defines type as 'path' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'dest' in argument_spec defines type as 'path' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'exclude' in argument_spec defines type as 'list' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'extra_opts' in argument_spec defines type as 'list' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'keep_newer' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'list_files' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'remote_src' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'src' in argument_spec defines type as 'path' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: E337 Argument 'validate_certs' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:76:5: E302 DOCUMENTATION is not valid YAML
test/sanity/validate-modules/ignore.txt:1535:1: A102 Remove since "lib/ansible/modules/files/unarchive.py" passes "E323" test

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

lib/ansible/modules/files/unarchive.py:76:5: error DOCUMENTATION: syntax error: could not find expected ':'

click here for bot help

@bcoca bcoca removed the needs_triage Needs a first human triage before being processed. label Jun 20, 2019
@samdoran samdoran added the ci_verified Changes made in this PR are causing tests to fail. label Jun 21, 2019
@jagibson
Copy link
Contributor Author

This is what I get for editing the docs in github directly. I'll have to try this locally.

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jun 29, 2019
Copy link
Contributor

@acozine acozine left a comment

Choose a reason for hiding this comment

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

@jagibson, I think accepting these two suggestions will get CI passing.

lib/ansible/modules/files/unarchive.py Outdated Show resolved Hide resolved
lib/ansible/modules/files/unarchive.py Outdated Show resolved Hide resolved
@acozine
Copy link
Contributor

acozine commented Jul 11, 2019

@jagibson re-read the PR and example - I misunderstood what you were saying at first. Do the updated suggestions express your meaning correctly?

@acozine acozine added the needs_info This issue requires further information. Please answer any outstanding questions. label Aug 12, 2019
@ansibot ansibot removed the needs_info This issue requires further information. Please answer any outstanding questions. label Aug 12, 2019
@ansibot ansibot added the needs_info This issue requires further information. Please answer any outstanding questions. label Aug 20, 2019
@dagwieers dagwieers added the docsite_pr This PR is created from documentation using the "Edit on GitHub" link. label Sep 5, 2019
@ansibot
Copy link
Contributor

ansibot commented Sep 23, 2019

@jagibson This pullrequest is waiting for your response. Please respond or the pullrequest will be closed.

click here for bot help

Co-Authored-By: Alicia Cozine <879121+acozine@users.noreply.github.com>
@ansibot ansibot removed ci_verified Changes made in this PR are causing tests to fail. small_patch labels Sep 24, 2019
@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Sep 24, 2019
Fix for multi-line

Co-Authored-By: Alicia Cozine <879121+acozine@users.noreply.github.com>
@ansibot
Copy link
Contributor

ansibot commented Sep 24, 2019

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

lib/ansible/modules/files/unarchive.py:0:0: doc-default-does-not-match-spec: Argument 'validate_certs' in argument_spec defines default as (True) but documentation defines default as (False)
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'creates' in argument_spec defines type as 'path' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'dest' in argument_spec defines type as 'path' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'exclude' in argument_spec defines type as 'list' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'extra_opts' in argument_spec defines type as 'list' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'keep_newer' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'list_files' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'remote_src' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'src' in argument_spec defines type as 'path' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:0:0: parameter-type-not-in-doc: Argument 'validate_certs' in argument_spec defines type as 'bool' but documentation doesn't define type
lib/ansible/modules/files/unarchive.py:76:5: documentation-syntax-error: DOCUMENTATION is not valid YAML
test/sanity/ignore.txt:2263:1: A100: Ignoring 'nonexistent-parameter-documented' on 'lib/ansible/modules/files/unarchive.py' is unnecessary

The test ansible-test sanity --test docs-build [explain] failed with the error:

Command "/usr/bin/python3.6 /root/ansible/test/sanity/code-smell/docs-build.py" returned exit status 1.
>>> Standard Error
Command 'make singlehtmldocs' failed with status code: 2
--> Standard Output
PYTHONPATH=../../lib ../../hacking/build-ansible.py collection-meta --template-file=../templates/collections_galaxy_meta.rst.j2 --output-dir=rst/dev_guide/ ../../lib/ansible/galaxy/data/collections_galaxy_meta.yml
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-config --template-file=../templates/config.rst.j2 --output-dir=rst/reference_appendices/ ../../lib/ansible/config/base.yml
mkdir -p rst/cli
PYTHONPATH=../../lib ../../hacking/build-ansible.py generate-man --template-file=../templates/cli_rst.j2 --output-dir=rst/cli/ --output-format rst ../../lib/ansible/cli/*.py
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-keywords --template-dir=../templates --output-dir=rst/reference_appendices/ ./keyword_desc.yml
PYTHONPATH=../../lib ../../hacking/build-ansible.py document-plugins -t rst --template-dir=../templates --module-dir=../../lib/ansible/modules -o rst/modules/ 
Evaluating module files...
Makefile:99: recipe for target 'modules' failed
--> Standard Error
Traceback (most recent call last):
  File "../../hacking/build-ansible.py", line 92, in <module>
    main()
  File "../../hacking/build-ansible.py", line 81, in main
    retval = command.main(args)
  File "/root/ansible/hacking/build_library/build_ansible/command_plugins/plugin_formatter.py", line 721, in main
    plugin_info, categories = get_plugin_info(args.module_dir, limit_to=args.limit_to, verbose=(args.verbosity > 0))
  File "/root/ansible/hacking/build_library/build_ansible/command_plugins/plugin_formatter.py", line 225, in get_plugin_info
    doc, examples, returndocs, metadata = plugin_docs.get_docstring(module_path, fragment_loader, verbose=verbose)
  File "/root/ansible/lib/ansible/utils/plugin_docs.py", line 124, in get_docstring
    data = read_docstring(filename, verbose=verbose, ignore_errors=ignore_errors)
  File "/root/ansible/lib/ansible/parsing/plugin_docs.py", line 59, in read_docstring
    data[varkey] = AnsibleLoader(child.value.s, file_name=filename).get_single_data()
  File "/usr/local/lib/python3.6/dist-packages/yaml/constructor.py", line 41, in get_single_data
    node = self.get_single_node()
  File "ext/_yaml.pyx", line 707, in _yaml.CParser.get_single_node
  File "ext/_yaml.pyx", line 725, in _yaml.CParser._compose_document
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 890, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 776, in _yaml.CParser._compose_node
  File "ext/_yaml.pyx", line 892, in _yaml.CParser._compose_mapping_node
  File "ext/_yaml.pyx", line 905, in _yaml.CParser._parse_next_event
yaml.parser.ParserError: while parsing a block mapping
  in "<unicode string>", line 56, column 5
did not find expected key
  in "<unicode string>", line 59, column 5
make: *** [modules] Error 1

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

Command "ansible-doc -t module unarchive" returned exit status 1.
>>> Standard Error
ERROR! module unarchive missing documentation (or could not parse documentation): while parsing a block mapping
  in "<unicode string>", line 56, column 5
did not find expected key
  in "<unicode string>", line 59, column 5

The test ansible-test sanity --test package-data [explain] failed with the error:

Command "/usr/bin/python3.6 /root/ansible/test/sanity/code-smell/package-data.py" returned exit status 1.
>>> Standard Error
Traceback (most recent call last):
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 383, in <module>
    main()
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 360, in main
    sdist_path = create_sdist(tmp_dir)
  File "/root/ansible/test/sanity/code-smell/package-data.py", line 174, in create_sdist
    raise Exception('make snapshot failed:\n%s' % stderr)
Exception: make snapshot failed:
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
fatal: not a git repository (or any parent up to mount point /)
Stopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).
docs/man/man1/ansible-galaxy.1.rst:51: (WARNING/2) Definition list ends without a blank line; unexpected unindent.
docs/man/man1/ansible-galaxy.1.rst:57: (WARNING/2) Definition list ends without a blank line; unexpected unindent.
ERROR! module unarchive at /tmp/tmpy8ky_0r7/lib/ansible/modules/files/unarchive.py has a documentation error formatting or is missing documentation.
Traceback (most recent call last):
  File "packaging/release/changelogs/changelog.py", line 835, in <module>
    main()
  File "packaging/release/changelogs/changelog.py", line 102, in main
    args.func(args)
  File "packaging/release/changelogs/changelog.py", line 132, in command_release
    plugins = load_plugins(version=version, force_reload=reload_plugins)
  File "packaging/release/changelogs/changelog.py", line 184, in load_plugins
    '--json', '--metadata-dump', '-t', plugin_type])
  File "/usr/lib/python3.6/subprocess.py", line 356, in check_output
    **kwargs).stdout
  File "/usr/lib/python3.6/subprocess.py", line 438, in run
    output=stdout, stderr=stderr)
subprocess.CalledProcessError: Command '['/tmp/tmpy8ky_0r7/bin/ansible-doc', '--json', '--metadata-dump', '-t', 'module']' returned non-zero exit status 1.
make: *** [changelog] Error 1

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

lib/ansible/modules/files/unarchive.py:76:110: W291: trailing whitespace

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

lib/ansible/modules/files/unarchive.py:76:5: error: DOCUMENTATION: syntax error: expected <block end>, but found '-'

click here for bot help

ERROR: lib/ansible/modules/files/unarchive.py:0:0: doc-default-does-not-match-spec: Argument 'validate_certs' in argument_spec defines default as (True) but documentation defines default as (False) (75%)

Changed doc to be "True" which matches the code.
@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Sep 24, 2019
Maybe it was a fluke that it failed before.
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 4, 2019
@acozine
Copy link
Contributor

acozine commented Oct 14, 2019

This looks good, thanks @jagibson.

@ansibot ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Oct 14, 2019
@acozine acozine merged commit e967060 into ansible:devel Oct 14, 2019
acozine pushed a commit to acozine/ansible that referenced this pull request Oct 15, 2019
* Update unarchive.py - Further clarify extra_opts
Update the description in the options to help assist with what the module is looking for in the extra_opts.  ansible#31873 alone is not enough since if you are not familiar with the option given in the example it may not be obvious that both elements are part of the same option instead of being two different options.

Co-Authored-By: Alicia Cozine <879121+acozine@users.noreply.github.com>
(cherry picked from commit e967060)
acozine added a commit that referenced this pull request Oct 16, 2019
* Update sros_config.py (#63132)

(cherry picked from commit b3deab4)

* add note about collection links (#63346)

(cherry picked from commit bdd0fac)

* Nosh module documentation update (#63303)
* detail the 'status' dictionary
* did not add individual descriptions, see nosh documentation for those
* sample: False not being rendered unless 'False' is quoted

(cherry picked from commit df28378)

* Move galaxy appendix info to a new Galaxy section (#63356)
* start galaxy docs restructure
* shared snippets in txt files
* moved all content to galaxy section

(cherry picked from commit ae265bc)

* Enter should be Italics as it is a menu option (#63441)

(cherry picked from commit 7a8c909)

* ini_file: remove incorrect documentation (#63394)

(cherry picked from commit ab249a4)

* Changed with_items to loop in VMware example (#63022)

(cherry picked from commit 876a2d5)

* ovirt update fetch_nested param doc (#63191)

(cherry picked from commit 0beab6b)

* Update 'delete' parameter description in synchronize module (#63450)

Co-Authored-By: Abhijeet Kasurde <akasurde@redhat.com>
(cherry picked from commit da46800)

* Update unarchive.py - Further clarify extra_opts (#58102)
Update the description in the options to help assist with what the module is looking for in the extra_opts.  #31873 alone is not enough since if you are not familiar with the option given in the example it may not be obvious that both elements are part of the same option instead of being two different options.

Co-Authored-By: Alicia Cozine <879121+acozine@users.noreply.github.com>
(cherry picked from commit e967060)

* ovirt_vm add warning about next run configuration (#63458)

(cherry picked from commit 16e49fd)

* Fix doc - porting_guide_2.8 value example (#63493)

(cherry picked from commit e0f67b5)

* Fix warnings for ufw examples (#63505)

(cherry picked from commit e4eea05)
@ansible ansible locked and limited conversation to collaborators Nov 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 docs This issue/PR relates to or includes documentation. docsite_pr This PR is created from documentation using the "Edit on GitHub" link. files Files category m:unarchive This issue/PR relates to the unarchive module. module This issue/PR relates to a module. needs_info This issue requires further information. Please answer any outstanding questions. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants