Repository: apache/airflow
Pull request: Include Selective Check to Ping Maintainer about Legacy WWW and API Changes

<!--
 Licensed to the Apache Software Foundation (ASF) under one
 or more contributor license agreements.  See the NOTICE file
 distributed with this work for additional information
 regarding copyright ownership.  The ASF licenses this file
 to you under the Apache License, Version 2.0 (the
 "License"); you may not use this file except in compliance
 with the License.  You may obtain a copy of the License at

   http://www.apache.org/licenses/LICENSE-2.0

 Unless required by applicable law or agreed to in writing,
 software distributed under the License is distributed on an
 "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
 KIND, either express or implied.  See the License for the
 specific language governing permissions and limitations
 under the License.
 -->

<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.

Feel free to ping committers for the review!

In case of an existing issue, reference it using one of the following:

closes: #ISSUE
related: #ISSUE

How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
Closes: #42031
Include new check in selective_checks.py for checking PR labels to ping maintainers about updating legacy ui or legacy api code piece

<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
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](https://github.com/apache/airflow/tree/main/newsfragments).


Problem statement:
Fail the PR if it modifies old "ui"
In case PR contains changes to the "old" uI ("airflow/www") and the PR does not have the "legacy UI" label set by the maintainer, the PR should fail. This is in order to limit changes that are implemented in the old UI to only bugfixes and necessary changes to enable "new UI" in Airflow 3.0.

This can be easily done via producing the right outputs in Breeze's selective checks.

* Breeze already has features to see if files in the incoming PR have belong to certain "file group" (and we have both "WWW" and "UI" groups defined
* Breeze already has access to labels that are assigned to PR


So it shoudl be as easy as simply exiting with error in selective checks and informing the user `Please ask maintainer to assign the "legacy UI" label to the PR in order to continue`

Hints from issue discussion:
Depends on a consensus in https://lists.apache.org/thread/db69p9fyt34d5tgo8pywpsp8ybjvflry
> Depends on a consensus in https://lists.apache.org/thread/db69p9fyt34d5tgo8pywpsp8ybjvflry

Yes depending on the outcome we might also want to add the `api_connexion` folder as we will also most likely migrate public endpoints to FastAPI and want to also limit contributions in that area.
Deleted an automated comment that looked like fishing, inviting people to an external weird looking URL. We might want to pay attention to that user in case it comes back again on other issues.
Yes. This is something we discuss in security groups (various) - there are currently very active actors with sophisticated methods where GitHub issues and PRs are getting used  example here: https://socket.dev/blog/github-users-targeted-by-new-wave-of-spambots-promoting-malicious-downloads

I will write an email to devlist for people to be aware of it. The best we can do (I just did it) is to report such attempts to Github via "Report" button. This can be done pretty easily and they react rather quickly

Do you think this issue is ready to be picked up? It looks like there is no one against it. Everyone was also informed about the new construction on FastAPI while some endpoints have already moved.
@bugraoz93 Go for it.
Just assigned you @bugraoz93 :)
Awesome, thanks both! :)

Commit message: Include new check in selective_checks.py for checking PR labels to ping maintainers about updating legacy ui or legacy api code piece

Patch to review:
diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
index 3b3408d06cfc2..32852325e33e1 100644
--- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py
+++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
@@ -76,6 +76,8 @@
 FULL_TESTS_NEEDED_LABEL = "full tests needed"
 INCLUDE_SUCCESS_OUTPUTS_LABEL = "include success outputs"
 LATEST_VERSIONS_ONLY_LABEL = "latest versions only"
+LEGACY_UI_LABEL = "legacy ui"
+LEGACY_API_LABEL = "legacy api"
 NON_COMMITTER_BUILD_LABEL = "non committer build"
 UPGRADE_TO_NEWER_DEPENDENCIES_LABEL = "upgrade to newer dependencies"
 USE_PUBLIC_RUNNERS_LABEL = "use public runners"
@@ -102,11 +104,12 @@ class FileGroupForCi(Enum):
     ALWAYS_TESTS_FILES = "always_test_files"
     API_TEST_FILES = "api_test_files"
     API_CODEGEN_FILES = "api_codegen_files"
+    LEGACY_API_FILES = "legacy_api_files"
     HELM_FILES = "helm_files"
     DEPENDENCY_FILES = "dependency_files"
     DOC_FILES = "doc_files"
     UI_FILES = "ui_files"
-    WWW_FILES = "www_files"
+    LEGACY_WWW_FILES = "legacy_www_files"
     SYSTEM_TEST_FILES = "system_tests"
     KUBERNETES_FILES = "kubernetes_files"
     ALL_PYTHON_FILES = "all_python_files"
@@ -157,6 +160,10 @@ def __hash__(self):
             r"^airflow/api_connexion/openapi/v1\.yaml",
             r"^clients/gen",
         ],
+        FileGroupForCi.LEGACY_API_FILES: [
+            r"^airflow/api/",
+            r"^airflow/api_connexion/",
+        ],
         FileGroupForCi.HELM_FILES: [
             r"^chart",
             r"^airflow/kubernetes",
@@ -185,7 +192,7 @@ def __hash__(self):
             r"^airflow/ui/[^/]+\.json$",
             r"^airflow/ui/.*\.lock$",
         ],
-        FileGroupForCi.WWW_FILES: [
+        FileGroupForCi.LEGACY_WWW_FILES: [
             r"^airflow/www/.*\.ts[x]?$",
             r"^airflow/www/.*\.js[x]?$",
             r"^airflow/www/[^/]+\.json$",
@@ -680,7 +687,7 @@ def run_ui_tests(self) -> bool:
 
     @cached_property
     def run_www_tests(self) -> bool:
-        return self._should_be_run(FileGroupForCi.WWW_FILES)
+        return self._should_be_run(FileGroupForCi.LEGACY_WWW_FILES)
 
     @cached_property
     def run_amazon_tests(self) -> bool:
@@ -1055,7 +1062,9 @@ def skip_pre_commits(self) -> str:
             # when full tests are needed, we do not want to skip any checks and we should
             # run all the pre-commits just to be sure everything is ok when some structural changes occurred
             return ",".join(sorted(pre_commits_to_skip))
-        if not self._matching_files(FileGroupForCi.WWW_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES):
+        if not self._matching_files(
+            FileGroupForCi.LEGACY_WWW_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES
+        ):
             pre_commits_to_skip.add("ts-compile-format-lint-www")
         if not self._matching_files(FileGroupForCi.UI_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES):
             pre_commits_to_skip.add("ts-compile-format-lint-ui")
@@ -1354,3 +1363,32 @@ def _is_canary_run(self):
             self._github_event in [GithubEvents.SCHEDULE, GithubEvents.PUSH]
             and self._github_repository == APACHE_AIRFLOW_GITHUB_REPOSITORY
         ) or CANARY_LABEL in self._pr_labels
+
+    @cached_property
+    def is_legacy_ui_api_labeled(self) -> bool:
+        # Selective check for legacy UI/API updates.
+        # It is to ping the maintainer to add the label and make them aware of the changes.
+        if (
+            self._matching_files(
+                FileGroupForCi.LEGACY_API_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES
+            )
+            and LEGACY_API_LABEL not in self._pr_labels
+        ):
+            get_console().print(
+                f"[error]Please ask maintainer to assign "
+                f"the '{LEGACY_API_LABEL}' label to the PR in order to continue"
+            )
+            sys.exit(1)
+        elif (
+            self._matching_files(
+                FileGroupForCi.LEGACY_WWW_FILES, CI_FILE_GROUP_MATCHES, CI_FILE_GROUP_EXCLUDES
+            )
+            and LEGACY_UI_LABEL not in self._pr_labels
+        ):
+            get_console().print(
+                f"[error]Please ask maintainer to assign "
+                f"the '{LEGACY_UI_LABEL}' label to the PR in order to continue"
+            )
+            sys.exit(1)
+        else:
+            return True


Code review comments:
diff_hunk: @@ -157,6 +160,10 @@ def __hash__(self):
             r"^airflow/api_connexion/openapi/v1\.yaml",
             r"^clients/gen",
         ],
+        FileGroupForCi.LEGACY_API_FILES: [
+            r"^airflow/api/",
path: dev/breeze/src/airflow_breeze/utils/selective_checks.py
text: @author:
On a second thought, maybe this should be deleted