saltstack / saltstack/salt

[DOCS] Inconsistent and incomplete documentation around returners

Open
#62,017 1 comment 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

docstring-update documentation help-wanted module doc standards Returners severity-high time-estimate-sprint
Dominant language
Python
Stars
15.7k
Forks
5.6k
Avg merge
2d 44m
Merged PRs (30d)
80

Description

There are (I think) three global options around returners - return, return_config, and return_kwargs.

https://github.com/saltstack/salt/blob/master/doc/ref/cli/salt.rst
Only documents return.

https://github.com/saltstack/salt/blob/master/salt/utils/parsers.py
Documents return, return_config, and return_kwargs. However, the return_config is incorrectly a duplicate of return.

https://github.com/saltstack/salt/blob/master/doc/topics/jobs/index.rst#scheduler-with-returner
Only mentions returner, by example. Also mentions schedule_returner without further details.

https://github.com/saltstack/salt/blob/master/salt/modules/schedule.py
Does not document any of the above, but the code lists them in SCHEDULE_CONF.

https://github.com/saltstack/salt/blob/master/salt/states/schedule.py
Has an example with returner, return_config, and return_kwargs, but does not explain them. There's some implication that return_config refers to a named section somewhere.

https://github.com/saltstack/salt/blob/master/doc/ref/returners/index.rst
Only mentions return, by example.

https://github.com/saltstack/salt/blob/master/salt/returners/__init__.py
From looking at existing returners, get_returner_options appears to be how you are supposed to access the config and kwargs, but doesn't actually mention them. The documentation it does have appears to have multiple errors:

  • virtualname: the __virtual__() function often returns either True, or (False, msg), not the value of __virtualname__. This may be a wider problem of modules not implementing __virtual__() correctly.
  • ret: the "if" / "if not" cases are identical
  • attrs: from reading the code this is actually supposed to be a dict, and it's unclear what the values are supposed to be
  • profile_attr: "the corresponding key of __salt__" doesn't make sense. __salt__ is the API loader, not a config structure
  • profile_attrs: "... a value is read in the are used to fetch a value ..." - a chunk seems to be missing from the middle of this thought
  • defaults: isn't documented

In short, if you're trying to write or use a returner, it's almost impossible to find how these options are supposed to work.
I think the return_config and return_kwargs are actually passed to the returner function via ret["ret_config"] and ret["ret_kwargs"]?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by comparing the returner options and documentation in doc/ref/cli/salt.rst, doc/topics/jobs/index.rst, doc/ref/returners/index.rst, salt/utils/parsers.py, salt/modules/schedule.py, salt/states/schedule.py, and salt/returners/init.py. Trace get_returner_options and the ret fields to establish the intended behavior. Done means the options and returner API are documented consistently, with the listed inaccuracies corrected.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
documentation
Issue type
Documentation
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.