You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/03/17 06:35:52 UTC
[GitHub] [airflow] uranusjr opened a new pull request #22334: Add fk between xcom and task instance
uranusjr opened a new pull request #22334:
URL: https://github.com/apache/airflow/pull/22334
As discussed previously.
Also improved the dag_run relationship to use the dag_run_id field directly because why not?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #22334: Add fk between xcom and task instance
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #22334:
URL: https://github.com/apache/airflow/pull/22334#issuecomment-1071199592
Alright hopefully this fixes everything. Mostly changing the test code, but I also needed to change how `airflow task test` works conceptually to make this foreign key work (see the commit referencing this command). Currently `airflow task test` would create a “fake” DAG run in-memory to run just that task, but this does not work well with database-level foreign key. So instead I make this command always create a “real” DAG run in the database, and delete it (with everything the run generates because `ON DELETE CASCADE`) after the task is run. Personally I think this is better because now this command is no longer a weird outlier on database integrity, but not sure if this is an acceptable semantic change.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #22334: Add fk between xcom and task instance
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #22334:
URL: https://github.com/apache/airflow/pull/22334#issuecomment-1072252614
Some good to me
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr merged pull request #22334: Add fk between xcom and task instance
Posted by GitBox <gi...@apache.org>.
uranusjr merged pull request #22334:
URL: https://github.com/apache/airflow/pull/22334
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #22334: Add fk between xcom and task instance
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #22334:
URL: https://github.com/apache/airflow/pull/22334#issuecomment-1070513951
Eh, need to fix tests now though, we are currently doing a lot of dangling inserts.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #22334: Add fk between xcom and task instance
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #22334:
URL: https://github.com/apache/airflow/pull/22334#issuecomment-1070513060
Forgot to mention, we can do this directly because the prior `select` in the migration already filtered out all `xcom` rows without a matching `dag_run` row, and since `task_instance` already has a foreign key to `dag_run`, it is not possible to have dangling `xcom` rows when we add the fk constraint in the migration.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #22334: Add fk between xcom and task instance
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #22334:
URL: https://github.com/apache/airflow/pull/22334#issuecomment-1072252614
Some good to me
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #22334: Add fk between xcom and task instance
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22334:
URL: https://github.com/apache/airflow/pull/22334#issuecomment-1070390295
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #22334: Add fk between xcom and task instance
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #22334:
URL: https://github.com/apache/airflow/pull/22334#issuecomment-1071199592
Alright hopefully this fixes everything. Mostly changing the test code, but I also needed to change how `airflow task test` works conceptually to make this foreign key work (see the commit referencing this command). Currently `airflow task test` would create a “fake” DAG run in-memory to run just that task, but this does not work well with database-level foreign key. So instead I make this command always create a “real” DAG run in the database, and delete it (with everything the run generates because `ON DELETE CASCADE`) after the task is run. Personally I think this is better because now this command is no longer a weird outlier on database integrity, but not sure if this is an acceptable semantic change.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org