SameerMesiah97 commented on code in PR #70081:
URL: https://github.com/apache/airflow/pull/70081#discussion_r3609612928
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py:
##########
@@ -27,22 +27,20 @@
from airflow.api_fastapi.common.db.common import SessionDep
from airflow.api_fastapi.core_api.base import BaseModel
+from airflow.api_fastapi.execution_api.datamodels.token import TIToken
from airflow.api_fastapi.execution_api.datamodels.xcom import (
XComResponse,
XComSequenceIndexResponse,
XComSequenceSliceResponse,
)
from airflow.api_fastapi.execution_api.security import CurrentTIToken
+from airflow.models.taskinstance import TaskInstance
from airflow.models.taskmap import TaskMap
from airflow.models.xcom import XComModel
from airflow.utils.db import get_query_count
def has_xcom_access(
- dag_id: str,
- run_id: str,
- task_id: str,
- xcom_key: Annotated[str, Path(alias="key", min_length=1)],
Review Comment:
Changing the signature of a public API by removing required parameters
breaks backwards compatibility. If you intend to deprecate them, it would
better to turn these into optional parameters (with default arguments of
`None`).
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py:
##########
@@ -359,15 +364,13 @@ def get_xcom(
# TODO: once we have JWT tokens, then remove dag_id/run_id/task_id from the
URL and just use the info in
# the token
@router.post(
- "/{dag_id}/{run_id}/{task_id}/{key:path}",
+ "/{key:path}",
Review Comment:
I think it would be better to keep the existing route for backwards
compatibility and add the new route with only `/{key:path}`.
--
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]
