Prepare docs for Jan 2023 wave of Providers #28651
Conversation
eladkal
requested review from
jedcunningham,
mik-laj and
turbaszek
as code owners
boring-cyborg
bot
added
provider:cncf-kubernetes
airflow/providers/imap/CHANGELOG.rst
Outdated
There was a problem hiding this comment.
Is it worth a release just for this change?
There was a problem hiding this comment.
The PR changed several providers I think it's preferred to release all of it at the same wave?
There was a problem hiding this comment.
I'm not sure it matters, I think it'd make more sense to have "meaningful" releases of any given provider.
I think it is ultimately up to the release manager though.
There was a problem hiding this comment.
let me drop it from the release
There was a problem hiding this comment.
I personally would not remove it feom the release.
I usually released all of the "actual" changes. If the release did not make any code modifications it could be marked as "doc only change" - which means that documentation would be updated but the package itself was not released.
With such "small" code changes, it's always on the fence and I think this is just a question of "do we want to mess arround with the same change next time when we release" or "should we release now and forget about it".
But from the release manager point of view - I prefer to release any "potentially breaking changes" as soon as they are merged. Even if they are just "pass" statement removals. Because even smallest code changes might contain completely non-obvious bugs.
The mantra "release early, release often" is very much applicable here. What if you keep on doing it (not releasing this change), then not releasing next similar "innocent" refactor next month for the same provider, and so on. And then releasing 5 such changes in 5 months when there is a bigger change and finding out that one of those "innocent" changes broke something. But in 5 months you not only have to spend time finding out which one broke it but also the person who made the change will completely forgot on why and how or might be not available any more.
By releasing now (and adding the "Status" Issue - you add an extra "peer" pressure on the user who implemented it. It is fresh. The user still remembers it. And more often than not they will actually install the new provider RC and do some testing with it. And even if not, if someone else finds a bug in the newly released provider, you know immediately whom to ask for help. Do you think it will happen if you release 5 similar changes are released together in 5 months? I think it's far, far, far less likely.
I think not relasing now is optimizing the wront thing. By not realising we seem to optimize "number of releases in PyPI". Which is wrong optimization goal IMHO. There is virtually no overhead for maintainers nor for users to release it - because releases are all automated, and MOST users will just get the bundle together with new version of Airflow. And those users who actually care and install new providers when they are relased will actually appreciate if there are many smaller upgrades rather than than one bigger upgrade because it is easier to go back and investigating problem when it happens.
Release (and upgrade) early and often - as counter-intuitive as it is., it gives much better results than release less often.
airflow/providers/sftp/CHANGELOG.rst
Outdated
There was a problem hiding this comment.
We don't typically add typo commits to release notes, and not sure it's worth releasing this set of changes.
There was a problem hiding this comment.
same comment as above :)
There was a problem hiding this comment.
Same comment as above. If there are code changes -> release. Early and often. This is the mantra I will keep on repeating, becaus I think avoiding releases is optimising the wrong thing.
The whole automation about releasing providers was built around that premise and mantra.
airflow/providers/imap/CHANGELOG.rst
Outdated
There was a problem hiding this comment.
I'm not sure it matters, I think it'd make more sense to have "meaningful" releases of any given provider.
I think it is ultimately up to the release manager though.
Just one comment here as well. I think it's also the matter of an agreement and general approach between the release managers when there is more than one. Because it is not only the decision impacting this single release, but it ALSO (and this is more important) impacts future releases of the same provider - i.e.. this change will have to be dealt with eventually. It could be done in the current release (by the current release manager) or in the future release (by the future one). I think any changes like that should be dealt with as soon as possible for the reasons explained above - the more time passes, the more "accumulation" of potential problems occur, the less "blurry" the memory about the changes introduced is and the less likely the contributor of a given change will help. The bad thing is - if you think about future self or future other relase manager is that the more we dely such release, the more difficult it might get to handle and you can either "face it" now or let you (or someone else) "face it" later. There is another angle of it. When you say "we released all there is to relase" as a release manager you say "I've completed the job fully and the next release manager has to care only about future changes". Vs. "ok I let the future release manager (which might be you) handle that in the future.". |
There was a problem hiding this comment.
I'm not sure why the static tests fails on
Checking docs/apache-airflow-providers-ftp/index.rst
The docs/apache-airflow-providers-ftp/index.rst does not contain System Tests TOC.
Make sure to add those lines to docs/apache-airflow-providers-ftp/index.rst:
.. toctree::
:hidden:
:caption: System tests
System Tests <_api/tests/system/providers/ftp/index>
In this PR. I would expect this to present failure on the PR that added the system tests for the provider #26974?
@potiuk is this expected?
in any case this commit should solve the problem
There was a problem hiding this comment.
Do you happen to have a link to the job where it failed (it's green now)?
There was a problem hiding this comment.
Ok. I know the reason.
39f501d added the TOC entry BUT in the section that is automatically generated and when you regenerated it, it was removed in your PR.
There was a problem hiding this comment.
so something is odd with the automation as it should have not been removed?
There was a problem hiding this comment.
Not really. It was just added in a wrong place. But our automation should filter out those automatically added statements and maybe explain it better. PR in a moment.
|
@potiuk are we ok with the common.sql version? it's the last thing to resolve before proceeding |
There was a problem hiding this comment.
All good. I looked at the change and if anything it shoudl be considered as a bug fix. So all good.
BTW. I think I might want to add a bit more requirements for changing the common.sql API - it should always be accompanied with CHANGELOG entry explaining it (something that @jedcunningham has been likely hinting about mentioning newsfragments). We might want to add newsfragments to providers indeed but I would likely want to do it after refactoring providers to the new structure as mentioned in #28292
|
Should be OK now |
3 participants
^ 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.