Skip to content

AIP-72: Add proper serialization for XCom and Variable responses#45104

Merged
kaxil merged 1 commit intoapache:mainfrom
astronomer:fix-handle-request-bug
Dec 20, 2024
Merged

AIP-72: Add proper serialization for XCom and Variable responses#45104
kaxil merged 1 commit intoapache:mainfrom
astronomer:fix-handle-request-bug

Conversation

@kaxil
Copy link
Member

@kaxil kaxil commented Dec 20, 2024

Currently Xcom & Variables fail on main with following error:

ValidationError: 1 validation error for
tagged-union[StartupDetails,XComResult,ConnectionResult,VariableResult,ErrorResponse]
  Unable to extract tag using discriminator 'type' [type=union_tag_not_found, input_value={'key':
'test_key', 'value': 'test_value'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.10/v/union_tag_not_found

The entire stacktrace is:

decoder.get_message()
task_sdk/src/airflow/sdk/execution_time/task_runner.py:177: in get_message
    msg = self.decoder.validate_json(line)
/usr/local/lib/python3.9/site-packages/pydantic/type_adapter.py:446: in validate_json
    return self.validator.validate_json(
E   pydantic_core._pydantic_core.ValidationError: 1 validation error for tagged-union[StartupDetails,XComResult,ConnectionResult,VariableResult,ErrorResponse]
E     Unable to extract tag using discriminator 'type' [type=union_tag_not_found, input_value={'key': 'test_key', 'value': 'test_value'}, input_type=dict]
E       For further information visit https://errors.pydantic.dev/2.10/v/union_tag_not_found

This commit addresses serialization bug for XCom and Variable responses. Specifically:

  • Added from_xcom_response and from_variable_response class methods in XComResult and VariableResult, respectively, to convert autogenerated API responses into runtime-compatible result models.

  • Updated the supervisor’s handle_requests method to use these converters for GetXCom and GetVariable message handling.

  • Adjusted tests in test_supervisor.py to verify the inclusion of the type field in serialized responses and ensure proper decoding using CommsDecoder.

  • Improved test coverage by asserting that serialized responses match expected formats and verifying deserialization consistency.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Copy link
Contributor

@amoghrajesh amoghrajesh left a comment

Choose a reason for hiding this comment

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

Looks good, one comment

@kaxil kaxil force-pushed the fix-handle-request-bug branch 3 times, most recently from 0824e88 to 20f320d Compare December 20, 2024 08:39
This commit addresses serialization bug for XCom and Variable responses. Specifically:
	- Added `from_xcom_response` and `from_variable_response` class methods in XComResult and VariableResult, respectively, to convert autogenerated API responses into runtime-compatible result models.
	- Updated the supervisor’s `handle_requests` method to use these converters for `GetXCom` and `GetVariable` message handling.
	- Adjusted tests in test_supervisor.py to verify the inclusion of the type field in serialized responses and ensure proper decoding using CommsDecoder.
	- Improved test coverage by asserting that serialized responses match expected formats and verifying deserialization consistency.
@kaxil kaxil force-pushed the fix-handle-request-bug branch from 20f320d to 8985b25 Compare December 20, 2024 08:58
@kaxil kaxil merged commit 0c2f18d into apache:main Dec 20, 2024
@kaxil kaxil deleted the fix-handle-request-bug branch December 20, 2024 09:35
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
…che#45104)

Currently Xcom & Variables fail on main with following error:

```py
ValidationError: 1 validation error for
tagged-union[StartupDetails,XComResult,ConnectionResult,VariableResult,ErrorResponse]
  Unable to extract tag using discriminator 'type' [type=union_tag_not_found, input_value={'key':
'test_key', 'value': 'test_value'}, input_type=dict]
    For further information visit https://errors.pydantic.dev/2.10/v/union_tag_not_found
```

The entire stacktrace is:

```py
decoder.get_message()
task_sdk/src/airflow/sdk/execution_time/task_runner.py:177: in get_message
    msg = self.decoder.validate_json(line)
/usr/local/lib/python3.9/site-packages/pydantic/type_adapter.py:446: in validate_json
    return self.validator.validate_json(
E   pydantic_core._pydantic_core.ValidationError: 1 validation error for tagged-union[StartupDetails,XComResult,ConnectionResult,VariableResult,ErrorResponse]
E     Unable to extract tag using discriminator 'type' [type=union_tag_not_found, input_value={'key': 'test_key', 'value': 'test_value'}, input_type=dict]
E       For further information visit https://errors.pydantic.dev/2.10/v/union_tag_not_found
```

This commit addresses serialization bug for XCom and Variable responses. Specifically:

- Added `from_xcom_response` and `from_variable_response` class methods in XComResult and VariableResult, respectively, to convert autogenerated API responses into runtime-compatible result models.

- Updated the supervisor’s `handle_requests` method to use these converters for `GetXCom` and `GetVariable` message handling.
- Adjusted tests in test_supervisor.py to verify the inclusion of the type field in serialized responses and ensure proper decoding using CommsDecoder.
- Improved test coverage by asserting that serialized responses match expected formats and verifying deserialization consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects

Development

Successfully merging this pull request may close these issues.

2 participants