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/02/21 10:09:36 UTC
[GitHub] [airflow] uranusjr opened a new pull request #21710: Add map_index to XCom's primary key and interface
uranusjr opened a new pull request #21710:
URL: https://github.com/apache/airflow/pull/21710
Since the latest XCom migration has not been publicly released, this
modifies that migration in place so we don't need to recreate the XCom
primary key *again*.
While I added the map_index argument to the functions, I'm honestly not
sure how it can be used for most of the interfaces. Those can hopefully
be added incrementally later.
--
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 #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #21710:
URL: https://github.com/apache/airflow/pull/21710#issuecomment-1047443298
Ah, adding `map_index` requires us to change the XCom interface (because the push/pull/clear interface now needs to accept `map_index` as an argument). This will need the patch-outdated treatment introduced to `serialize` in #19505.
--
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 #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #21710:
URL: https://github.com/apache/airflow/pull/21710#issuecomment-1060383994
Easier to redo this one top of main instead of rebasing.
--
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 a change in pull request #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21710:
URL: https://github.com/apache/airflow/pull/21710#discussion_r811810618
##########
File path: airflow/models/xcom.py
##########
@@ -108,6 +111,7 @@ def set(
dag_id: str,
task_id: str,
run_id: str,
+ map_index: int = -1,
Review comment:
I wonder if instead of all these columns we should take a `Union[TaskInstance, TaskInstanceKey]`?
--
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 #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #21710:
URL: https://github.com/apache/airflow/pull/21710#issuecomment-1054204468
Decided to fix up the Operator links first, so this depends on https://github.com/apache/airflow/pull/21798
--
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 a change in pull request #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #21710:
URL: https://github.com/apache/airflow/pull/21710#discussion_r811812901
##########
File path: airflow/models/xcom.py
##########
@@ -108,6 +111,7 @@ def set(
dag_id: str,
task_id: str,
run_id: str,
+ map_index: int = -1,
Review comment:
I was wondering about just `TaskInstanceKey`; it’s quite trivial to build a key from a ti anyway.
--
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 a change in pull request #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21710:
URL: https://github.com/apache/airflow/pull/21710#discussion_r813326423
##########
File path: airflow/models/xcom.py
##########
@@ -108,6 +111,7 @@ def set(
dag_id: str,
task_id: str,
run_id: str,
+ map_index: int = -1,
Review comment:
It's `ti.key` infact :)
--
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 a change in pull request #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21710:
URL: https://github.com/apache/airflow/pull/21710#discussion_r813976386
##########
File path: airflow/models/xcom.py
##########
@@ -108,6 +111,7 @@ def set(
dag_id: str,
task_id: str,
run_id: str,
+ map_index: int = -1,
Review comment:
Hmmm, I've started making this change and I'm not too sure about it. Will continue for now.
--
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 closed pull request #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
uranusjr closed pull request #21710:
URL: https://github.com/apache/airflow/pull/21710
--
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 #21710: Add map_index to XCom's primary key and interface
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21710:
URL: https://github.com/apache/airflow/pull/21710#issuecomment-1047665268
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