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.

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

Jump to bottom

docker modules: improve documentation of list options #62179

Merged
merged 1 commit into from
Oct 4, 2019
Merged

docker modules: improve documentation of list options #62179

merged 1 commit into from
Oct 4, 2019

Conversation

felixfontein
Copy link
Contributor

SUMMARY

Since #59244 was merged, list options can be documented better. This PR adds the new feature to all docker modules.

I've also added this for return data, even though it's not supported yet.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

docker_compose
docker_container
docker_host_info
docker_image
docker_image_info
docker_network
docker_node
docker_node_info
docker_prune
docker_stack
docker_swarm
docker_swarm_info
docker_swarm_service

@ansibot
Copy link
Contributor

ansibot commented Sep 12, 2019

cc @DBendit @WojciechowskiPiotr @akshay196 @chouseknecht @cove @danihodovic @dariko @dusdanig @hannseman @joshuaconner @jwitko @kassiansun @keitwb @olsaki @sluther @softzilla @ssbarnea @tbouvet @zfil
click here for bot help

@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. affects_2.10 This issue/PR affects Ansible v2.10 cloud docker docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. owner_pr This PR is made by the module's maintainer. support:community This issue/PR relates to code supported by the Ansible community. labels Sep 12, 2019
@ansibot
Copy link
Contributor

ansibot commented Sep 12, 2019

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

lib/ansible/modules/cloud/docker/docker_host_info.py:0:0: return-syntax-error: RETURN.containers.elements: extra keys not allowed @ data['containers']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_host_info.py:0:0: return-syntax-error: RETURN.images.elements: extra keys not allowed @ data['images']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_host_info.py:0:0: return-syntax-error: RETURN.networks.elements: extra keys not allowed @ data['networks']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_host_info.py:0:0: return-syntax-error: RETURN.volumes.elements: extra keys not allowed @ data['volumes']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_image_info.py:0:0: return-syntax-error: RETURN.images.elements: extra keys not allowed @ data['images']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_node_info.py:0:0: return-syntax-error: RETURN.nodes.elements: extra keys not allowed @ data['nodes']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_prune.py:0:0: return-syntax-error: RETURN.containers.elements: extra keys not allowed @ data['containers']['elements']. Got 'str'
lib/ansible/modules/cloud/docker/docker_prune.py:0:0: return-syntax-error: RETURN.images.elements: extra keys not allowed @ data['images']['elements']. Got 'str'
lib/ansible/modules/cloud/docker/docker_prune.py:0:0: return-syntax-error: RETURN.networks.elements: extra keys not allowed @ data['networks']['elements']. Got 'str'
lib/ansible/modules/cloud/docker/docker_prune.py:0:0: return-syntax-error: RETURN.volumes.elements: extra keys not allowed @ data['volumes']['elements']. Got 'str'
lib/ansible/modules/cloud/docker/docker_swarm.py:0:0: return-syntax-error: RETURN.actions.elements: extra keys not allowed @ data['actions']['elements']. Got 'str'
lib/ansible/modules/cloud/docker/docker_swarm_info.py:0:0: return-syntax-error: RETURN.nodes.elements: extra keys not allowed @ data['nodes']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_swarm_info.py:0:0: return-syntax-error: RETURN.services.elements: extra keys not allowed @ data['services']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_swarm_info.py:0:0: return-syntax-error: RETURN.tasks.elements: extra keys not allowed @ data['tasks']['elements']. Got 'dict'
lib/ansible/modules/cloud/docker/docker_swarm_service.py:0:0: return-syntax-error: RETURN.changes.elements: extra keys not allowed @ data['changes']['elements']. Got 'str'

click here for bot help

@ansibot ansibot added the ci_verified Changes made in this PR are causing tests to fail. label Sep 12, 2019
@ansibot ansibot removed the ci_verified Changes made in this PR are causing tests to fail. label Sep 15, 2019
@felixfontein
Copy link
Contributor Author

I split up the PR into two commits, one which should work right now, and the other in devel...felixfontein:docker-use-elements-docs-return which needs additional core support.

@felixfontein felixfontein changed the title [WIP] docker modules: improve documentation of list options docker modules: improve documentation of list options Sep 15, 2019
@felixfontein
Copy link
Contributor Author

ready_for_review

Especially the changes to docker_swarm_service are more complex and should be considered in detail.

@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. labels Sep 15, 2019
@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 Sep 23, 2019
Andersson007
Andersson007 reviewed Oct 3, 2019
View reviewed changes
lib/ansible/modules/cloud/docker/docker_compose.py
@@ -47,6 +47,7 @@
- List of Compose file names relative to I(project_src). Overrides C(docker-compose.yml) or C(docker-compose.yaml).
- Files are loaded and merged in the order given.
type: list
elements: path
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, haven't heard of it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has only been around since #59244, i.e. it is available only for 2.9 and devel.

Copy link
Contributor

Choose a reason for hiding this comment

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

cool, thanks, will know!

lib/ansible/modules/cloud/docker/docker_container.py
@@ -692,6 +712,7 @@
description:
- Mount a tmpfs directory
type: list
elements: str
Copy link
Contributor

Choose a reason for hiding this comment

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

There are some points which could be improved too while you are here :), e.g.

  1. some descriptions end with dots, some not
  2. in comparisons's parameter there are big bunches of texts, could be split
  3. some sentences of descriptions are in quotes (without need)
  4. too long sentences happen
  5. seealso section could be added (at list with general links to the docker documentation, and other related docker modules)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These are things that can also be backported to stable-2.8, so I'd rather do that in another PR.

Andersson007
Andersson007 approved these changes Oct 4, 2019
View reviewed changes
Copy link
Contributor

@Andersson007 Andersson007 left a comment

Choose a reason for hiding this comment

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

shipit

@felixfontein felixfontein merged commit a59b9d4 into ansible:devel Oct 4, 2019
@felixfontein felixfontein deleted the docker-use-elements-docs branch October 4, 2019 19:50
@felixfontein
Copy link
Contributor Author

@Andersson007 also thanks for reviewing this one!

@Andersson007
Copy link
Contributor

thank you for improving the documentation!

felixfontein added a commit to felixfontein/ansible that referenced this pull request Oct 4, 2019
@felixfontein
Improve type documentation. (ansible#62179)
8235836 
(cherry picked from commit a59b9d4)
@felixfontein felixfontein mentioned this pull request Oct 4, 2019
Merged
This was referenced Oct 4, 2019
Merged
Merged
@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Oct 8, 2019
acozine pushed a commit that referenced this pull request Oct 11, 2019
@felixfontein @acozine
Improve type documentation. (#62179) (#63142)
84ad291 
(cherry picked from commit a59b9d4)
@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.
Reviewers

@Andersson007 Andersson007 Andersson007 approved these changes

Assignees
No one assigned
Labels
affects_2.10 This issue/PR affects Ansible v2.10 cloud community_review In order to be merged, this PR must follow the community review workflow. docker docs This issue/PR relates to or includes documentation. module This issue/PR relates to a module. owner_pr This PR is made by the module's maintainer. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@felixfontein @ansibot @Andersson007 @sivel