Skip to content

Conversation

@potiuk
Copy link
Member

@potiuk potiuk commented Nov 3, 2023

Validate that documentation in provider files is properly added
and adds missing documentation if not there.

The provider documentation for all community providers follows
the same structure. This PR adds pre-commit that verifies that
some common structure for all provider documentation is in place
and is properly indexed. Since we are adding a lot of providers
now, it's worth making a common pre-commit that will verify, and
if needed update and regenerate the content to follow the same
patterns.

This PR also changes security.rst to be included content rather
than one generated during release. Initially this file was
supposed to be generated at release time and contain generated
dynamic information, but we will change the approach to be similar
to the one in configuration and CLI where the dynamic content
is dynamically generated via sphinx directives in the included
file.

It also turned out that Kubernetes and AWS executors were missing
and entry for those in provider.yaml file - which made them
not discoverable via "airflow providers executors" CLI - this is also
verified now.

Needs #35435 and #35436 (only last commit should remain).


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@potiuk
Copy link
Member Author

potiuk commented Nov 3, 2023

This one should prevent problems like the ones solved in #35420

@Taragolis
Copy link
Contributor

Just wondering is it possible that changes to core documentation released before than provider documentation released?

@potiuk potiuk force-pushed the add-providers-docs-verification branch from ad93078 to 5ea99a8 Compare November 4, 2023 09:38
@potiuk potiuk requested a review from o-nikolas as a code owner November 4, 2023 09:38
@potiuk potiuk changed the title Verify that provider docs are as expected Add verificationy that provider docs are as expected Nov 4, 2023
@potiuk
Copy link
Member Author

potiuk commented Nov 4, 2023

Just wondering is it possible that changes to core documentation released before than provider documentation released?

Not really - the changes in core are relased in next MINOR release unless we explicitly cherry-pick them

@potiuk
Copy link
Member Author

potiuk commented Nov 4, 2023

BTW. I heavily extended this one and now it performs much more validation + it fixes the problem that security.rst was actually needlessly copied over in all providers.

@utkarsharma2 @pankajastro @kaxil @pankajkoti -> this one should help (afte merging and rebasing) in making sure that the provider documentation is as expected for all the providers you developed.

cc: @bolkedebruin -> we were talking about it when you added common-io. Rather than "templating" those separately - I added a pre-commit that will:

a) generate missing documentation
b) check if the index entries are added
c) point out if executors are missing in provider.yaml (cc: @o-nikolas @vincbeck - it was missing for ECS but also it was missing for Kubernetes)
d) if CLI-ref is defined, it will check that it is referred properly from Airlfow
e) if configs are defined, it will check that they are referred properly from Airflow

Genearally - that one will do all kinds of "let's check and generate consistent docs for new providers".

@potiuk potiuk requested a review from bolkedebruin November 4, 2023 09:45
@potiuk
Copy link
Member Author

potiuk commented Nov 4, 2023

Just wondering is it possible that changes to core documentation released before than provider documentation released?

Not really - the changes in core are relased in next MINOR release unless we explicitly cherry-pick them

But yes @Taragolis -> you are absolutely right - both CLI and CONFIG index of provider-specific configurations/CLI should be moved to https://airflow.apache.org/docs/apache-airflow-providers/index.html (and Airflow docs should point there). This is how we do with everything else that comes from multiple providers.

I will do it as a follow-up once this one is merged.

@Taragolis
Copy link
Contributor

Just wondering is it possible that changes to core documentation released before than provider documentation released?

Not really - the changes in core are relased in next MINOR release unless we explicitly cherry-pick them

No sure how it could prevent situation that it released in core before then in providers. As far as I remember we had the same problem when annotations for task decorators, task.kubernetes released in core (2.4.0) before then it k8s provider released

So I assume some kind of race condition between providers/core could be exists here, it is not critical since need to wait a bit and 404 would be replaced by actual page

@Taragolis
Copy link
Contributor

Just wondering is it possible that changes to core documentation released before than provider documentation released?

Not really - the changes in core are relased in next MINOR release unless we explicitly cherry-pick them

But yes @Taragolis -> you are absolutely right - both CLI and CONFIG index of provider-specific configurations/CLI should be moved to https://airflow.apache.org/docs/apache-airflow-providers/index.html (and Airflow docs should point there). This is how we do with everything else that comes from multiple providers.

I will do it as a follow-up once this one is merged.

Sorry miss your comment. Yeah it is good idea to move it there as follow up

@potiuk
Copy link
Member Author

potiuk commented Nov 4, 2023

And I will also separate out the security.rst change to a separate PR.

@potiuk potiuk changed the title Add verificationy that provider docs are as expected Add verification that provider docs are as expected Nov 4, 2023
@potiuk potiuk force-pushed the add-providers-docs-verification branch from 5ea99a8 to 458ffe6 Compare November 4, 2023 10:30
@potiuk
Copy link
Member Author

potiuk commented Nov 4, 2023

Also separated out removal of empty lines in CHANGELOG #35436

@potiuk potiuk force-pushed the add-providers-docs-verification branch from 458ffe6 to 93b074f Compare November 4, 2023 10:40
@potiuk
Copy link
Member Author

potiuk commented Nov 4, 2023

(now when I separated those two - changelog/security, I can actually modify the last commit to move the links to providers - it will be small enough to fit it in.

@potiuk potiuk force-pushed the add-providers-docs-verification branch from 93b074f to 3901c4a Compare November 4, 2023 11:08
@potiuk
Copy link
Member Author

potiuk commented Nov 4, 2023

OK @Taragolis . Removed the links to configuration from "airflow" and replaced them with the link to

https://airflow.apache.org/docs/apache-airflow-providers/core-extensions/configurations.html

Also made that page a little nicer by adding "human" names of the providers.

image

I decided to leave the "CLI" links in "apache-airflow" - those CLI extensions are very limited and I doubt we will ever have more CLIs than "celery" or "kubernetes" and I thin it's worth to keep direct links to those two - if only for historical reasons.

@potiuk potiuk force-pushed the add-providers-docs-verification branch 2 times, most recently from baa2d7f to bb638cd Compare November 4, 2023 11:20
@potiuk
Copy link
Member Author

potiuk commented Nov 4, 2023

The new "Airflow" configuration page is also slightly improved with discoverabiility that there are actually other options in providers:

image

@potiuk potiuk force-pushed the add-providers-docs-verification branch 3 times, most recently from 48a3be2 to 38985a9 Compare November 4, 2023 11:24
Validate that documentation in provider files is properly added
and adds missing documentation if not there.

The provider documentation for all community providers follows
the same structure. This PR adds pre-commit that verifies that
some common structure for all provider documentation is in place
and is properly indexed. Since we are adding a lot of providers
now, it's worth making a common pre-commit that will verify, and
if needed update and regenerate the content to follow the same
patterns.

It also turned out that Kubernetes and AWS executors were missing
and entry for those in provider.yaml file - which made them
not discoverable via "airflow providers executors" CLI - this is also
verified now.

List of providers that are contributing their configuration is
removed from Airflow - instead the documentation points to the
common "providers" documentation where list of packages and links
to their configuration are generated automatically.

https://airflow.apache.org/docs/apache-airflow-providers/core-extensions/configurations.html
@potiuk potiuk force-pushed the add-providers-docs-verification branch from 38985a9 to d870896 Compare November 4, 2023 11:38
@potiuk potiuk mentioned this pull request Nov 4, 2023
Copy link
Contributor

@Taragolis Taragolis left a comment

Choose a reason for hiding this comment

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

Looks nice.

I've also build docs locally

@potiuk potiuk merged commit a61da3c into apache:main Nov 4, 2023
@potiuk potiuk deleted the add-providers-docs-verification branch November 4, 2023 13:01
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Nov 10, 2023
Validate that documentation in provider files is properly added
and adds missing documentation if not there.

The provider documentation for all community providers follows
the same structure. This PR adds pre-commit that verifies that
some common structure for all provider documentation is in place
and is properly indexed. Since we are adding a lot of providers
now, it's worth making a common pre-commit that will verify, and
if needed update and regenerate the content to follow the same
patterns.

It also turned out that Kubernetes and AWS executors were missing
and entry for those in provider.yaml file - which made them
not discoverable via "airflow providers executors" CLI - this is also
verified now.

List of providers that are contributing their configuration is
removed from Airflow - instead the documentation points to the
common "providers" documentation where list of packages and links
to their configuration are generated automatically.

https://airflow.apache.org/docs/apache-airflow-providers/core-extensions/configurations.html
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:dev-tools changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) kind:documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants