-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Add verification that provider docs are as expected #35424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
This one should prevent problems like the ones solved in #35420 |
|
Just wondering is it possible that changes to core documentation released before than provider documentation released? |
ad93078 to
5ea99a8
Compare
Not really - the changes in core are relased in next MINOR release unless we explicitly cherry-pick them |
|
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 a) generate missing documentation Genearally - that one will do all kinds of "let's check and generate consistent docs for new providers". |
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. |
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 |
Sorry miss your comment. Yeah it is good idea to move it there as follow up |
|
And I will also separate out the security.rst change to a separate PR. |
5ea99a8 to
458ffe6
Compare
|
Also separated out removal of empty lines in CHANGELOG #35436 |
458ffe6 to
93b074f
Compare
|
(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. |
93b074f to
3901c4a
Compare
|
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. 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. |
baa2d7f to
bb638cd
Compare
48a3be2 to
38985a9
Compare
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
38985a9 to
d870896
Compare
Taragolis
left a comment
There was a problem hiding this 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
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


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.rstor{issue_number}.significant.rst, in newsfragments.