jedcunningham commented on code in PR #76329:
URL: https://github.com/apache/airflow/pull/76329#discussion_r4450515467
##########
airflow-core/docs/administration-and-deployment/dag-bundles.rst:
##########
@@ -263,17 +260,15 @@ Some caveats:
Dags are parsed.
* The version and the import-error count together cannot confirm that a
particular commit parsed,
so do not treat the pair as a deployment-health check. Import errors are not
recorded against a
- version, a file whose parse exceeds
+ version, and a file whose parse exceeds
:ref:`config:dag_processor__dag_file_processor_timeout` is killed without
writing an import-error
- row at all, and the count only covers errors you are allowed to read. A
count of zero therefore
- means "nothing I can show you", not "this commit is healthy". To tie a
running Dag to a commit,
- read the Dag's version in ``/api/v2/dags/{dag_id}/dagVersions``.
+ row at all. A count of zero therefore does not mean "this commit is
healthy". To tie a running Dag
+ to a commit, read the Dag's version in ``/api/v2/dags/{dag_id}/dagVersions``.
* With several Dag processors the row reflects whichever refreshed last, as
they share one row per
bundle.
-* A bundle is listed for users who can read at least one Dag recorded against
it. A bundle holding
- no registered Dag at all -- a first deploy whose only file fails to import,
for instance -- has no
- Dag to authorize against, so it is listed only for users who may read import
errors for files with
- no registered Dag (``Admin`` by default under the FAB auth manager).
+* Bundles have their own permission rather than following Dag access, so a
user who holds it sees
+ every file and count in a bundle, whatever Dags they can read. Which users
hold it by default is
+ up to the auth manager.
Review Comment:
Done.
##########
scripts/tests/ci/prek/test_fab_permissions_doc.py:
##########
@@ -0,0 +1,44 @@
+# 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.
+from __future__ import annotations
+
+import sys
+import textwrap
+from pathlib import Path
+from unittest import mock
+
+REPO_ROOT = Path(__file__).resolve().parents[4]
+sys.path.insert(0, str(REPO_ROOT / "scripts" / "ci" / "prek"))
Review Comment:
Removed.
##########
providers/keycloak/tests/unit/keycloak/auth_manager/test_keycloak_auth_manager.py:
##########
@@ -1013,6 +1013,11 @@ def test_is_authorized_view(
("AUDIT_LOGS_ALL", "False", "View#GET"),
("IMPORT_ERRORS", "True", "View#GET"),
("PLUGINS", "True", "View#GET"),
+ *(
+ [("DAG_BUNDLES", "True", "AdminView#GET"), ("DAG_BUNDLES",
"False", "View#GET")]
+ if hasattr(AccessView, "DAG_BUNDLES")
Review Comment:
Done.
##########
airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_dag_bundles.py:
##########
@@ -821,64 +561,45 @@ def test_should_raise_401_unauthenticated(self,
unauthenticated_test_client):
def test_should_raise_403_unauthorized(self, unauthorized_test_client):
assert
unauthorized_test_client.get(f"/dagBundles/{GIT_BUNDLE}/files").status_code ==
403
- def test_lists_files_ordered_by_path(self, admin_client):
- response = admin_client.get(f"/dagBundles/{GIT_BUNDLE}/files")
+ def test_should_raise_403_without_the_bundle_view(self,
no_bundle_view_client):
+ assert
no_bundle_view_client.get(f"/dagBundles/{GIT_BUNDLE}/files").status_code == 403
+
+ def test_lists_files_ordered_by_path(self, test_client):
+ response = test_client.get(f"/dagBundles/{GIT_BUNDLE}/files")
assert response.status_code == 200
body = response.json()
- assert [file["relative_fileloc"] for file in body["dag_bundle_files"]]
== [
- UNREGISTERED_FILE,
- REGISTERED_FILE,
- ]
- assert body["total_entries"] == 2
+ assert [file["relative_fileloc"] for file in body["dag_bundle_files"]]
== GIT_BUNDLE_FILES
+ assert body["total_entries"] == len(GIT_BUNDLE_FILES)
- def test_reports_parse_time_and_duration(self, dag_scoped_client):
- body = dag_scoped_client.get(f"/dagBundles/{GIT_BUNDLE}/files").json()
+ def test_lists_every_file_to_a_caller_who_may_read_no_dag(self,
no_dag_read_client):
+ """The list is the bundle's, so it is not narrowed to the files of
readable Dags."""
+ body = no_dag_read_client.get(f"/dagBundles/{GIT_BUNDLE}/files").json()
by_path = {file["relative_fileloc"]: file for file in
body["dag_bundle_files"]}
- assert by_path[REGISTERED_FILE]["dag_count"] == 1
- assert by_path[REGISTERED_FILE]["last_parsed_time"] ==
"2026-09-10T12:01:00Z"
- assert by_path[REGISTERED_FILE]["last_parse_duration"] ==
PARSE_DURATION
+ assert list(by_path) == GIT_BUNDLE_FILES
+ assert by_path[UNREADABLE_FILE]["dag_count"] == 1
Review Comment:
Renamed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
