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

Open
wants to merge 2 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@jagibson
Copy link
Contributor

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.
Update unarchive.py - Further clarify extra_opts
##### 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
@ansibot

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2019

@ansibot ansibot removed the ci_verified label Jun 20, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

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 label Jun 20, 2019

@jagibson

This comment has been minimized.

Copy link
Contributor Author

commented Jun 21, 2019

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 label Jun 29, 2019

@acozine
Copy link
Contributor

left a comment

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

@@ -71,7 +71,8 @@
version_added: "2.1"
extra_opts:
description:
- Specify additional options by passing in an array.
- Specify additional options by passing in an array. Each space-separated part of the archive program's command-line option should be a new element\

This comment has been minimized.

Copy link
@acozine

acozine Jul 11, 2019

Contributor
Suggested change
- Specify additional options by passing in an array. Each space-separated part of the archive program's command-line option should be a new element\
- Specify additional options by passing in an array.
- Each space-separated command-line option should be a new element of the array. See examples.
@@ -71,7 +71,8 @@
version_added: "2.1"
extra_opts:
description:
- Specify additional options by passing in an array.
- Specify additional options by passing in an array. Each space-separated part of the archive program's command-line option should be a new element\
of the array.

This comment has been minimized.

Copy link
@acozine

acozine Jul 11, 2019

Contributor
Suggested change
of the array.
- Command-line options with multiple elements must use multiple lines in the array, one for each element.
@acozine

This comment has been minimized.

Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.