ephraimbuddy commented on code in PR #76188:
URL: https://github.com/apache/airflow/pull/76188#discussion_r4455507492


##########
airflow-core/tests/unit/dag_processing/bundles/test_dag_bundle_manager.py:
##########
@@ -621,6 +629,214 @@ def test_multiple_bundles_one_fails(clear_db, session):
         assert bundle_names == {"my-test-bundle"}
 
 
+class BundleReadingConnection(BasicBundle):
+    """
+    Test bundle that reads a connection from the metadata database.
+
+    It reads the connection in its constructor, as ``GitDagBundle`` does, or 
when it builds
+    its URL template, as ``WasbDagBundle`` does.
+    """

Review Comment:
   ```suggestion
       """Read a connection during construction or URL-template generation."""
   ```



##########
airflow-core/src/airflow/dag_processing/bundles/manager.py:
##########
@@ -387,6 +387,20 @@ def _extract_and_sign_template(bundle_name: str) -> 
tuple[str | None, dict]:
                     self.log.debug("Signed URL template for bundle %s", 
bundle_name)
             return new_template_, new_params_
 
+        # _extract_and_sign_template can read a connection from the metadata 
database. For example,
+        # GitDagBundle reads its connection in its constructor, and 
S3DagBundle and WasbDagBundle read their
+        # connections in view_url_template(). Each of these reads goes through
+        # MetastoreBackend.get_connection, whose @provide_session wrapper gets 
the scoped session that
+        # sync_bundles_to_db uses and closes that session after the read. 
Closing the scoped session detaches
+        # every row that the session loaded. A write to a detached row is lost 
without an error. So call
+        # _extract_and_sign_template for every bundle before loading any row.

Review Comment:
   ```suggestion
           # Bundle initialization and template generation can close the shared 
session,
           # detaching loaded rows. Complete them before loading rows to update.
   ```



##########
airflow-core/src/airflow/dag_processing/bundles/manager.py:
##########
@@ -400,13 +414,18 @@ def _extract_and_sign_template(bundle_name: str) -> 
tuple[str | None, dict]:
                 if not team:
                     raise _bundle_item_exc(f"Team '{config.team_name}' does 
not exist")
 
-            try:
-                new_template, new_params = _extract_and_sign_template(name)
-            except Exception as e:
-                self.log.exception("Error creating bundle '%s': %s", name, e)
+            bundle = stored.pop(name, None)
+            if name in templates:
+                new_template, new_params = templates[name]
+            elif bundle:
+                # Creating this bundle or building its URL template failed, so 
keep the stored URL fields.
+                # The bundle is still in the config, so the code below sets 
its row to active and applies
+                # the team from the config.

Review Comment:
   ```suggestion
                   # Template failure does not mean the bundle was removed from 
configuration.
   ```



-- 
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]

Reply via email to