kadubhumika commented on PR #74330: URL: https://github.com/apache/airflow/pull/74330#issuecomment-6042008216
> Thanks @kadubhumika. Re-reviewed at [5083889](https://github.com/apache/airflow/commit/5083889efd76199751f81078e86c5a769a28cf35). > > Option B looks right: fire and forget keeps caller outlets (including []), warns when outlets are set, and docs say the event means submission rather than a Unity table refresh. The three warn / empty / waiting tests are solid. > > I would request changes on one P1: > > 1. P1 — Use UnityTableIdentity in examples and docs. After [Notify downstream Dags when COPY INTO writes a Unity (Databricks) table #74191](https://github.com/apache/airflow/pull/74191), the supported way to build a Unity outlet is UnityTableIdentity(...).to_asset(), which lowercases host/catalog/schema/table and normalizes the host the same way the connection does. The system example and the SQL / RunNow / SubmitRun docs currently use Asset("databricks://..."). That can drift from COPY INTO and from the Delta sensor identities, and it disagrees with the guidance already on the COPY INTO page. Please switch the examples and docs to UnityTableIdentity, for example: > from airflow.providers.databricks.assets.databricks import UnityTableIdentity > table = UnityTableIdentity( > host="my-workspace.cloud.databricks.com", > catalog="main", > schema="default", > table="my_airflow_table", > ) > outlets=[table.to_asset()] > 2. P2 — Add a blank line before the Explicit assets heading in run_now.rst and submit_run.rst so Sphinx does not glue it to the deferrable paragraph. > 3. P2 — Soften the sync/deferrable line to “task success after remote completion”, and note that authors who need table ready should use wait_for_termination=True or the Delta table sensor ([Databricks: detect Delta table version changes (#74195). #74225](https://github.com/apache/airflow/pull/74225)). Declared outlets still do not prove a new Delta commit. > > Optional: the same warning block is copied in three **init** methods; a tiny shared helper would be fine once you touch that file again. > > CI Static checks failure looks like an unrelated snowflake ruff fix on main (timezone.utc → UTC), same as other open PRs. > > Happy to re-review quickly once the UnityTableIdentity examples land. @zozo123 Sure1 Thank You! -- 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]
