nathadfield commented on code in PR #67285:
URL: https://github.com/apache/airflow/pull/67285#discussion_r3374405844
##########
airflow-core/docs/migrations-ref.rst:
##########
@@ -39,7 +39,10 @@ Here's the list of all the Database Migrations that are
executed via when you ru
+-------------------------+------------------+-------------------+--------------------------------------------------------------+
| Revision ID | Revises ID | Airflow Version | Description
|
+=========================+==================+===================+==============================================================+
-| ``9ff64e1c35d3`` (head) | ``dd5f3a8e2b91`` | ``3.3.0`` | Add indexes
on dag_run.created_dag_version_id and |
+| ``d2f4e1b3c5a7`` (head) | ``9ff64e1c35d3`` | ``3.3.0`` | Add
partition_date to asset_event and |
Review Comment:
Makes sense, happy to drop the column. One refinement on the re-derivation:
`register_asset_change` already receives the producer's `partition_date` as a
parameter (from `ti.dag_run.partition_date`); it just isn't threaded into the
queue helpers right now, which read it back off `event.partition_date`. Since
the APDR is created synchronously in that same call, I can thread the param
through `_queue_dagruns` -> `_queue_partitioned_dags` into
`_compute_target_partition_date` and skip the DagRun lookup entirely. That
gives the IdentityMapper source date with no column and no extra query, and
`AssetEventResponse.partition_date` goes away as you suggested (the pruned-run
case returns `None`). Sound good? If so I'll push that.
--
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]
