Skip to content
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

Improve provider documentation and README structure #32125

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Jun 25, 2023

The changelong in provider docs has been part of the README which became part of the page displayed in PyPI as well as the main page displayed when you want to see provider documentation. This was fine when the changelog was short, but with providers history growing, the main page became longer and longer and the generated index on the right side (from the changelog) became less and less usable, and changelog itself became far less discoverable.

This change introduces another approach. Changelog gets its own, separate page and it has quite a few positive consequences:

  1. we can add separate "Changelog" URL in PyPI so that it is
    prominently visible at the TOC in PyPI URLS
  2. we can link to the changelog easily from stable page - without
    worrrying which version we link to
  3. the README file is part of the package so fixing the changelog
    in case of a doc-only mistake will not fix the PyPI page (it
    will only be fixed in future releases)
  4. the link tree on the right side of the provider index page becomes back
    usable
  5. the left-side TOD on the provider's index page containes two
    prominent links that were missing: Home (back to the index provider
    page), Changelog, and Security - all under "Basics" link
  6. The TOC is not repeated in the index page itself, making the content
    immediately visible rather than having to scroll trough the long
    TOC (in some cases like amazon the TOC was reallly long).

^ 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 Jun 25, 2023

cc: @eladkal. @pierrejeambrun, @ephraimbuddy much better provider packages docs:

Main page:

Screenshot 2023-06-25 at 20 45 02

No changelog in the main page:

Screenshot 2023-06-25 at 20 47 31

Separate changelog page:

Screenshot 2023-06-25 at 20 48 52

Seperate security page:

Screenshot 2023-06-25 at 20 49 06

@potiuk
Copy link
Member Author

potiuk commented Jun 25, 2023

Compare to original page:

Screenshot 2023-06-25 at 23 32 50

@potiuk potiuk requested review from pankajkoti, pankajastro and phanikumv and removed request for utkarsharma2 June 25, 2023 21:36
@potiuk potiuk force-pushed the move-changelogs-in-provider-docs branch 2 times, most recently from 2946f87 to e8afb0d Compare June 26, 2023 06:47
@potiuk
Copy link
Member Author

potiuk commented Jun 26, 2023

BTW. Once this is merged I will also add redirection to "index" for historical versions in https://github.com/apache/airflow-site/blob/main/post-docs/add-back-references.py and post-process older versions to contain the redirects.

Copy link
Member

@pankajkoti pankajkoti left a comment

Choose a reason for hiding this comment

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

Nice one, some minor questions/comments inline.

airflow/providers/elasticsearch/__init__.py Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
@potiuk potiuk force-pushed the move-changelogs-in-provider-docs branch from e8afb0d to f7de112 Compare June 26, 2023 10:25
@potiuk potiuk added the allow suspended provider changes Allow changes in suspended providers label Jun 26, 2023
@potiuk potiuk force-pushed the move-changelogs-in-provider-docs branch from f7de112 to 5c2356f Compare June 26, 2023 10:26
@potiuk
Copy link
Member Author

potiuk commented Jun 26, 2023

Applied next round of comments. Also added "allow suspended providers changes" as this one also includes yandex command.

airflow/providers/elasticsearch/provider.yaml Outdated Show resolved Hide resolved
---------

Changelog for ``{{ package_name }}``
{{ '-' * (18 + package_name | length) }}
Copy link
Member

Choose a reason for hiding this comment

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

Nice 👍🏽

I just feel sometimes that GitHub could provide adding emojis to react on lines while reviewing.

@potiuk potiuk force-pushed the move-changelogs-in-provider-docs branch from 5c2356f to 444bdca Compare June 26, 2023 11:27
Comment on lines +19 to +38
Releasing security patches
--------------------------

Airflow providers are released independently from Airflow itself and the information about vulnerabilities
is published separately. You can upgrade providers independently from Airflow itself, following the
instructions found in :doc:`apache-airflow:installation/installing-from-pypi`.

When we release Provider version, the development is always done from the ``main`` branch where we prepare
the next version. The provider uses strict `SemVer <https://semver.org>`_ versioning policy. Depending on
the scope of the change, Provider will get ''MAJOR'' version upgrade when there are
breaking changes, ``MINOR`` version upgrade when there are new features or ``PATCHLEVEL`` version upgrade
when there are only bug fixes (including security bugfixes) - and this is the only version that receives
security fixes by default, so you should upgrade to latest version of the provider if you want to receive
all released security fixes.

The only exception to that rule is when we have a critical security fix and good reason to provide an
out-of-band release for the provider, in which case stakeholders in the provider might decide to cherry-pick
and prepare a branch for an older version of the provider following the
`mixed governance model <https://github.com/apache/airflow/blob/main/PROVIDERS.rst#mixed-governance-model>`_
and requires interested parties to cherry-pick and test the fixes.
Copy link
Contributor

@eladkal eladkal Jun 26, 2023

Choose a reason for hiding this comment

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

This is a repeated text for all the providers.
Maybe we should extract this text to
docs/apache-airflow-providers/security.rst and just share a link here?
The issue is that every edit to this content will force handling doc release for all providers.

Copy link
Member Author

Choose a reason for hiding this comment

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

This was a deliberate choice, actually. I think we want a copy of security page for each provider.

This text will be updated when we will publish SBOM information with links to SBOM information, so the content of this one will be different for every provider anyway (It has no variable part but it will be coming).

It's very similar to those pages:

https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable/installing-providers-from-sources.html

Where links to .sha/downloaded tar.gz are generated automatically when documentation is published, but other than that the content will be the same.

I think it is important to have this information "in-provider" separately for each provider so that you do not have to go "elsewhere" to find the information and follow some "generic" advices - just to link directly to the information you need.

The issue is that every edit to this content will force handling doc release for all providers.

Correct. This is how it should be really, it's handled by "doc-only changes" option - this one will not release provider itself but it will update the information in the latest released version of the provider. And once we add those links (I hope it will be before the next wave) we will have to updated them for all providers anyway (possibly even without actually releasing the providers because SBOM information is "post-processing" the releases - does not have to cause a new provider release. I am actually even planning to retroactively add such generated "security" pages back in all released version of the provider docs so that the user can see the SBOM information even in old versions of the docs.

Copy link
Member Author

@potiuk potiuk Jun 27, 2023

Choose a reason for hiding this comment

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

One more comment - for me this s really a matter of UI/UX - simply speakiing - in documenation, there is often a value when the same information is available directly when you look for it rather than having to do "one-more-click" - we need such security page "per-provider" anyway. We could extract the common part/text to a separate document and link to it of course. But it makes it "one-more-click-away" - i.e many peopke wont make the effort to follow the link.

Copy link
Contributor

@eladkal eladkal Jun 27, 2023

Choose a reason for hiding this comment

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

This was a deliberate choice

this explanation is enough for me :)

The changelong in provider docs has been part of the README which
became part of the page displayed in `PyPI` as well as the main
page displayed when you want to see provider documentation. This
was fine when the changelog was short, but with providers history
growing, the main page became longer and longer and the generated
index on the right side (from the changelog) became less and less
usable, and changelog itself became far less discoverable.

This change introduces another approach. Changelog gets its
own, separate page and it has quite a few positive consequences:

1) we can add separate "Changelog" URL in PyPI so that it is
   prominently visible at the TOC in PyPI URLS
2) we can link to the changelog easily from stable page - without
   worrrying which version we link to
3) the README file is part of the package so fixing the changelog
   in case of a doc-only mistake will not fix the PyPI page (it
   will only be fixed in future releases)
4) the link tree on the right side of the provider index page becomes back
   usable
5) the left-side TOD  on the provider's index page containes two
   prominent links that were missing: Home (back to the index provider
   page), Changelog, and Security - all under "Basics" link
6) The TOC is not repeated in the index page itself, making the content
   immediately visible rather than having to scroll trough the long
   TOC (in some cases like amazon the TOC was reallly long).
@potiuk potiuk force-pushed the move-changelogs-in-provider-docs branch from 444bdca to 873c778 Compare June 27, 2023 12:40
@potiuk potiuk merged commit 09d4718 into apache:main Jun 27, 2023
@potiuk potiuk deleted the move-changelogs-in-provider-docs branch June 27, 2023 17:05
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
allow suspended provider changes Allow changes in suspended providers area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) provider:celery provider:cncf-kubernetes Kubernetes provider related issues provider:common-sql provider:databricks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants