From a2ceff1517bbb8dbbeb5013742c2897ada3c6662 Mon Sep 17 00:00:00 2001 From: Israel Date: Mon, 11 Sep 2023 10:41:34 -0300 Subject: [PATCH] Generate documentation of private members through sphinx docs (#2831) * Generate documentation of private members through sphinx docs With this commit we make sphinx build API docs for the following things, which were missing up to this point: * `__init__` method of classes; * "private" members (properties, functions, methods, attributes, etc., which name starts with an underscore); * members that are missing a docstring, so we can still reference them with links in the documentation. The third point can be removed later, if we wish, when we reach a point where everything has proper docstrings in the Patroni code base. * Fix documentation problems found after enabling private methods in sphinx * `:cvar:` is not a valid domain role. Replaced with `:attr:`. * documentation for `consul.base.Consul.__init__` has a single backtick quoted string which is interpreted as a reference which cannot be found. Therefore, the docstring has been copied as a block quote. * various list spacing problems and indentation problems. * code blocks added where indentation is interpreted incorrectly * literal string quoting issues. --------- Signed-off-by: Israel Barth Rubio Co-authored-by: Matt Baker --- docs/conf.py | 16 +++++ patroni/api.py | 2 +- patroni/config.py | 2 +- patroni/dcs/consul.py | 30 ++++++++ patroni/postgresql/validator.py | 124 +++++++++++++++++--------------- patroni/validator.py | 84 ++++++++++++---------- 6 files changed, 158 insertions(+), 100 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 2228c72e6..f94b96601 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -54,6 +54,13 @@ apidoc_excluded_paths = excludes apidoc_separate_modules = True +# Include autodoc for all members, including private ones and the ones that are missing a docstring. +autodoc_default_options = { + "members": True, + "undoc-members": True, + "private-members": True, +} + # Add any paths that contain templates here, relative to this directory. templates_path = ['_templates'] @@ -280,6 +287,14 @@ def doctree_read(app, doctree): toc_tree_node['entries'].remove(e) +def autodoc_skip(app, what, name, obj, would_skip, options): + """Include autodoc of ``__init__`` methods, which are skipped by default.""" + if name == "__init__": + return False + return would_skip + + + # A possibility to have an own stylesheet, to add new rules or override existing ones # For the latter case, the CSS specificity of the rules should be higher than the default ones def setup(app): @@ -292,3 +307,4 @@ def setup(app): app.connect('builder-inited', builder_inited) app.connect('env-get-outdated', env_get_outdated) app.connect('doctree-read', doctree_read) + app.connect("autodoc-skip-member", autodoc_skip) diff --git a/patroni/api.py b/patroni/api.py index 4647e7c2f..c53205062 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -1567,7 +1567,7 @@ def __initialize(self, listen: str, ssl_options: Dict[str, Any]) -> None: :param listen: IP and port to bind REST API to. It should be a string in the format ``host:port``, where ``host`` can be a hostname or IP address. It is the value of ``restapi.listen`` setting. :param ssl_options: dictionary that may contain the following keys, depending on what has been configured in - ``restapi` section: + ``restapi`` section: * ``certfile``: path to PEM certificate. If given, will start in HTTPS mode; * ``keyfile``: path to key of ``certfile``; diff --git a/patroni/config.py b/patroni/config.py index 55c8f4bf8..65991b0ed 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -257,7 +257,7 @@ def __init__(self, configfile: str, * file or directory path passed as command-line argument (*configfile*), if it exists and the file or files found in the directory can be parsed (see :meth:`~Config._load_config_path`), otherwise - * YAML file passed via the environment variable (see :cvar:`PATRONI_CONFIG_VARIABLE`), if the referenced + * YAML file passed via the environment variable (see :attr:`PATRONI_CONFIG_VARIABLE`), if the referenced file exists and can be parsed, otherwise * from configuration values defined as environment variables, see :meth:`~Config._build_environment_configuration`. diff --git a/patroni/dcs/consul.py b/patroni/dcs/consul.py index 5dd69c86f..b2a6c4788 100644 --- a/patroni/dcs/consul.py +++ b/patroni/dcs/consul.py @@ -141,6 +141,36 @@ def wrapper(callback: Callable[[Response], Union[bool, Any, Tuple[str, Any]]], p class ConsulClient(base.Consul): def __init__(self, *args: Any, **kwargs: Any) -> None: + """ + Consul client with Patroni customisations. + + .. note:: + + Parameters, *token*, *cert* and *ca_cert* are not passed to the parent class :class:`consul.base.Consul`. + + Original class documentation, + + *token* is an optional ``ACL token``. If supplied it will be used by + default for all requests made with this client session. It's still + possible to override this token by passing a token explicitly for a + request. + + *consistency* sets the consistency mode to use by default for all reads + that support the consistency option. It's still possible to override + this by passing explicitly for a given request. *consistency* can be + either 'default', 'consistent' or 'stale'. + + *dc* is the datacenter that this agent will communicate with. + By default, the datacenter of the host is used. + + *verify* is whether to verify the SSL certificate for HTTPS requests + + *cert* client side certificates for HTTPS requests + + :param args: positional arguments to pass to :class:`consul.base.Consul` + :param kwargs: keyword arguments, with *cert*, *ca_cert* and *token* removed, passed to + :class:`consul.base.Consul` + """ self._cert = kwargs.pop('cert', None) self._ca_cert = kwargs.pop('ca_cert', None) self.token = kwargs.get('token') diff --git a/patroni/postgresql/validator.py b/patroni/postgresql/validator.py index d568fa740..384db08df 100644 --- a/patroni/postgresql/validator.py +++ b/patroni/postgresql/validator.py @@ -290,7 +290,7 @@ def _load_postgres_gucs_validators() -> None: Any problem faced while reading or parsing files will be logged as a ``WARNING`` by the child function, and the corresponding file or validator will be ignored. - By default Patroni only ships the file ``0_postgres.yml``, which contains Community Postgres GUCs validators, but + By default, Patroni only ships the file ``0_postgres.yml``, which contains Community Postgres GUCs validators, but that behavior can be extended. For example: if a vendor wants to add GUC validators to Patroni for covering a custom Postgres build, then they can create their custom YAML files under ``available_parameters`` directory. @@ -300,8 +300,10 @@ def _load_postgres_gucs_validators() -> None: writes them to ``postgresql.conf`` if running PG 12 and above). Then, each of these sections, if specified, may contain one or more attributes with the following structure: + * key: the name of a GUC; * value: a list of validators. Each item in the list must contain a ``type`` attribute, which must be one among: + * ``Bool``; or * ``Integer``; or * ``Real``; or @@ -313,6 +315,7 @@ def _load_postgres_gucs_validators() -> None: class in this module. .. seealso:: + * :class:`Bool`; * :class:`Integer`; * :class:`Real`; @@ -325,61 +328,62 @@ class in this module. This is a sample content for an YAML file based on Postgres GUCs, showing each of the supported types and sections: - ```yaml - parameters: - archive_command: - - type: String - version_from: 90300 - version_till: null - archive_mode: - - type: Bool - version_from: 90300 - version_till: 90500 - - type: EnumBool - version_from: 90500 - version_till: null - possible_values: - - always - archive_timeout: - - type: Integer - version_from: 90300 - version_till: null - min_val: 0 - max_val: 1073741823 - unit: s - autovacuum_vacuum_cost_delay: - - type: Integer - version_from: 90300 - version_till: 120000 - min_val: -1 - max_val: 100 - unit: ms - - type: Real - version_from: 120000 - version_till: null - min_val: -1 - max_val: 100 - unit: ms - client_min_messages: - - type: Enum - version_from: 90300 - version_till: null - possible_values: - - debug5 - - debug4 - - debug3 - - debug2 - - debug1 - - log - - notice - - warning - - error - recovery_parameters: - archive_cleanup_command: - - type: String - version_from: 90300 - version_till: null - ``` + .. code-block:: yaml + + parameters: + archive_command: + - type: String + version_from: 90300 + version_till: null + archive_mode: + - type: Bool + version_from: 90300 + version_till: 90500 + - type: EnumBool + version_from: 90500 + version_till: null + possible_values: + - always + archive_timeout: + - type: Integer + version_from: 90300 + version_till: null + min_val: 0 + max_val: 1073741823 + unit: s + autovacuum_vacuum_cost_delay: + - type: Integer + version_from: 90300 + version_till: 120000 + min_val: -1 + max_val: 100 + unit: ms + - type: Real + version_from: 120000 + version_till: null + min_val: -1 + max_val: 100 + unit: ms + client_min_messages: + - type: Enum + version_from: 90300 + version_till: null + possible_values: + - debug5 + - debug4 + - debug3 + - debug2 + - debug1 + - log + - notice + - warning + - error + recovery_parameters: + archive_cleanup_command: + - type: String + version_from: 90300 + version_till: null + """ conf_dir = os.path.join( os.path.dirname(os.path.abspath(__file__)), @@ -434,13 +438,15 @@ def _transform_parameter_value(validators: MutableMapping[str, Tuple[_Transforma :param value: value of the Postgres GUC. :param available_gucs: a set of all GUCs available in Postgres *version*. Each item is the name of a Postgres GUC. Used for a couple purposes: + * Disallow writing GUCs to ``postgresql.conf`` (or ``recovery.conf``) that does not exist in Postgres *version*; * Avoid ignoring GUC *name* if it does not have a validator in *validators*, but is a valid GUC in Postgres - *version*. + *version*. :returns: the return value may be one among: - * *value* transformed to the expected format for GUC *name* in Postgres *version*, if *name* is present in - *available_gucs* and has a validator in *validators* for the corresponding Postgres *version*; or + + * *value* transformed to the expected format for GUC *name* in Postgres *version*, if *name* is present + in *available_gucs* and has a validator in *validators* for the corresponding Postgres *version*; or * The own *value* if *name* is present in *available_gucs* but not in *validators*; or * ``None`` if *name* is not present in *available_gucs*. """ diff --git a/patroni/validator.py b/patroni/validator.py index ddfb2c071..bc308f71a 100644 --- a/patroni/validator.py +++ b/patroni/validator.py @@ -333,15 +333,17 @@ def __init__(self, schema: Dict[str, Any]) -> None: """Create a :class:`Case` object. :param schema: the schema for validating a set of attributes that may be available in the configuration. - Each key is the configuration that is available in a given scope and that should be validated, and the - related value is the validation function or expected type. + Each key is the configuration that is available in a given scope and that should be validated, + and the related value is the validation function or expected type. :Example: - Case({ - "host": validate_host_port, - "url": str, - }) + .. code-block:: python + + Case({ + "host": validate_host_port, + "url": str, + }) That will check that ``host`` configuration, if given, is valid based on :func:`validate_host_port`, and will also check that ``url`` configuration, if given, is a ``str`` instance. @@ -363,14 +365,16 @@ def __init__(self, *args: Any) -> None: :Example: - Or("host", "hosts"): Case({ - "host": validate_host_port, - "hosts": Or(comma_separated_host_port, [validate_host_port]), - }) + .. code-block:: python + + Or("host", "hosts"): Case({ + "host": validate_host_port, + "hosts": Or(comma_separated_host_port, [validate_host_port]), + }) - The outer :class:`Or` is used to define that ``host`` and ``hosts`` are possible options in this scope. - The inner :class`Or` in the ``hosts`` key value is used to define that ``hosts`` option is valid if either of - :func:`comma_separated_host_port` or :func:`validate_host_port` succeed to validate it. + The outer :class:`Or` is used to define that ``host`` and ``hosts`` are possible options in this scope. + The inner :class`Or` in the ``hosts`` key value is used to define that ``hosts`` option is valid if either + of :func:`comma_separated_host_port` or :func:`validate_host_port` succeed to validate it. """ self.args = args @@ -535,32 +539,34 @@ def __init__(self, validator: Any) -> None: :Example: - Schema({ - "application_name": str, - "bind": { - "host": validate_host, - "port": int, - }, - "aliases": [str], - Optional("data_directory"): "/var/lib/myapp", - Or("log_to_file", "log_to_db"): Case({ - "log_to_file": bool, - "log_to_db": bool, - }), - "version": Or(int, float), - }) - - This sample schema defines that your YAML configuration follows these rules: - - * It must contain an ``application_name`` entry which value should be a :class:`str` instance; - * It must contain a ``bind.host`` entry which value should be valid as per function ``validate_host``; - * It must contain a ``bind.port`` entry which value should be an :class:`int` instance; - * It must contain a ``aliases`` entry which value should be a :class:`list` of :class:`str` instances; - * It may optionally contain a ``data_directory`` entry, with a value which should be a string; - * It must contain at least one of ``log_to_file`` or ``log_to_db``, with a value which should be a - :class:`bool` instance; - * It must contain a ``version`` entry which value should be either an :class:`int` or a :class:`float` - instance. + .. code-block:: python + + Schema({ + "application_name": str, + "bind": { + "host": validate_host, + "port": int, + }, + "aliases": [str], + Optional("data_directory"): "/var/lib/myapp", + Or("log_to_file", "log_to_db"): Case({ + "log_to_file": bool, + "log_to_db": bool, + }), + "version": Or(int, float), + }) + + This sample schema defines that your YAML configuration follows these rules: + + * It must contain an ``application_name`` entry which value should be a :class:`str` instance; + * It must contain a ``bind.host`` entry which value should be valid as per function ``validate_host``; + * It must contain a ``bind.port`` entry which value should be an :class:`int` instance; + * It must contain a ``aliases`` entry which value should be a :class:`list` of :class:`str` instances; + * It may optionally contain a ``data_directory`` entry, with a value which should be a string; + * It must contain at least one of ``log_to_file`` or ``log_to_db``, with a value which should be a + :class:`bool` instance; + * It must contain a ``version`` entry which value should be either an :class:`int` or a :class:`float` + instance. """ self.validator = validator