Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 94 additions & 9 deletions airflow/api_connexion/endpoints/xcom_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,47 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from flask import request
from typing import List, Optional

from flask import Response, request
from marshmallow import ValidationError
from sqlalchemy import and_, func
from sqlalchemy.orm.session import Session

from airflow.api_connexion import parameters
from airflow.api_connexion.exceptions import NotFound
from airflow.api_connexion.exceptions import BadRequest, NotFound
from airflow.api_connexion.schemas.xcom_schema import (
XComCollection, XComCollectionItemSchema, XComCollectionSchema, xcom_collection_item_schema,
xcom_collection_schema,
xcom_collection_schema, xcom_schema,
)
from airflow.models import DagRun as DR, XCom
from airflow.utils.session import provide_session


def delete_xcom_entry():
@provide_session
def delete_xcom_entry(
dag_id: str,
dag_run_id: str,
task_id: str,
xcom_key: str,
session: Session
):
"""
Delete an XCom entry
"""
raise NotImplementedError("Not implemented yet.")

dag_run = session.query(DR).filter(DR.run_id == dag_run_id,
DR.dag_id == dag_id).first()
if not dag_run:
raise NotFound(f'DAGRun with dag_id:{dag_id} and run_id:{dag_run_id} not found')

entry = session.query(XCom).filter(XCom.dag_id == dag_id,
XCom.task_id == task_id,
XCom.key == xcom_key).delete()
if not entry:
Comment thread
ephraimbuddy marked this conversation as resolved.
Resolved
raise NotFound("XCom entry not found")

return Response('', 204)


@provide_session
Expand Down Expand Up @@ -89,15 +111,78 @@ def get_xcom_entry(
return xcom_collection_item_schema.dump(query_object)


def patch_xcom_entry():
@provide_session
def patch_xcom_entry(
session: Session,
dag_id: str,
dag_run_id: str,
task_id: str,
xcom_key: str,
update_mask: Optional[List[str]] = None
):
"""
Update an XCom entry
"""
raise NotImplementedError("Not implemented yet.")
dag_run = session.query(DR).filter(DR.run_id == dag_run_id,
DR.dag_id == dag_id).first()
if not dag_run:
raise NotFound(f'DAGRun with dag_id:{dag_id} and run_id:{dag_run_id} not found')

xcom_entry = session.query(XCom).filter(XCom.dag_id == dag_id,
XCom.task_id == task_id,
XCom.key == xcom_key).first()

if not xcom_entry:
raise NotFound("XCom not found")
try:
body = xcom_schema.load(request.json, partial=("dag_id", "task_id",
"key", "execution_date"))
except ValidationError as err:
raise BadRequest(detail=err.messages.get('_schema', [str(err.messages)])[0])
data = body.data
# Check that other attributes are not being updated
for field in data:
if field != 'value' and data[field] != getattr(xcom_entry, field):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this may result in error if there's no field attribute of xcom_entry.

raise BadRequest(detail=f"'{field}' cannot be updated")
if update_mask:
update_mask = [i.strip() for i in update_mask]
data_ = {}
for field in update_mask:
if field in data and field != 'value':
Comment on lines +148 to +151

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
update_mask = [i.strip() for i in update_mask]
data_ = {}
for field in update_mask:
if field in data and field != 'value':
data_ = {}
for field in update_mask:
field = field.strip()
if field in data and field != 'value':

One loop only, WDYT?

raise BadRequest(detail=f"{field} cannot be updated")
elif field in data and field == 'value':
data_[field] = data[field]
else:
raise BadRequest(detail=f"Unknown field '{field}' in update_mask")
Comment on lines +151 to +156

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if field in data and field != 'value':
raise BadRequest(detail=f"{field} cannot be updated")
elif field in data and field == 'value':
data_[field] = data[field]
else:
raise BadRequest(detail=f"Unknown field '{field}' in update_mask")
if field not in data:
raise BadRequest(detail=f"Unknown field '{field}' in update_mask")
elif field != 'value':
raise BadRequest(detail=f"{field} cannot be updated")
elif field == 'value':
data_[field] = data[field]

WDYT?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice 👍

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about:

            if field not in data:
                raise BadRequest(detail=f"Unknown field '{field}' in update_mask")
            if field != 'value':
                raise BadRequest(detail=f"{field} cannot be updated")
            if field == 'value':
                data_[field] = data[field]

data = data_
data['value'] = XCom.serialize_value(data['value'])
xcom_entry.value = data['value']
session.add(xcom_entry)
session.commit()
return xcom_schema.dump(xcom_entry)
Comment on lines +161 to +162

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit is performed by @provide_session and I'm wondering if we need another one here explicit. I think using the one from provide_session has the advantage that if returning response will fail then there will be no change on db level. WDYT? @mik-laj @feluelle @potiuk

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...] then there will be no change on db level.

Which is correct. So I am for removing the manual commit from there.



def post_xcom_entries():
@provide_session
def post_xcom_entries(dag_id: str,
dag_run_id: str,
task_id: str,
session: Session
):
"""
Create an XCom entry
"""
raise NotImplementedError("Not implemented yet.")

dag_run = session.query(DR).filter(DR.run_id == dag_run_id, DR.dag_id == dag_id).first()
if not dag_run:
raise NotFound(f'DAGRun with dag_id:{dag_id} and run_id:{dag_run_id} not found')
try:
xcom_entry = xcom_schema.load(request.json)
except ValidationError as err:
raise BadRequest(detail=err.messages.get('_schema', [str(err.messages)])[0])
session.query(XCom).filter(XCom.dag_id == dag_id, XCom.task_id == task_id).delete()
data = xcom_entry.data
data['value'] = XCom.serialize_value(data['value'])
xcom = XCom(**data)
session.add(xcom)
session.commit()
return xcom_schema.dump(xcom)
11 changes: 9 additions & 2 deletions airflow/api_connexion/schemas/xcom_schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
# under the License.
from typing import List, NamedTuple

from marshmallow import Schema, fields
from marshmallow import Schema, ValidationError, fields, validates_schema
from marshmallow_sqlalchemy import SQLAlchemySchema, auto_field

from airflow.models import XCom
Expand All @@ -37,13 +37,20 @@ class Meta:
task_id = auto_field()
dag_id = auto_field()

@validates_schema(pass_original=True)
def check_unknown_fields(self, data, original_data):
""" Validates unknown field """
unknown = set(original_data) - set(self.fields)
if unknown:
raise ValidationError(f'Extra arguments passed: {list(unknown)}')


class XComSchema(XComCollectionItemSchema):
"""
XCom schema
"""

value = auto_field()
value = auto_field(required=True)


class XComCollection(NamedTuple):
Expand Down
Loading