Improve documentation on `Params` #20567

MatrixManAtYrService · 2021-12-29T23:36:09Z

I think that this doc could be improved by adding examples of how to reference the params in your dag. (Also, the current example code causes this: #20559.)

While trying to find the right place to work a few reference examples in, I ended up rewriting quite a lot of it.
Let me know if you think that this is an improvement.

I haven't yet figured out how to build this and view it locally, and I'd want to do that as a sanity check before merging it, but I figured get feedback on what I've written before I do that.

potiuk · 2021-12-29T23:46:54Z

I haven't yet figured out how to build this and view it locally, and I'd want to do that as a sanity check before merging it, but I figured get feedback on what I've written before I do that.

./breeze build-docs -- --package-filter apache-airflow

Then ./docs/start_doc_server.sh

You will also see result of building the docs here in CI .

More info here: https://github.com/apache/airflow/blob/main/docs/README.rst

potiuk · 2021-12-30T17:06:36Z

That looks good to me but you need to fix static checks and docs builds .

For static checks pre-commit is your friend :)

kaxil · 2021-12-30T17:24:18Z

Static check failure looks unrelated, but the doc failure is related.

Do rebase on main and build docs again

kaxil · 2021-12-30T17:25:46Z

docs/apache-airflow/concepts/params.rst


 .. code-block::
+   :caption a simple DAG with a parameter


Suggested change

:caption a simple DAG with a parameter

:caption: a simple DAG with a parameter

Check: https://www.sphinx-doc.org/en/1.4.9/markup/code.html#caption-and-name

kaxil · 2021-12-30T17:25:55Z

docs/apache-airflow/concepts/params.rst

+If there's already a dag param with that name, the task-level default will take precedence over the dag-level default.
+
+.. code-block::
+   :caption tasks can have parameters too


kaxil · 2021-12-30T17:26:07Z

docs/apache-airflow/concepts/params.rst

+If templates aren't your style, you can access params in via the context.
+
+.. code-block::
+   :caption use the context kwarg


MatrixManAtYrService · 2021-12-31T16:56:35Z

Thanks for the tips @potiuk

When I do that (on Ubuntu 20.04, in a fresh venv) I get some errors:

#################### Running docs building ####################

apache-airflow : Building documentation
apache-airflow : Running sphinx. The output is hidden until an error occurs.
apache-airflow : Finished docs building with errors
#################### Output for documentation build apache-airflow ####################

apache-airflow : ################################################################################
apache-airflow  [01mRunning Sphinx v3.4.3[39;49;00m
apache-airflow
apache-airflow  Traceback (most recent call last):
apache-airflow    File "/usr/local/lib/python3.8/site-packages/sphinx/registry.py", line 417, in load_extension
apache-airflow      mod = import_module(extname)
apache-airflow    File "/usr/local/lib/python3.8/importlib/__init__.py", line 127, in import_module
apache-airflow      return _bootstrap._gcd_import(name, package, level)
apache-airflow    File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
apache-airflow    File "<frozen importlib._bootstrap>", line 991, in _find_and_load
apache-airflow    File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
apache-airflow    File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
apache-airflow    File "<frozen importlib._bootstrap_external>", line 843, in exec_module
apache-airflow    File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
apache-airflow    File "/opt/airflow/docs/exts/exampleinclude.py", line 31, in <module>
apache-airflow      from sphinx.ext.viewcode import viewcode_anchor
apache-airflow  ImportError: cannot import name 'viewcode_anchor' from 'sphinx.ext.viewcode' (/usr/local/lib/python3.8/site-packages/sphinx/ext/viewcode.py)
apache-airflow
apache-airflow The above exception was the direct cause of the following exception:
apache-airflow
apache-airflow Traceback (most recent call last):
apache-airflow   File "/usr/local/lib/python3.8/site-packages/sphinx/cmd/build.py", line 276, in build_main
apache-airflow     app = Sphinx(args.sourcedir, args.confdir, args.outputdir,
apache-airflow   File "/usr/local/lib/python3.8/site-packages/sphinx/application.py", line 245, in __init__
apache-airflow     self.setup_extension(extension)
apache-airflow   File "/usr/local/lib/python3.8/site-packages/sphinx/application.py", line 402, in setup_extension
apache-airflow     self.registry.load_extension(self, extname)
apache-airflow   File "/usr/local/lib/python3.8/site-packages/sphinx/registry.py", line 420, in load_extension
apache-airflow     raise ExtensionError(__('Could not import extension %s') % extname,
apache-airflow sphinx.errors.ExtensionError: Could not import extension exampleinclude (exception: cannot import name
'viewcode_anchor' from 'sphinx.ext.viewcode' (/usr/local/lib/python3.8/site-packages/sphinx/ext/viewcode.py))
apache-airflow
apache-airflow  [91mExtension error:[39;49;00m
apache-airflow  Could not import extension exampleinclude (exception: cannot import name 'viewcode_anchor' from 'sphinx.ext.viewcode'
(/usr/local/lib/python3.8/site-packages/sphinx/ext/viewcode.py))

After which I can run the server, but I see a 404 error when I navigate here http://localhost:8000/docs/apache-airflow/latest/index.html

It seems to work OK on my Macbook though, so I'll just use that for now (though I'm happy to help toubleshoot the above error if you'd like, I'm just not sure where to start).

potiuk · 2021-12-31T17:17:12Z

When I do that (on Ubuntu 20.04, in a fresh venv) I get some errors:

Did you run this?

./breeze build-docs -- --package-filter apache-airflow

If so - make sure to pull&build the image when asked.

MatrixManAtYrService · 2021-12-31T19:36:32Z

I don't know how I overlooked that prompt (several times). It's working now, thanks.

MatrixManAtYrService · 2022-01-03T16:32:39Z

I think I should've made this a draft PR until I was ready. Sorry about the extra noise.

Anyhow, I think that it is now actually ready for a review.

docs/apache-airflow/concepts/params.rst

kaxil

Awesome, well done @MatrixManAtYrService 🎉

github-actions · 2022-01-04T02:53:23Z

The PR is likely ready to be merged. No tests are needed as no important environment files, nor python files were modified by it. However, committers might decide that full test matrix is needed and add the 'full tests needed' label. Then you should rebase it to the latest main or amend the last commit of the PR, and push it with --force-with-lease.

I think that this doc could be improved by adding examples of how to reference the params in your dag. (Also, the current example code causes this: #20559.) While trying to find the right place to work a few reference examples in, I ended up rewriting quite a lot of it. Let me know if you think that this is an improvement. I haven't yet figured out how to build this and view it locally, and I'd want to do that as a sanity check before merging it, but I figured get feedback on what I've written before I do that. (cherry picked from commit 064efbe)

MatrixManAtYrService requested review from kaxil and potiuk as code owners December 29, 2021 23:36

boring-cyborg bot added the kind:documentation label Dec 29, 2021

potiuk requested review from ephraimbuddy, msumit, BasPH and jedcunningham December 29, 2021 23:49

kaxil reviewed Dec 30, 2021

View reviewed changes

MatrixManAtYrService added 3 commits December 31, 2021 09:42

reworked the params concept doc

becdc22

fixed a clumsy sentance

b4fe600

fixed caption syntax

8c6e990

MatrixManAtYrService force-pushed the params_docs branch from e902022 to 8c6e990 Compare December 31, 2021 16:43

two-backticks for inline code blocks

f94378b

a refactor, now with subsections

7ff2407

eladkal self-requested a review December 31, 2021 20:52

MatrixManAtYrService added 6 commits December 31, 2021 14:30

fixed references

41694c7

just reworking some things

e9dc147

just reworking some things (II)

88945b6

a small reorg

7f3024f

clarify default precedence

2d224f0

aligned code to match prose

d16c7ef

kaxil reviewed Jan 4, 2022

View reviewed changes

docs/apache-airflow/concepts/params.rst Outdated Show resolved Hide resolved

Apply suggestions from code review

0c373be

kaxil approved these changes Jan 4, 2022

View reviewed changes

github-actions bot added the okay to merge It's ok to merge this PR as it does not require more tests label Jan 4, 2022

kaxil changed the title ~~params concept doc rework~~ Improve documentation on Params Jan 4, 2022

kaxil merged commit 064efbe into apache:main Jan 4, 2022

MatrixManAtYrService deleted the params_docs branch January 4, 2022 04:19

potiuk added the type:doc-only Changelog: Doc Only label Jan 23, 2022

potiuk added this to the Airflow 2.2.4 milestone Jan 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve documentation on `Params` #20567

Improve documentation on `Params` #20567

MatrixManAtYrService commented Dec 29, 2021

potiuk commented Dec 29, 2021 •

edited

Loading

potiuk commented Dec 30, 2021

kaxil commented Dec 30, 2021

kaxil Dec 30, 2021

kaxil Dec 30, 2021

kaxil Dec 30, 2021

MatrixManAtYrService commented Dec 31, 2021 •

edited

Loading

potiuk commented Dec 31, 2021

MatrixManAtYrService commented Dec 31, 2021

MatrixManAtYrService commented Jan 3, 2022

kaxil left a comment

github-actions bot commented Jan 4, 2022

	:caption a simple DAG with a parameter
	:caption: a simple DAG with a parameter

Improve documentation on Params #20567

Improve documentation on Params #20567

Conversation

MatrixManAtYrService commented Dec 29, 2021

potiuk commented Dec 29, 2021 • edited Loading

potiuk commented Dec 30, 2021

kaxil commented Dec 30, 2021

kaxil Dec 30, 2021

Choose a reason for hiding this comment

kaxil Dec 30, 2021

Choose a reason for hiding this comment

kaxil Dec 30, 2021

Choose a reason for hiding this comment

MatrixManAtYrService commented Dec 31, 2021 • edited Loading

potiuk commented Dec 31, 2021

MatrixManAtYrService commented Dec 31, 2021

MatrixManAtYrService commented Jan 3, 2022

kaxil left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 4, 2022

Improve documentation on `Params` #20567

Improve documentation on `Params` #20567

potiuk commented Dec 29, 2021 •

edited

Loading

MatrixManAtYrService commented Dec 31, 2021 •

edited

Loading