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/03 11:56:46 UTC
[GitHub] [airflow] norm opened a new pull request #21965: WIP Task instance endpoint
norm opened a new pull request #21965:
URL: https://github.com/apache/airflow/pull/21965
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
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] ephraimbuddy commented on a change in pull request #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r820142225
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -134,6 +134,7 @@ info:
| v2.0 | Initial release |
| v2.0.2 | Added /plugins endpoint |
| v2.1 | New providers endpoint |
+ | v2.3.0 | New mapped task instance endpoint |
Review comment:
Problem I see is how easy it is to forget to update it every time we make changes. And a pre-commit for it is almost impossible (not sure)
--
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 #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#issuecomment-1059251874
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] ashb commented on a change in pull request #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r819654562
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -86,6 +87,64 @@ def get_task_instance(
RTIF.task_id == TI.task_id,
),
).add_entity(RTIF)
+
+ try:
+ task_instance = query.one_or_none()
+ except MultipleResultsFound:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+ if task_instance is None:
+ raise NotFound("Task instance not found")
+ if task_instance[0].map_index != -1:
Review comment:
```suggestion
if task_instance[0].map_index < 0:
```
--
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] kaxil merged pull request #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
kaxil merged pull request #21965:
URL: https://github.com/apache/airflow/pull/21965
--
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 #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
uranusjr commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r820108710
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -86,6 +87,64 @@ def get_task_instance(
RTIF.task_id == TI.task_id,
),
).add_entity(RTIF)
+
+ try:
+ task_instance = query.one_or_none()
+ except MultipleResultsFound:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+ if task_instance is None:
+ raise NotFound("Task instance not found")
+ if task_instance[0].map_index != -1:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+
+ return task_instance_schema.dump(task_instance)
+
+
+@security.requires_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ],
+)
+@provide_session
+def get_mapped_task_instance(
+ *,
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ map_index: int,
+ session: Session = NEW_SESSION,
+) -> APIResponse:
+ """Get task instance"""
+ query = (
+ session.query(TI)
+ .filter(
+ TI.dag_id == dag_id, DR.run_id == dag_run_id, TI.task_id == task_id, TI.map_index == map_index
Review comment:
I don’t think there’s an actual difference (we need to join the DagRun table anyway), but it is just more consistent to always use the fields on the TI table.
--
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] norm commented on a change in pull request #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
norm commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r821569140
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -86,6 +87,64 @@ def get_task_instance(
RTIF.task_id == TI.task_id,
),
).add_entity(RTIF)
+
+ try:
+ task_instance = query.one_or_none()
+ except MultipleResultsFound:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+ if task_instance is None:
+ raise NotFound("Task instance not found")
+ if task_instance[0].map_index != -1:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+
+ return task_instance_schema.dump(task_instance)
+
+
+@security.requires_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ],
+)
+@provide_session
+def get_mapped_task_instance(
+ *,
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ map_index: int,
+ session: Session = NEW_SESSION,
+) -> APIResponse:
+ """Get task instance"""
+ query = (
+ session.query(TI)
+ .filter(
+ TI.dag_id == dag_id, DR.run_id == dag_run_id, TI.task_id == task_id, TI.map_index == map_index
Review comment:
Okay, have changed for both mapped and non-mapped task instance endpoint.
--
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 #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r819655067
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -86,6 +87,64 @@ def get_task_instance(
RTIF.task_id == TI.task_id,
),
).add_entity(RTIF)
+
+ try:
+ task_instance = query.one_or_none()
+ except MultipleResultsFound:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+ if task_instance is None:
+ raise NotFound("Task instance not found")
+ if task_instance[0].map_index != -1:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+
+ return task_instance_schema.dump(task_instance)
+
+
+@security.requires_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ],
+)
+@provide_session
+def get_mapped_task_instance(
+ *,
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ map_index: int,
+ session: Session = NEW_SESSION,
+) -> APIResponse:
+ """Get task instance"""
+ query = (
+ session.query(TI)
+ .filter(
+ TI.dag_id == dag_id, DR.run_id == dag_run_id, TI.task_id == task_id, TI.map_index == map_index
Review comment:
```suggestion
TI.dag_id == dag_id, TI.run_id == dag_run_id, TI.task_id == task_id, TI.map_index == map_index
```
--
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] ephraimbuddy commented on a change in pull request #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r819705023
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -134,6 +134,7 @@ info:
| v2.0 | Initial release |
| v2.0.2 | Added /plugins endpoint |
| v2.1 | New providers endpoint |
+ | v2.3.0 | New mapped task instance endpoint |
Review comment:
Looks like we should remove this summary of changes since we now have the lines like `*New in version 2.3.0*`. Not only the one you added but the whole of it
--
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] norm commented on a change in pull request #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
norm commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r821568802
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -134,6 +134,7 @@ info:
| v2.0 | Initial release |
| v2.0.2 | Added /plugins endpoint |
| v2.1 | New providers endpoint |
+ | v2.3.0 | New mapped task instance endpoint |
Review comment:
👍
--
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 #21965: WIP Task instance endpoint
Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r818715328
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -93,6 +93,54 @@ def get_task_instance(
return task_instance_schema.dump(task_instance)
+@security.requires_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ],
+)
+@provide_session
+def get_mapped_task_instance(
+ *,
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ map_index: int,
+ session: Session = NEW_SESSION,
+) -> APIResponse:
+ """Get task instance"""
+ query = (
+ session.query(TI)
+ .filter(
+ TI.dag_id == dag_id, DR.run_id == dag_run_id, TI.task_id == task_id, TI.map_index == map_index
+ )
+ .join(TI.dag_run)
+ .outerjoin(
+ SlaMiss,
+ and_(
+ SlaMiss.dag_id == TI.dag_id,
+ SlaMiss.execution_date == DR.execution_date,
+ SlaMiss.task_id == TI.task_id,
+ ),
Review comment:
TODO/note to future us: SlaMisss is going to need to be migrated to (dag_id,run_id,task_id,map_index) as it's PK soon
--
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] norm commented on a change in pull request #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
norm commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r820067571
##########
File path: airflow/api_connexion/endpoints/task_instance_endpoint.py
##########
@@ -86,6 +87,64 @@ def get_task_instance(
RTIF.task_id == TI.task_id,
),
).add_entity(RTIF)
+
+ try:
+ task_instance = query.one_or_none()
+ except MultipleResultsFound:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+ if task_instance is None:
+ raise NotFound("Task instance not found")
+ if task_instance[0].map_index != -1:
+ raise NotFound(
+ "Task instance not found", detail="Task instance is mapped, add the map_index value to the URL"
+ )
+
+ return task_instance_schema.dump(task_instance)
+
+
+@security.requires_access(
+ [
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN),
+ (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE),
+ ],
+)
+@provide_session
+def get_mapped_task_instance(
+ *,
+ dag_id: str,
+ dag_run_id: str,
+ task_id: str,
+ map_index: int,
+ session: Session = NEW_SESSION,
+) -> APIResponse:
+ """Get task instance"""
+ query = (
+ session.query(TI)
+ .filter(
+ TI.dag_id == dag_id, DR.run_id == dag_run_id, TI.task_id == task_id, TI.map_index == map_index
Review comment:
For my info, is this specific to mapped TIs?
--
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] norm commented on a change in pull request #21965: Mapped task instance endpoint
Posted by GitBox <gi...@apache.org>.
norm commented on a change in pull request #21965:
URL: https://github.com/apache/airflow/pull/21965#discussion_r820067781
##########
File path: airflow/api_connexion/openapi/v1.yaml
##########
@@ -134,6 +134,7 @@ info:
| v2.0 | Initial release |
| v2.0.2 | Added /plugins endpoint |
| v2.1 | New providers endpoint |
+ | v2.3.0 | New mapped task instance endpoint |
Review comment:
Or we should update the summary to be more detailed and accurate?
--
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