You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "dstandish (via GitHub)" <gi...@apache.org> on 2023/11/21 17:27:43 UTC

[PR] Add semi-automated deprecation removal process [airflow]

dstandish opened a new pull request, #31830:
URL: https://github.com/apache/airflow/pull/31830

   Ok taking another crack at this.
   
   So I think I may have come up with something that would allow folks who are _interested_ in ensuring timely removal of deprecated code without overly burdening the release manager.
   
   I provide a few mechanisms for marking something as deprecated.
   
   If we are emitting a deprecation warning, we can use a wrapper that would document (1) the provider version when the code should be changed, or (2) the airflow version at which which, when it becomes the min airflow version for the provider, the code should be changed, and (3) the user who initiated the deprecation.
   
   My current thinking is the list of "pending deprecations" -- i.e. the deprecations which are due to be updated -- would be maintained by pre-commit.
   
   Then, when release manager runs generate_issue_content_providers, we would add content that would automatically tag the authors and this would give them a chance to enact the deprecations.  It would not block release though.
   
   This is just a draft, but just wanted to share the thought since I picked it back up.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] github-actions[bot] commented on pull request #31830: Add semi-automated deprecation removal process

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1718468762

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1891072052

   And just to clarify 1) is done using `breeze release-management prepare-providers-documentation` command :)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1831110615

   > When you say "task" do you mean add a new breeze command that generates a "todo" issue?
   
   Yes. task to run with breeze. should be easy to add (I can do it) - but we do not have to do it "now" it's more of the process and "who" is doing it I am concerned about. With things like that, you have to consider how much overhead it introduces to someone who is going to remove it , and who that "someone" will be. And even if we do not implement all the tooling that is needed "now" - we should know what is the goal and what is missing to get there.
   
   Adding TODO:s in this way is the first part of the process "how we mark it" but we should also at least decide on what happens with it - i.e., how we remove it. Otherwise it's like a data that you keep by a company but have no idea how to use it. Typical Big-data projects companies have.
   
   I think we need to really start with "how we envision the process of removing the TODOs". The problem with the current proposal is the "moment in time" when those deprecations are collected and explained. What you've proposed in this PR - this collection happens AFTER the documentation is updated, packages (RC1) are already preparead and signed and commited to SVN and release manager is about to send "VOTE" email.
   
   So the "breeze task" that you put it in is just bad idea - because at that moment release manager has already done their job and if all is well, the RCs  will be accepted as the new release.
   
   Maybe you have not realized that and that's why you put it there :). 
   
   My point is that - we need to design "when" in the whole process this information should be collected and how to surface it to people who can do "something" about it - before the new provider's release is actually done.
   
   IMHO. the process should look like:
   
   1) RM prepares a new wave and prepares documentation PR (changelog/commits)- also decides that some of the providers are going to have breaking change
   
   2) The PR is there, and is going to be iterated for a few days (usualy happens) when others are rushing with some "last minute" merges. I think HERE is the right moment to automatically generate an issue with information "we are releasing new breaking providers : A, B, C  and for each provider we have a list of things to be fixed.
   
   During iteration we are likely regenerate the info to track progress of what's still left to do from deprecations.
   
   3) RM merges the doc PR (after merging last fixes and regenerating the changelog/commits documentation including those last minutes changes.
   
   4) Send information to VOTE on the providers
   <-- CURRENT implementation only 
   
   We cannot really put the burden of fixes on the shoulders of RM - we need to give RM some tooling to be able to identify and engage (ideally semi-automatically rather than individuallly finding and pinging the people) those who should fix the deprecations. RM is really supposed to the "mechanics" of release, generally speaking except committing the changelog, and maybe cherry-picking commits (in case of Airflow) RM should not "implement" anything - RM can at most coordinate others implementing PRs (as a general rule).
   
   So IMHO we need to do something to a) generate b) track what's left between 2) and 3) and not as part of 4) as is currently. and seems that having a task to do that in breeze and adding it as part of [release process](https://github.com/apache/airflow/blob/main/dev/README_RELEASE_PROVIDER_PACKAGES.md#prepare-documentation) 
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #31830: Add semi-automated deprecation removal process
URL: https://github.com/apache/airflow/pull/31830


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #31830:
URL: https://github.com/apache/airflow/pull/31830#discussion_r1401250414


##########
dev/breeze/src/airflow_breeze/commands/release_management_commands.py:
##########
@@ -16,22 +16,27 @@
 # under the License.
 from __future__ import annotations
 
+import logging
 import os
 import re
 import shlex
 import shutil
 import sys
 import textwrap
 import time
+from collections import defaultdict
 from copy import deepcopy
 from datetime import datetime
 from pathlib import Path
 from subprocess import DEVNULL
 from typing import IO, Any, Generator, NamedTuple
 
 import click
+import semver
+from github import Github, UnknownObjectException
 from rich.progress import Progress
 from rich.syntax import Syntax
+from tests.test_utils.providers import get_provider_min_airflow_version

Review Comment:
   BTW. Breeze has quite comprehensive and "unitized" set of utils - each group of utils has it's own module in utils. I am from the school where those util classes should be split to many independend small set of closely related utils that can be imported and used independently from each other (avoid all kinds of circular dependencies and diamond relationships.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] github-actions[bot] closed pull request #31830: Add semi-automated deprecation removal process

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #31830: Add semi-automated deprecation removal process
URL: https://github.com/apache/airflow/pull/31830


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on code in PR #31830:
URL: https://github.com/apache/airflow/pull/31830#discussion_r1401245775


##########
dev/breeze/src/airflow_breeze/commands/release_management_commands.py:
##########
@@ -16,22 +16,27 @@
 # under the License.
 from __future__ import annotations
 
+import logging
 import os
 import re
 import shlex
 import shutil
 import sys
 import textwrap
 import time
+from collections import defaultdict
 from copy import deepcopy
 from datetime import datetime
 from pathlib import Path
 from subprocess import DEVNULL
 from typing import IO, Any, Generator, NamedTuple
 
 import click
+import semver
+from github import Github, UnknownObjectException
 from rich.progress import Progress
 from rich.syntax import Syntax
+from tests.test_utils.providers import get_provider_min_airflow_version

Review Comment:
   No imports from airflow in `breeze` sorry :) . Breeze venv is (and should be) separate from Airflow :).
   
   You should use `get_min_airflow_version` from breeze's `utils.packages` 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1821766334

   > What this does is add two standardized "todo" directives that will allow us to essentially set a reminder to make certain code changes at provider release time.
   
   Yes. I love the idea - it's much better than the automated tests you toyed with before.
   
   I think one thing to add is that it should be somewhat flaggable (not sure how yet) to and converted to a TODO list for people to pick while preparing the release. I think having it as part of `release` command is a bit late and puts a lot of effort on the shoulders of release manager. 
   
   Maybe simply a task that produces an issue in Github with all those things to fix that release manager could prepare and share with others at the moment the preparation start - similar to our "Status" issues when we release? It's ratehr easy command to prepare such issue similarly to the release ones based on those todos: (and with your author's comments it could even be automatically added as @dstandish mention (similarly as we do in the "status" issues).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1970144428

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] potiuk commented on pull request #31830: Add semi-automated deprecation removal process

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1586236654

   Looks cool.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1890331636

   @potiuk i think the current PR generates adds the todos to the docs PR -- not the Vote message -- can you double check?  from my reading of your message, it's seems that is where you want it too?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [airflow] github-actions[bot] commented on pull request #31830: Add semi-automated deprecation removal process

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1652711629

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on code in PR #31830:
URL: https://github.com/apache/airflow/pull/31830#discussion_r1402044218


##########
dev/breeze/src/airflow_breeze/commands/release_management_commands.py:
##########
@@ -16,22 +16,27 @@
 # under the License.
 from __future__ import annotations
 
+import logging
 import os
 import re
 import shlex
 import shutil
 import sys
 import textwrap
 import time
+from collections import defaultdict
 from copy import deepcopy
 from datetime import datetime
 from pathlib import Path
 from subprocess import DEVNULL
 from typing import IO, Any, Generator, NamedTuple
 
 import click
+import semver
+from github import Github, UnknownObjectException
 from rich.progress import Progress
 from rich.syntax import Syntax
+from tests.test_utils.providers import get_provider_min_airflow_version

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "dstandish (via GitHub)" <gi...@apache.org>.
dstandish commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1822776940

   > I think one thing to add is that it should be somewhat flaggable (not sure how yet) to and converted to a TODO list for people to pick while preparing the release. I think having it as part of `release` command is a bit late and puts a lot of effort on the shoulders of release manager.
   > 
   > Maybe simply a task that produces an issue in Github with all those things to fix that release manager could prepare and share with others at the moment the preparation start - similar to our "Status" issues when we release? It's ratehr easy command to prepare such issue similarly to the release ones based on those todos: (and with your author's comments it could even be automatically added as @dstandish mention (similarly as we do in the "status" issues).
   
   When you say "task" do you mean add a new breeze command that generates a "todo" issue?
   
   I realize that the release status page is a bit late for this... but I was thinking it would be a reasonable first step as we try this out since it woulld not add any more steps for release manager.... except when someone responds and does these fixes you'd have another RC.  Also, I'm not sure that the versions get updated in the code until this time, so we don't know for sure what the version will be, which is an important part of this.
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Add semi-automated deprecation removal process [airflow]

Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #31830:
URL: https://github.com/apache/airflow/pull/31830#issuecomment-1891071716

   > @potiuk i think the current PR generates adds the todos to the docs PR -- not the Vote message -- can you double check? from my reading of your message, it's seems that is where you want it too?
   
   Nope. It adds the TODOs at the moment where "issue generation" happens. Issue generation happens when the Release candidates are already in SVN and they are about to be announced.
   
   This is https://github.com/apache/airflow/blob/main/dev/README_RELEASE_PROVIDER_PACKAGES.md#prepare-issue-in-github-to-keep-status-of-testing
   
   
   The process looks like follows:
   
   1) RM generates PR with CHANGELOG and version bump for all providers that had merged changes since their last release
   2) This PR can be rebased and updated as the RM makes a "last call" - hey are there any PRs that should be merged
   3) Once this PR is rebased one last time it gets merged (after checking that no new changes were merged)
   4) At this poing RM creates an RC tag and builds packages (from the merged changelog commit)
   5) The next step is that RM creates an issue that summarizes all the issues/prs/changes and ask people to test it
   6) Then RM sends VOTE email with link to that issue
   7) After few days when things get approved, all providers that were not `-1`-ed are relased from that commit from point 3)
   8) All the `-1`-ed providers are moved to next wave (which might be accelerated - only the `-1`ed providers with bugfixes, or regular one - where all providers that got changes after 3) are included ->  this is something I added recently
   
   So what you added is happening at point 5) -> TODOS generated here are far to late to do anything about it, because the issue is about TESTING already released RCs 
   
   What I **think** your change could do is to have a NEW issue (fix those deprecations) happening either after 1) or even before 1) is even attempted.
   
   
   
   
   
   
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org