ansible / ansible-sdk

The Ansible SDK
Apache License 2.0
25 stars 14 forks source link

Docs: Autogenerate API reference content #40

Closed oraNod closed 2 years ago

oraNod commented 2 years ago

This PR uses the immaterial extension with Sphinx to generate API docs.

To do:

webknjaz commented 2 years ago
/home/runner/work/ansible-sdk/ansible-sdk/ansible_sdk.AnsibleJobDef:1: WARNING: Parameter name 'data_dir' does not match any of the parameters defined in the signature: []

Things like this seem to be happening to data classes where the :param: entries are declared in the class docstring, not in __init__() (which is not there). The docs don't mention anything special about the dataclasses, nor do the GitHub issues. I suppose one could file a feature request to infer an initializer signature from the attributes but currently, the workaround could be just documenting those attributes. For example, https://github.com/ansible/ansible-sdk/blob/main/ansible_sdk/model/job_def.py could have a class changed as follows:

 @dataclass(frozen=True, kw_only=True)
 class AnsibleJobDef(_DataclassReplaceMixin):
-    """
-    Common Ansible job definition values
-    :param data_dir: path to a directory structure containing the Ansible project content and inventory.
-    :param playbook: relative path or FQCN of a playbook to run under ``data_dir``
-    :param inventory: relative path to inventory file(s) or directory under ``data_dir``, analogue to Ansible's ``-i`` option
-    :param extra_vars: dictionary of variables for highest precedence level, analogue to Ansible's ``-e`` option
-    :param verbosity: None for default verbosity or 1-5, equivalent to Ansible's ``-v`` options
-     """
+    """Common Ansible job definition values."""
     # currently analogue to runner's private_data_dir; do we want to construct this ourselves?
     data_dir: str
+   """Path to a directory structure containing the Ansible project content and inventory."""
+
     # relative path to playbook in data_dir or FQCN
     playbook: str
+   """Relative path or FQCN of a playbook to run under ``data_dir``."""
+
     inventory: t.Optional[t.Union[str, list[str]]] = None  # FUTURE: high-level inventory types?
+   """Relative path to inventory file(s) or directory under ``data_dir``, analogue to Ansible's ``-i`` option."""
+
     extra_vars: dict[str, t.Any] = field(default_factory=dict)
+   """Dictionary of variables for highest precedence level, analogue to Ansible's ``-e`` option."""
+
     verbosity: t.Optional[int] = None  # None or 1-5
+   """:data:`None` for default verbosity or 1-5, equivalent to Ansible's ``-v`` options."""

The above example uses attribute docstrings (check out PEP 224, PEP 257, PEP 258 for the origins of this approach). Sphinx's apidoc supports them natively.

But there's another syntax that Sphinx supports for documenting attributes — comment-like strings that have a leading #: and are supposed to be on the previous or the same line, unlike attribute docstrings that go on the line following the attribute declaration (https://www.sphinx-doc.org/en/master/usage/extensions/autodoc.html#directive-autoattribute). Here's an example of using this syntax instead:

 @dataclass(frozen=True, kw_only=True)
 class AnsibleJobDef(_DataclassReplaceMixin):
-    """
-    Common Ansible job definition values
-    :param data_dir: path to a directory structure containing the Ansible project content and inventory.
-    :param playbook: relative path or FQCN of a playbook to run under ``data_dir``
-    :param inventory: relative path to inventory file(s) or directory under ``data_dir``, analogue to Ansible's ``-i`` option
-    :param extra_vars: dictionary of variables for highest precedence level, analogue to Ansible's ``-e`` option
-    :param verbosity: None for default verbosity or 1-5, equivalent to Ansible's ``-v`` options
-     """
+    """Common Ansible job definition values."""
     # currently analogue to runner's private_data_dir; do we want to construct this ourselves?
+   #: Path to a directory structure containing the Ansible project content and inventory
     data_dir: str
+
     # relative path to playbook in data_dir or FQCN
+   #: Relative path or FQCN of a playbook to run under ``data_dir``
     playbook: str
+
+   #: Relative path to inventory file(s) or directory under ``data_dir``, analogue to Ansible's ``-i`` option
     inventory: t.Optional[t.Union[str, list[str]]] = None  # FUTURE: high-level inventory types?
+
+   #: Dictionary of variables for highest precedence level, analogue to Ansible's ``-e`` option
     extra_vars: dict[str, t.Any] = field(default_factory=dict)
+
+   #: :data:`None` for default verbosity or 1-5, equivalent to Ansible's ``-v`` options
     verbosity: t.Optional[int] = None  # None or 1-5

I haven't tested these but I'm pretty confident that such a workaround should address the corresponding warnings.

nitzmahone commented 2 years ago

But there's another syntax that Sphinx supports for documenting attributes — comment-like strings that have a leading #:

@webknjaz thanks for that one- I didn't do my research to find this syntax when I added those docstrings, but it sounds like exactly what I was wishing for. I'd much prefer the docstrings to be attribute/property-adjacent like that than having them in a block at the top. We don't need to address it in this PR, but I'd be strongly in favor of doing it that way if it doesn't cause other problems.

webknjaz commented 2 years ago

FWIW I personally tend to prefer attribute docstrings rather than the Sphinx hack with #:.

We don't need to address it in this PR

If the warnings were caused by this change, it'd be reasonable to fix it here. If this is also happening with the current apidoc integration, then definitely in a separate PR. I haven't checked if they're already present on the default branch.

oraNod commented 2 years ago

Update on this PR. I propose that we create two conf.py files and have separate builds. One build for concepts and procedures (docs/source/conf.py). One build for API reference generated from docstrings (docs/api_reference/conf.py). IMO a separate build for the API reference content will make it easier to have a single-source for community and downstream.

I also propose that we fix the remaining warnings in a different PR because they require modifications to the docstrings that will add a lot more changes.

WARNING: Parameter name 'data_dir' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'playbook' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'inventory' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'extra_vars' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'verbosity' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'container_image_ref' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'event' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'event_data' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'stdout' does not match any of the parameters defined in the signature: []
WARNING: Parameter name 'container_image_ref' does not match any of the parameters defined in the signature: []
oraNod commented 2 years ago

Closing this to work on https://github.com/ansible/ansible-sdk/pull/54 instead