Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix BatchOperator links on wait_for_completion = True #25228

Merged
merged 1 commit into from Jul 22, 2022

Conversation

Taragolis
Copy link
Contributor

Fix incidentally wrong usage of list.extend method in operator_extra_links property of BatchOperator

Broken DAG: [/files/dags/example_batch.py] Traceback (most recent call last):
  File "/opt/airflow/airflow/serialization/serialized_objects.py", line 658, in _serialize_node
    if op.operator_extra_links:
  File "/opt/airflow/airflow/providers/amazon/aws/operators/batch.py", line 118, in operator_extra_links
    op_extra_links.extend(BatchJobDefinitionLink(), BatchJobQueueLink())
TypeError: extend() takes exactly one argument (2 given)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/opt/airflow/airflow/serialization/serialized_objects.py", line 1125, in to_dict
    json_dict = {"__version": cls.SERIALIZER_VERSION, "dag": cls.serialize_dag(var)}
  File "/opt/airflow/airflow/serialization/serialized_objects.py", line 1033, in serialize_dag
    raise SerializationError(f'Failed to serialize DAG {dag.dag_id!r}: {e}')
airflow.exceptions.SerializationError: Failed to serialize DAG 'example_batch_submit_job_2': extend() takes exactly one argument (2 given)

@boring-cyborg boring-cyborg bot added area:providers provider:amazon-aws AWS/Amazon - related issues labels Jul 22, 2022
@josh-fell
Copy link
Contributor

@Taragolis Have you happened to use the BatchOperator with Dynamic Task Mapping? The reason I ask is there was a previously reported issue with operator links declared this way and it breaking dynamically mapped tasks. There are some fixes out there as well: #24676 and #25215.

Going to block this in the meantime.

Copy link
Contributor

@josh-fell josh-fell left a comment

Choose a reason for hiding this comment

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

Holding off on merging this for a response on Dynamic Task Mapping impact. However, if anyone feels strongly, we can merge this and look at a fix (if one is needed) as a fast-follow perhaps?

@Taragolis
Copy link
Contributor Author

I thought it required changes in DAG serialisation, if attribute is property it is required get actual value of property rather than object and only affect serialisation error for Dynamic Task Mapping since Airflow 2.3.

I could also look on issue related to DAG serialisation (separate PR), however this PR fix general Operator issue.

@josh-fell
Copy link
Contributor

I did a quick test. Definitely the same serialization issue. See #25243.

@josh-fell josh-fell merged commit ceb1658 into apache:main Jul 22, 2022
@josh-fell
Copy link
Contributor

I thought it required changes in DAG serialisation, if attribute is property it is required get actual value of property rather than object and only affect serialisation error for Dynamic Task Mapping since Airflow 2.3.

I could also look on issue related to DAG serialisation (separate PR), however this PR fix general Operator issue.

No worries! Just curious if you had tried it. Good points. The issue can be tackled later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers provider:amazon-aws AWS/Amazon - related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants