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

Add collection version to plugin pages and collection plugin index #200

Merged
merged 14 commits into from
Nov 30, 2020
Merged

Add collection version to plugin pages and collection plugin index #200

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
105b314
Add collection versions.
felixfontein Oct 3, 2020
0307b28
Linting.
felixfontein Nov 3, 2020
28d4130
Also retrieve collection paths.
felixfontein Nov 19, 2020
b60e840
Address review comments.
felixfontein Nov 24, 2020
ef068c6
collection_info -> collection_to_plugin_info
felixfontein Nov 24, 2020
e041575
collection_infos -> collection_metadata
felixfontein Nov 24, 2020
8521bdc
Remove optionals.
felixfontein Nov 24, 2020
1f2dc20
Fix mistake.
felixfontein Nov 24, 2020
0996b65
Convert AnsibleCollectionDocs into a NamedTuple.
felixfontein Nov 24, 2020
3a29a34
Add collection name as title. Fixes #219.
felixfontein Nov 24, 2020
892998d
Replace NamedTuple with plain tuple.
felixfontein Nov 24, 2020
9f0a1ca
Crash if collection has no metadata.
felixfontein Nov 25, 2020
54ed7ce
Use concrete type.
felixfontein Nov 25, 2020
298eafb
Linting.
felixfontein Nov 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion antsibull/cli/doc_commands/plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from ... import app_context
from ...augment_docs import augment_docs
from ...compat import asyncio_run
from ...docs_parsing import AnsibleCollectionMetadata
from ...docs_parsing.fqcn import get_fqcn_parts, is_fqcn
from ...jinja2.environment import doc_environment
from ...logging import log
Expand Down Expand Up @@ -103,7 +104,7 @@ def generate_docs() -> int:
error_tmpl = env.get_template('plugin-error.rst.j2')

asyncio_run(write_rst(
'.'.join([namespace, collection]), plugin, plugin_type,
'.'.join([namespace, collection]), AnsibleCollectionMetadata.empty(), plugin, plugin_type,
plugin_info, errors, plugin_tmpl, error_tmpl, '',
path_override=output_path))
flog.debug('Finished writing plugin docs')
Expand Down
20 changes: 13 additions & 7 deletions antsibull/cli/doc_commands/stable.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
from ...schemas.docs import DOCS_SCHEMAS
from ...venv import VenvRunner, FakeVenvRunner
from ...write_docs import (
CollectionInfoT,
output_all_plugin_rst,
output_collection_index,
output_indexes,
Expand Down Expand Up @@ -221,7 +222,7 @@ def get_plugin_contents(plugin_info: t.Mapping[str, t.Mapping[str, t.Any]],


def get_collection_contents(plugin_content: t.Mapping[str, t.Mapping[str, t.Mapping[str, str]]],
) -> t.DefaultDict[str, t.Dict[str, t.Mapping[str, str]]]:
) -> CollectionInfoT:
Copy link
Contributor

Choose a reason for hiding this comment

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

The reason not to do this would be that the types for return values should be concrete: We know we're returning a DefaultDict of Dicts because we're creating those inside of this function. So anything making use of our return values would know they can depend on those behaviours being present. Typing it as generic Mapping of Mapping means that code using our return value can't depend on those behaviours.

Input types, otoh, should specify the least common denominator that they need. So in terms of taking input, a Mapping is appropriate unless the function wanted to do something that depended on the DefaultDict's behaviour.

I kind of feel like we should keep return types and input types separate for that reason (when they're different). Does it make sense to define a return spec alongside the input spec so that it's easier to write this sort of thing and see that they're compatible types? (like RetCollectionInfoT as a counterpart to CollectionInfoT?) Or does that defeat the purpose of keeping the input type as lowest common denominator?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Usually things like "this implementation uses a defaultdict" is an implementation detail and callers should not depend on that, because it makes it really hard to change the implementation without having to go through all callers to make sure they don't depend on it. (If a generic Mapping is documented, they can of course still use it like a defaultdict, but then they're clearly faulty.)

Using CollectionInfoT here says to API users that "this collection returns what can be used in other APIs as CollectionInfoT", as opposed to "we return this concrete mapping type, which might be similar to one of the input types of other functions, but you have to figure that out by yourself". Also, defining RetCollectionInfoT different from CollectionInfoT makes it impossible to ever provide any function returning that which does not use a similar implementation (based on defaultdict).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed in 54ed7ce.

Copy link
Contributor

Choose a reason for hiding this comment

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

Interesting..... So I don't think of static typing info as part of the API but I can definitely see your point that if it is, then we should be using it to define capabilities to expect rather than the specific type. I guess there's two separate use cases (static typing to catch errors in the current implementation vs static typing to define the expected API) and in this specific case they're a little bit at odds.

There are cases where defaultdict and dict behaviour would be different even though you are only using functions that are common to both, so I think it can be beneficial to know concretely what we're returning. To be fair, I don't think static typing will prevent it (at this stage in type checkers' development, at least), though so type checking might not be the place to specify this and enforce it.... An example would be the following code:

def test() -> defauldict:
    return defaultdict(int)

d = test()
try:
    print(d['x'])
except KeyError:
    print("d['x'] is unknown")

"""
Return the plugins which are in each collection.

Expand Down Expand Up @@ -268,10 +269,12 @@ def generate_docs_for_all_collections(venv: t.Union[VenvRunner, FakeVenvRunner],
"""

# Get the info from the plugins
plugin_info = asyncio_run(get_ansible_plugin_info(
plugin_info, collection_metadata = asyncio_run(get_ansible_plugin_info(
venv, collection_dir, collection_names=collection_names))
flog.notice('Finished parsing info from plugins')
flog.notice('Finished parsing info from plugins and collections')
# flog.fields(plugin_info=plugin_info).debug('Plugin data')
# flog.fields(
# collection_metadata=collection_metadata).debug('Collection metadata')

"""
# Turn these into some sort of decorator that will choose to dump or load the values
Expand Down Expand Up @@ -311,21 +314,24 @@ def generate_docs_for_all_collections(venv: t.Union[VenvRunner, FakeVenvRunner],
"""

plugin_contents = get_plugin_contents(plugin_info, nonfatal_errors)
collection_info = get_collection_contents(plugin_contents)
collection_to_plugin_info = get_collection_contents(plugin_contents)
flog.debug('Finished getting collection data')

# Only build top-level index if requested
if create_indexes:
asyncio_run(output_collection_index(collection_info, dest_dir))
asyncio_run(output_collection_index(collection_to_plugin_info, dest_dir))
flog.notice('Finished writing collection index')
asyncio_run(output_plugin_indexes(plugin_contents, dest_dir))
flog.notice('Finished writing plugin indexes')

asyncio_run(output_indexes(collection_info, dest_dir, squash_hierarchy=squash_hierarchy))
asyncio_run(output_indexes(collection_to_plugin_info, dest_dir,
collection_metadata=collection_metadata,
squash_hierarchy=squash_hierarchy))
flog.notice('Finished writing indexes')

asyncio_run(output_all_plugin_rst(collection_info, plugin_info,
asyncio_run(output_all_plugin_rst(collection_to_plugin_info, plugin_info,
nonfatal_errors, dest_dir,
collection_metadata=collection_metadata,
squash_hierarchy=squash_hierarchy))
flog.debug('Finished writing plugin docs')

Expand Down
2 changes: 1 addition & 1 deletion antsibull/data/docsite/plugin.rst.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
the same module name.
{% else %}
.. note::
This plugin is part of the `@{collection}@ collection <https://galaxy.ansible.com/@{collection | replace('.', '/', 1)}@>`_.
This plugin is part of the `@{collection}@ collection <https://galaxy.ansible.com/@{collection | replace('.', '/', 1)}@>`_{% if collection_version %} (version @{ collection_version }@){% endif %}.

To install it use: :code:`ansible-galaxy collection install @{collection}@`.

Expand Down
17 changes: 12 additions & 5 deletions antsibull/data/docsite/plugins_by_collection.rst.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,29 @@

.. _plugins_in_@{collection_name}@:

Plugin Index
============
@{collection_name.title()}@
@{ '=' * (collection_name | length) }@

These are the plugins in the @{collection_name}@ collection
{% if collection_version %}
Collection version @{ collection_version }@
{% endif %}

.. toctree::
:maxdepth: 1

Plugin Index
------------

These are the plugins in the @{collection_name}@ collection

{% for category, plugins in plugin_maps.items() | sort %}

{% if category == 'module' %}
Modules
-------
~~~~~~~
{% else %}
@{ category | capitalize }@ Plugins
@{ '-' * ((category | length) + 8) }@
@{ '~' * ((category | length) + 8) }@
{% endif %}

{% for name, desc in plugins.items() | sort %}
Expand Down
16 changes: 16 additions & 0 deletions antsibull/docs_parsing/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,19 @@ def _get_environment(collection_dir: t.Optional[str]) -> t.Dict[str, str]:
if env_var in os.environ:
env[env_var] = os.environ[env_var]
return env


class AnsibleCollectionMetadata:
path: str
version: t.Optional[str]

def __init__(self, path: str, version: t.Optional[str]):
self.path = path
self.version = version

def __repr__(self):
return 'AnsibleCollectionMetadata({0}, {1})'.format(repr(self.path), repr(self.version))

@classmethod
def empty(cls, path='.'):
return cls(path=path, version=None)
68 changes: 60 additions & 8 deletions antsibull/docs_parsing/ansible_doc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import asyncio
import json
import sys
import os
import traceback
import typing as t
from concurrent.futures import ThreadPoolExecutor
Expand All @@ -18,7 +19,7 @@
from ..logging import log
from ..vendored.json_utils import _filter_non_json_lines
from .fqcn import get_fqcn_parts
from . import _get_environment, ParsingError
from . import _get_environment, ParsingError, AnsibleCollectionMetadata

if t.TYPE_CHECKING:
from ..venv import VenvRunner, FakeVenvRunner
Expand Down Expand Up @@ -158,10 +159,56 @@ async def _get_plugin_info(plugin_type: str, ansible_doc: 'sh.Command',
return results


def get_collection_metadata(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
env: t.Dict[str, str],
collection_names: t.Optional[t.List[str]] = None,
) -> t.Dict[str, AnsibleCollectionMetadata]:
collection_metadata = {}

# Obtain ansible.builtin version
if collection_names is None or 'ansible.builtin' in collection_names:
venv_ansible = venv.get_command('ansible')
ansible_version_cmd = venv_ansible('--version', _env=env)
raw_result = ansible_version_cmd.stdout.decode('utf-8', errors='surrogateescape')
path: t.Optional[str] = None
version: t.Optional[str] = None
for line in raw_result.splitlines():
if line.strip().startswith('ansible python module location'):
path = line.split('=', 2)[1].strip()
if line.startswith('ansible '):
version = line[len('ansible '):]
collection_metadata['ansible.builtin'] = AnsibleCollectionMetadata(
path=path, version=version)

# Obtain collection versions
venv_ansible_galaxy = venv.get_command('ansible-galaxy')
ansible_collection_list_cmd = venv_ansible_galaxy('collection', 'list', _env=env)
raw_result = ansible_collection_list_cmd.stdout.decode('utf-8', errors='surrogateescape')
current_base_path = None
for line in raw_result.splitlines():
parts = line.split()
if len(parts) >= 2:
if parts[0] == '#':
current_base_path = parts[1]
else:
collection_name = parts[0]
version = parts[1]
if '.' in collection_name:
if collection_names is None or collection_name in collection_names:
namespace, name = collection_name.split('.', 2)
collection_metadata[collection_name] = AnsibleCollectionMetadata(
path=os.path.join(current_base_path, namespace, name),
version=None if version == '*' else version)

return collection_metadata


async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
collection_dir: t.Optional[str],
collection_names: t.Optional[t.List[str]] = None
) -> t.Dict[str, t.Dict[str, t.Any]]:
) -> t.Tuple[
t.Mapping[str, t.Mapping[str, t.Any]],
t.Mapping[str, t.Any]]:
"""
Retrieve information about all of the Ansible Plugins.

Expand All @@ -171,12 +218,14 @@ async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
search path for Ansible.
:arg collection_names: Optional list of collections. If specified, will only collect
information for plugins in these collections.
:returns: A nested directory structure that looks like::
:returns: An tuple. The first component is a nested directory structure that looks like:

plugin_type:
plugin_name: # Includes namespace and collection.
{information from ansible-doc --json. See the ansible-doc documentation for more
info.}
plugin_type:
plugin_name: # Includes namespace and collection.
{information from ansible-doc --json. See the ansible-doc documentation
for more info.}

The second component is a Mapping of collection names to metadata.
"""
flog = mlog.fields(func='get_ansible_plugin_info')
flog.debug('Enter')
Expand Down Expand Up @@ -246,5 +295,8 @@ async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
# done so, we want to then fail by raising one of the exceptions.
raise ParsingError('Parsing of plugins failed')

flog.debug('Retrieving collection metadata')
collection_metadata = get_collection_metadata(venv, env, collection_names)

flog.debug('Leave')
return plugin_map
return (plugin_map, collection_metadata)
26 changes: 17 additions & 9 deletions antsibull/docs_parsing/ansible_internal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from ..logging import log
from ..utils.get_pkg_data import get_antsibull_data
from ..vendored.json_utils import _filter_non_json_lines
from . import _get_environment
from . import _get_environment, AnsibleCollectionMetadata

if t.TYPE_CHECKING:
from ..venv import VenvRunner, FakeVenvRunner
Expand All @@ -23,7 +23,9 @@
async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
collection_dir: t.Optional[str],
collection_names: t.Optional[t.List[str]] = None
) -> t.Dict[str, t.Dict[str, t.Any]]:
) -> t.Tuple[
t.Mapping[str, t.Mapping[str, t.Any]],
t.Mapping[str, t.Any]]:
"""
Retrieve information about all of the Ansible Plugins.

Expand All @@ -33,12 +35,14 @@ async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
search path for Ansible.
:arg collection_names: Optional list of collections. If specified, will only collect
information for plugins in these collections.
:returns: A nested directory structure that looks like::
:returns: An tuple. The first component is a nested directory structure that looks like:

plugin_type:
plugin_name: # Includes namespace and collection.
{information from ansible-doc --json. See the ansible-doc documentation for more
info.}
plugin_type:
plugin_name: # Includes namespace and collection.
{information from ansible-doc --json. See the ansible-doc documentation
for more info.}

The second component is a Mapping of collection names to metadata.
"""
flog = mlog.fields(func='get_ansible_plugin_info')
flog.debug('Enter')
Expand Down Expand Up @@ -72,7 +76,11 @@ async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
plugin_log.fields(error=plugin_data['error']).error(
'Error while extracting documentation. Will not document this plugin.')

# TODO: use result['collections']
collection_metadata = {}
for collection_name, collection_data in result['collections'].items():
collection_metadata[collection_name] = AnsibleCollectionMetadata(
path=collection_data['path'],
version=collection_data.get('version'))

flog.debug('Leave')
return plugin_map
return (plugin_map, collection_metadata)
17 changes: 11 additions & 6 deletions antsibull/docs_parsing/parsing.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@
async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
collection_dir: t.Optional[str],
collection_names: t.Optional[t.List[str]] = None
) -> t.Dict[str, t.Dict[str, t.Any]]:
) -> t.Tuple[
t.Mapping[str, t.Mapping[str, t.Any]],
t.Mapping[str, t.Any]]:
"""
Retrieve information about all of the Ansible Plugins.

Expand All @@ -30,12 +32,15 @@ async def get_ansible_plugin_info(venv: t.Union['VenvRunner', 'FakeVenvRunner'],
search path for Ansible.
:arg collection_names: Optional list of collections. If specified, will only collect
information for plugins in these collections.
:returns: A nested directory structure that looks like::
:returns: An tuple. The first component is a nested directory structure that looks like:

plugin_type:
plugin_name: # Includes namespace and collection.
{information from ansible-doc --json. See the ansible-doc documentation
for more info.}

The second component is a Mapping of collection names to metadata.

plugin_type:
plugin_name: # Includes namespace and collection.
{information from ansible-doc --json. See the ansible-doc documentation for more
info.}
"""
lib_ctx = app_context.lib_ctx.get()

Expand Down
Loading