Ensure that Connection extra can get masked without causing an error - #54769

Merged
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking
Aug 21, 2025
Merged

Ensure that Connection extra can get masked without causing an error#54769
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking

Conversation

@ashb

@ashbashb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Fixes#54768

This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because Iterable is too open-ended a type

Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.


^ 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 airflow-core/newsfragments.

Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
@VladaZakharova

Copy link
Copy Markdown
Contributor

Hi
thank you for the fix
Did you check this on different types? can you please add unit tests for that?

@potiukpotiuk left a comment

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.

Damn. Pydantic runtime type usage has side effects - that change looks lile it changes nothing....

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hi thank you for the fix Did you check this on different types? can you please add unit tests for that?

@ashb wrote:

Yes, this should absolutely have unit tests to go with the fix, but a fix is better than nothing, and I'm about to leave on a camping holiday.

I guess we should add unit tests as follow-up without breaking @ashb's plans.

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Anyone hitting this on 3.0.5 andwho doesn't want to rollback to 3.0.4: a work around for now is to manually apply this patch to your Airflow.

If you are using docker to deploy your airflow then something like this should help hopefully:

USER root
RUN apt update && apt install -y patch patchutils
RUN set -ex; \
cd /usr/local/lib/python3.12/site-packages/airflow; \
curl -L https://patch-diff.githubusercontent.com/raw/apache/airflow/pull/54769.patch \
| filterdiff -p1 -i 'task-sdk/src/airflow/*' | patch -p4 -u --verbose
USER airflow

The path to site-packages and the user to switch back to at the end would need adapting to your specific Dockerfile

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Did you check this on different types? can you please add unit tests for that?

I manually tested it with this:

First set the conneciton:

export AIRFLOW_CONN_TEST='{"conn_type": "google_cloud_default", "extra": {"key_path": "/files/airflow-breeze-config/keys2/keys.json", "scope": "https://www.googleapis.com/auth/cloud-platform", "project": "project_id", "num_retries": 6}}

Than I ran this DAG:

@taskdefmy_function() ->None:
conn=Connection.get("test")
print(f"{conn.conn_id=}{conn.password=}{conn.extra_dejson=}{conn=}")
withDAG("test_dag") asdag:
my_function()

@potiuk

Copy link
Copy Markdown
Member

I manually tested it with this:

I will take it from now on @ashb -> thanks for the repro scenarios

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn!

@amoghrajesh

Copy link
Copy Markdown
Contributor

@ashb mypy will need a fix:

 task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:127: error: Argument "value" to "MaskSecret" has incompatible type "str | Iterable[Any]"; expected "JsonValue" [arg-type]
comms.send(MaskSecret(value=secret, name=name))
^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

@potiuk

Copy link
Copy Markdown
Member

^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

I'll copy the PR and fix it -> and I will ask others to help :) .. @ashb will not be available to fix it @amoghrajesh

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Ah... It's in `apache/airflow" so we can push to it directly

@VladaZakharova

Copy link
Copy Markdown
Contributor

Originally, there was also the problem with triggerers with the following errors:

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

do you think we can also fix the types for masks to solve this problem too?

@gopidesupavan

Copy link
Copy Markdown
Member

oh geeee...

@amoghrajesh

Copy link
Copy Markdown
Contributor

@potiuk you can push directly to the PR. No need to copy it.

@potiuk

Copy link
Copy Markdown
Member

@potiuk you can push directly to the PR. No need to copy it.

Yep. Will do

@potiuk

Copy link
Copy Markdown
Member

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

@potiuk

Copy link
Copy Markdown
Member

Let's just push fixups without -f : to not override each-others's changes.

@amoghrajesh

amoghrajesh commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Dropped you a message on slack, looking into mypy failures

@potiuk

Copy link
Copy Markdown
Member

pushed mypy fix :)

@VladaZakharova

VladaZakharova commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

@potiuk

Copy link
Copy Markdown
Member

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

@VladaZakharova

Copy link
Copy Markdown
Contributor

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

agree :)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Agreed too. Let's fix the error at hand first and we can always follow up for the non breaking / burning things...

@potiuk

Copy link
Copy Markdown
Member

fixup with tests is here

@potiuk

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Yeah I do not think it is directly linked to this PR

@kaxil

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

Yeah unrelated

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtask-sdk/tests/task_sdk/definitions/test_secrets_masker.py
@kaxil

Copy link
Copy Markdown
Member

Compat test change: #54776

@potiuk

Copy link
Copy Markdown
Member

Merging it 3.0.5 fails clearly because of yanking :) and #54766 is already on the way.

@potiuk
potiuk merged commit 5aec867 into mainAug 21, 2025
74 of 75 checks passed
@potiuk
potiuk deleted the fix-extra-connection-masking branch August 21, 2025 12:40
github-actionsBot pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-0-test

StatusBranchResult
v3-0-testPR Link

kaxil pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
kaxil pushed a commit that referenced this pull request Aug 22, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#54769)
* Ensure that Connection extra can get masked without causing an error
Fixesapache#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of conn.extra_dejson in Airflow 3.0.5 fails with NotImplementedError error

9 participants

@ashb@VladaZakharova@potiuk@amoghrajesh@gopidesupavan@kaxil@dshvedchenko@Lee-W@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Ensure that Connection extra can get masked without causing an error - #54769

Merged
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking
Aug 21, 2025
Merged

Ensure that Connection extra can get masked without causing an error#54769
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking

Conversation

@ashb

@ashbashb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Fixes#54768

This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because Iterable is too open-ended a type

Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.


^ 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 airflow-core/newsfragments.

Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
@VladaZakharova

Copy link
Copy Markdown
Contributor

Hi
thank you for the fix
Did you check this on different types? can you please add unit tests for that?

@potiukpotiuk left a comment

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.

Damn. Pydantic runtime type usage has side effects - that change looks lile it changes nothing....

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hi thank you for the fix Did you check this on different types? can you please add unit tests for that?

@ashb wrote:

Yes, this should absolutely have unit tests to go with the fix, but a fix is better than nothing, and I'm about to leave on a camping holiday.

I guess we should add unit tests as follow-up without breaking @ashb's plans.

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Anyone hitting this on 3.0.5 andwho doesn't want to rollback to 3.0.4: a work around for now is to manually apply this patch to your Airflow.

If you are using docker to deploy your airflow then something like this should help hopefully:

USER root
RUN apt update && apt install -y patch patchutils
RUN set -ex; \
cd /usr/local/lib/python3.12/site-packages/airflow; \
curl -L https://patch-diff.githubusercontent.com/raw/apache/airflow/pull/54769.patch \
| filterdiff -p1 -i 'task-sdk/src/airflow/*' | patch -p4 -u --verbose
USER airflow

The path to site-packages and the user to switch back to at the end would need adapting to your specific Dockerfile

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Did you check this on different types? can you please add unit tests for that?

I manually tested it with this:

First set the conneciton:

export AIRFLOW_CONN_TEST='{"conn_type": "google_cloud_default", "extra": {"key_path": "/files/airflow-breeze-config/keys2/keys.json", "scope": "https://www.googleapis.com/auth/cloud-platform", "project": "project_id", "num_retries": 6}}

Than I ran this DAG:

@taskdefmy_function() ->None:
conn=Connection.get("test")
print(f"{conn.conn_id=}{conn.password=}{conn.extra_dejson=}{conn=}")
withDAG("test_dag") asdag:
my_function()

@potiuk

Copy link
Copy Markdown
Member

I manually tested it with this:

I will take it from now on @ashb -> thanks for the repro scenarios

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn!

@amoghrajesh

Copy link
Copy Markdown
Contributor

@ashb mypy will need a fix:

 task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:127: error: Argument "value" to "MaskSecret" has incompatible type "str | Iterable[Any]"; expected "JsonValue" [arg-type]
comms.send(MaskSecret(value=secret, name=name))
^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

@potiuk

Copy link
Copy Markdown
Member

^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

I'll copy the PR and fix it -> and I will ask others to help :) .. @ashb will not be available to fix it @amoghrajesh

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Ah... It's in `apache/airflow" so we can push to it directly

@VladaZakharova

Copy link
Copy Markdown
Contributor

Originally, there was also the problem with triggerers with the following errors:

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

do you think we can also fix the types for masks to solve this problem too?

@gopidesupavan

Copy link
Copy Markdown
Member

oh geeee...

@amoghrajesh

Copy link
Copy Markdown
Contributor

@potiuk you can push directly to the PR. No need to copy it.

@potiuk

Copy link
Copy Markdown
Member

@potiuk you can push directly to the PR. No need to copy it.

Yep. Will do

@potiuk

Copy link
Copy Markdown
Member

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

@potiuk

Copy link
Copy Markdown
Member

Let's just push fixups without -f : to not override each-others's changes.

@amoghrajesh

amoghrajesh commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Dropped you a message on slack, looking into mypy failures

@potiuk

Copy link
Copy Markdown
Member

pushed mypy fix :)

@VladaZakharova

VladaZakharova commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

@potiuk

Copy link
Copy Markdown
Member

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

@VladaZakharova

Copy link
Copy Markdown
Contributor

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

agree :)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Agreed too. Let's fix the error at hand first and we can always follow up for the non breaking / burning things...

@potiuk

Copy link
Copy Markdown
Member

fixup with tests is here

@potiuk

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Yeah I do not think it is directly linked to this PR

@kaxil

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

Yeah unrelated

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtask-sdk/tests/task_sdk/definitions/test_secrets_masker.py
@kaxil

Copy link
Copy Markdown
Member

Compat test change: #54776

@potiuk

Copy link
Copy Markdown
Member

Merging it 3.0.5 fails clearly because of yanking :) and #54766 is already on the way.

@potiuk
potiuk merged commit 5aec867 into mainAug 21, 2025
74 of 75 checks passed
@potiuk
potiuk deleted the fix-extra-connection-masking branch August 21, 2025 12:40
github-actionsBot pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-0-test

StatusBranchResult
v3-0-testPR Link

kaxil pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
kaxil pushed a commit that referenced this pull request Aug 22, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#54769)
* Ensure that Connection extra can get masked without causing an error
Fixesapache#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of conn.extra_dejson in Airflow 3.0.5 fails with NotImplementedError error

9 participants

@ashb@VladaZakharova@potiuk@amoghrajesh@gopidesupavan@kaxil@dshvedchenko@Lee-W@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Ensure that Connection extra can get masked without causing an error - #54769

Merged
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking
Aug 21, 2025
Merged

Ensure that Connection extra can get masked without causing an error#54769
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking

Conversation

@ashb

@ashbashb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Fixes#54768

This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because Iterable is too open-ended a type

Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.


^ 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 airflow-core/newsfragments.

Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
@VladaZakharova

Copy link
Copy Markdown
Contributor

Hi
thank you for the fix
Did you check this on different types? can you please add unit tests for that?

@potiukpotiuk left a comment

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.

Damn. Pydantic runtime type usage has side effects - that change looks lile it changes nothing....

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hi thank you for the fix Did you check this on different types? can you please add unit tests for that?

@ashb wrote:

Yes, this should absolutely have unit tests to go with the fix, but a fix is better than nothing, and I'm about to leave on a camping holiday.

I guess we should add unit tests as follow-up without breaking @ashb's plans.

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Anyone hitting this on 3.0.5 andwho doesn't want to rollback to 3.0.4: a work around for now is to manually apply this patch to your Airflow.

If you are using docker to deploy your airflow then something like this should help hopefully:

USER root
RUN apt update && apt install -y patch patchutils
RUN set -ex; \
cd /usr/local/lib/python3.12/site-packages/airflow; \
curl -L https://patch-diff.githubusercontent.com/raw/apache/airflow/pull/54769.patch \
| filterdiff -p1 -i 'task-sdk/src/airflow/*' | patch -p4 -u --verbose
USER airflow

The path to site-packages and the user to switch back to at the end would need adapting to your specific Dockerfile

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Did you check this on different types? can you please add unit tests for that?

I manually tested it with this:

First set the conneciton:

export AIRFLOW_CONN_TEST='{"conn_type": "google_cloud_default", "extra": {"key_path": "/files/airflow-breeze-config/keys2/keys.json", "scope": "https://www.googleapis.com/auth/cloud-platform", "project": "project_id", "num_retries": 6}}

Than I ran this DAG:

@taskdefmy_function() ->None:
conn=Connection.get("test")
print(f"{conn.conn_id=}{conn.password=}{conn.extra_dejson=}{conn=}")
withDAG("test_dag") asdag:
my_function()

@potiuk

Copy link
Copy Markdown
Member

I manually tested it with this:

I will take it from now on @ashb -> thanks for the repro scenarios

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn!

@amoghrajesh

Copy link
Copy Markdown
Contributor

@ashb mypy will need a fix:

 task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:127: error: Argument "value" to "MaskSecret" has incompatible type "str | Iterable[Any]"; expected "JsonValue" [arg-type]
comms.send(MaskSecret(value=secret, name=name))
^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

@potiuk

Copy link
Copy Markdown
Member

^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

I'll copy the PR and fix it -> and I will ask others to help :) .. @ashb will not be available to fix it @amoghrajesh

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Ah... It's in `apache/airflow" so we can push to it directly

@VladaZakharova

Copy link
Copy Markdown
Contributor

Originally, there was also the problem with triggerers with the following errors:

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

do you think we can also fix the types for masks to solve this problem too?

@gopidesupavan

Copy link
Copy Markdown
Member

oh geeee...

@amoghrajesh

Copy link
Copy Markdown
Contributor

@potiuk you can push directly to the PR. No need to copy it.

@potiuk

Copy link
Copy Markdown
Member

@potiuk you can push directly to the PR. No need to copy it.

Yep. Will do

@potiuk

Copy link
Copy Markdown
Member

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

@potiuk

Copy link
Copy Markdown
Member

Let's just push fixups without -f : to not override each-others's changes.

@amoghrajesh

amoghrajesh commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Dropped you a message on slack, looking into mypy failures

@potiuk

Copy link
Copy Markdown
Member

pushed mypy fix :)

@VladaZakharova

VladaZakharova commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

@potiuk

Copy link
Copy Markdown
Member

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

@VladaZakharova

Copy link
Copy Markdown
Contributor

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

agree :)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Agreed too. Let's fix the error at hand first and we can always follow up for the non breaking / burning things...

@potiuk

Copy link
Copy Markdown
Member

fixup with tests is here

@potiuk

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Yeah I do not think it is directly linked to this PR

@kaxil

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

Yeah unrelated

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtask-sdk/tests/task_sdk/definitions/test_secrets_masker.py
@kaxil

Copy link
Copy Markdown
Member

Compat test change: #54776

@potiuk

Copy link
Copy Markdown
Member

Merging it 3.0.5 fails clearly because of yanking :) and #54766 is already on the way.

@potiuk
potiuk merged commit 5aec867 into mainAug 21, 2025
74 of 75 checks passed
@potiuk
potiuk deleted the fix-extra-connection-masking branch August 21, 2025 12:40
github-actionsBot pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-0-test

StatusBranchResult
v3-0-testPR Link

kaxil pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
kaxil pushed a commit that referenced this pull request Aug 22, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#54769)
* Ensure that Connection extra can get masked without causing an error
Fixesapache#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of conn.extra_dejson in Airflow 3.0.5 fails with NotImplementedError error

9 participants

@ashb@VladaZakharova@potiuk@amoghrajesh@gopidesupavan@kaxil@dshvedchenko@Lee-W@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Ensure that Connection extra can get masked without causing an error - #54769

Merged
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking
Aug 21, 2025
Merged

Ensure that Connection extra can get masked without causing an error#54769
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking

Conversation

@ashb

@ashbashb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Fixes#54768

This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because Iterable is too open-ended a type

Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.


^ 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 airflow-core/newsfragments.

Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
@VladaZakharova

Copy link
Copy Markdown
Contributor

Hi
thank you for the fix
Did you check this on different types? can you please add unit tests for that?

@potiukpotiuk left a comment

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.

Damn. Pydantic runtime type usage has side effects - that change looks lile it changes nothing....

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hi thank you for the fix Did you check this on different types? can you please add unit tests for that?

@ashb wrote:

Yes, this should absolutely have unit tests to go with the fix, but a fix is better than nothing, and I'm about to leave on a camping holiday.

I guess we should add unit tests as follow-up without breaking @ashb's plans.

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Anyone hitting this on 3.0.5 andwho doesn't want to rollback to 3.0.4: a work around for now is to manually apply this patch to your Airflow.

If you are using docker to deploy your airflow then something like this should help hopefully:

USER root
RUN apt update && apt install -y patch patchutils
RUN set -ex; \
cd /usr/local/lib/python3.12/site-packages/airflow; \
curl -L https://patch-diff.githubusercontent.com/raw/apache/airflow/pull/54769.patch \
| filterdiff -p1 -i 'task-sdk/src/airflow/*' | patch -p4 -u --verbose
USER airflow

The path to site-packages and the user to switch back to at the end would need adapting to your specific Dockerfile

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Did you check this on different types? can you please add unit tests for that?

I manually tested it with this:

First set the conneciton:

export AIRFLOW_CONN_TEST='{"conn_type": "google_cloud_default", "extra": {"key_path": "/files/airflow-breeze-config/keys2/keys.json", "scope": "https://www.googleapis.com/auth/cloud-platform", "project": "project_id", "num_retries": 6}}

Than I ran this DAG:

@taskdefmy_function() ->None:
conn=Connection.get("test")
print(f"{conn.conn_id=}{conn.password=}{conn.extra_dejson=}{conn=}")
withDAG("test_dag") asdag:
my_function()

@potiuk

Copy link
Copy Markdown
Member

I manually tested it with this:

I will take it from now on @ashb -> thanks for the repro scenarios

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn!

@amoghrajesh

Copy link
Copy Markdown
Contributor

@ashb mypy will need a fix:

 task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:127: error: Argument "value" to "MaskSecret" has incompatible type "str | Iterable[Any]"; expected "JsonValue" [arg-type]
comms.send(MaskSecret(value=secret, name=name))
^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

@potiuk

Copy link
Copy Markdown
Member

^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

I'll copy the PR and fix it -> and I will ask others to help :) .. @ashb will not be available to fix it @amoghrajesh

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Ah... It's in `apache/airflow" so we can push to it directly

@VladaZakharova

Copy link
Copy Markdown
Contributor

Originally, there was also the problem with triggerers with the following errors:

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

do you think we can also fix the types for masks to solve this problem too?

@gopidesupavan

Copy link
Copy Markdown
Member

oh geeee...

@amoghrajesh

Copy link
Copy Markdown
Contributor

@potiuk you can push directly to the PR. No need to copy it.

@potiuk

Copy link
Copy Markdown
Member

@potiuk you can push directly to the PR. No need to copy it.

Yep. Will do

@potiuk

Copy link
Copy Markdown
Member

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

@potiuk

Copy link
Copy Markdown
Member

Let's just push fixups without -f : to not override each-others's changes.

@amoghrajesh

amoghrajesh commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Dropped you a message on slack, looking into mypy failures

@potiuk

Copy link
Copy Markdown
Member

pushed mypy fix :)

@VladaZakharova

VladaZakharova commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

@potiuk

Copy link
Copy Markdown
Member

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

@VladaZakharova

Copy link
Copy Markdown
Contributor

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

agree :)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Agreed too. Let's fix the error at hand first and we can always follow up for the non breaking / burning things...

@potiuk

Copy link
Copy Markdown
Member

fixup with tests is here

@potiuk

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Yeah I do not think it is directly linked to this PR

@kaxil

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

Yeah unrelated

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtask-sdk/tests/task_sdk/definitions/test_secrets_masker.py
@kaxil

Copy link
Copy Markdown
Member

Compat test change: #54776

@potiuk

Copy link
Copy Markdown
Member

Merging it 3.0.5 fails clearly because of yanking :) and #54766 is already on the way.

@potiuk
potiuk merged commit 5aec867 into mainAug 21, 2025
74 of 75 checks passed
@potiuk
potiuk deleted the fix-extra-connection-masking branch August 21, 2025 12:40
github-actionsBot pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-0-test

StatusBranchResult
v3-0-testPR Link

kaxil pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
kaxil pushed a commit that referenced this pull request Aug 22, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#54769)
* Ensure that Connection extra can get masked without causing an error
Fixesapache#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of conn.extra_dejson in Airflow 3.0.5 fails with NotImplementedError error

9 participants

@ashb@VladaZakharova@potiuk@amoghrajesh@gopidesupavan@kaxil@dshvedchenko@Lee-W@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Ensure that Connection extra can get masked without causing an error - #54769

Merged
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking
Aug 21, 2025
Merged

Ensure that Connection extra can get masked without causing an error#54769
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking

Conversation

@ashb

@ashbashb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Fixes#54768

This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because Iterable is too open-ended a type

Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.


^ 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 airflow-core/newsfragments.

Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
@VladaZakharova

Copy link
Copy Markdown
Contributor

Hi
thank you for the fix
Did you check this on different types? can you please add unit tests for that?

@potiukpotiuk left a comment

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.

Damn. Pydantic runtime type usage has side effects - that change looks lile it changes nothing....

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hi thank you for the fix Did you check this on different types? can you please add unit tests for that?

@ashb wrote:

Yes, this should absolutely have unit tests to go with the fix, but a fix is better than nothing, and I'm about to leave on a camping holiday.

I guess we should add unit tests as follow-up without breaking @ashb's plans.

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Anyone hitting this on 3.0.5 andwho doesn't want to rollback to 3.0.4: a work around for now is to manually apply this patch to your Airflow.

If you are using docker to deploy your airflow then something like this should help hopefully:

USER root
RUN apt update && apt install -y patch patchutils
RUN set -ex; \
cd /usr/local/lib/python3.12/site-packages/airflow; \
curl -L https://patch-diff.githubusercontent.com/raw/apache/airflow/pull/54769.patch \
| filterdiff -p1 -i 'task-sdk/src/airflow/*' | patch -p4 -u --verbose
USER airflow

The path to site-packages and the user to switch back to at the end would need adapting to your specific Dockerfile

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Did you check this on different types? can you please add unit tests for that?

I manually tested it with this:

First set the conneciton:

export AIRFLOW_CONN_TEST='{"conn_type": "google_cloud_default", "extra": {"key_path": "/files/airflow-breeze-config/keys2/keys.json", "scope": "https://www.googleapis.com/auth/cloud-platform", "project": "project_id", "num_retries": 6}}

Than I ran this DAG:

@taskdefmy_function() ->None:
conn=Connection.get("test")
print(f"{conn.conn_id=}{conn.password=}{conn.extra_dejson=}{conn=}")
withDAG("test_dag") asdag:
my_function()

@potiuk

Copy link
Copy Markdown
Member

I manually tested it with this:

I will take it from now on @ashb -> thanks for the repro scenarios

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn!

@amoghrajesh

Copy link
Copy Markdown
Contributor

@ashb mypy will need a fix:

 task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:127: error: Argument "value" to "MaskSecret" has incompatible type "str | Iterable[Any]"; expected "JsonValue" [arg-type]
comms.send(MaskSecret(value=secret, name=name))
^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

@potiuk

Copy link
Copy Markdown
Member

^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

I'll copy the PR and fix it -> and I will ask others to help :) .. @ashb will not be available to fix it @amoghrajesh

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Ah... It's in `apache/airflow" so we can push to it directly

@VladaZakharova

Copy link
Copy Markdown
Contributor

Originally, there was also the problem with triggerers with the following errors:

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

do you think we can also fix the types for masks to solve this problem too?

@gopidesupavan

Copy link
Copy Markdown
Member

oh geeee...

@amoghrajesh

Copy link
Copy Markdown
Contributor

@potiuk you can push directly to the PR. No need to copy it.

@potiuk

Copy link
Copy Markdown
Member

@potiuk you can push directly to the PR. No need to copy it.

Yep. Will do

@potiuk

Copy link
Copy Markdown
Member

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

@potiuk

Copy link
Copy Markdown
Member

Let's just push fixups without -f : to not override each-others's changes.

@amoghrajesh

amoghrajesh commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Dropped you a message on slack, looking into mypy failures

@potiuk

Copy link
Copy Markdown
Member

pushed mypy fix :)

@VladaZakharova

VladaZakharova commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

@potiuk

Copy link
Copy Markdown
Member

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

@VladaZakharova

Copy link
Copy Markdown
Contributor

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

agree :)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Agreed too. Let's fix the error at hand first and we can always follow up for the non breaking / burning things...

@potiuk

Copy link
Copy Markdown
Member

fixup with tests is here

@potiuk

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Yeah I do not think it is directly linked to this PR

@kaxil

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

Yeah unrelated

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtask-sdk/tests/task_sdk/definitions/test_secrets_masker.py
@kaxil

Copy link
Copy Markdown
Member

Compat test change: #54776

@potiuk

Copy link
Copy Markdown
Member

Merging it 3.0.5 fails clearly because of yanking :) and #54766 is already on the way.

@potiuk
potiuk merged commit 5aec867 into mainAug 21, 2025
74 of 75 checks passed
@potiuk
potiuk deleted the fix-extra-connection-masking branch August 21, 2025 12:40
github-actionsBot pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-0-test

StatusBranchResult
v3-0-testPR Link

kaxil pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
kaxil pushed a commit that referenced this pull request Aug 22, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#54769)
* Ensure that Connection extra can get masked without causing an error
Fixesapache#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of conn.extra_dejson in Airflow 3.0.5 fails with NotImplementedError error

9 participants

@ashb@VladaZakharova@potiuk@amoghrajesh@gopidesupavan@kaxil@dshvedchenko@Lee-W@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Ensure that Connection extra can get masked without causing an error - #54769

Merged
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking
Aug 21, 2025
Merged

Ensure that Connection extra can get masked without causing an error#54769
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking

Conversation

@ashb

@ashbashb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Fixes#54768

This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because Iterable is too open-ended a type

Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.


^ 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 airflow-core/newsfragments.

Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
@VladaZakharova

Copy link
Copy Markdown
Contributor

Hi
thank you for the fix
Did you check this on different types? can you please add unit tests for that?

@potiukpotiuk left a comment

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.

Damn. Pydantic runtime type usage has side effects - that change looks lile it changes nothing....

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hi thank you for the fix Did you check this on different types? can you please add unit tests for that?

@ashb wrote:

Yes, this should absolutely have unit tests to go with the fix, but a fix is better than nothing, and I'm about to leave on a camping holiday.

I guess we should add unit tests as follow-up without breaking @ashb's plans.

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Anyone hitting this on 3.0.5 andwho doesn't want to rollback to 3.0.4: a work around for now is to manually apply this patch to your Airflow.

If you are using docker to deploy your airflow then something like this should help hopefully:

USER root
RUN apt update && apt install -y patch patchutils
RUN set -ex; \
cd /usr/local/lib/python3.12/site-packages/airflow; \
curl -L https://patch-diff.githubusercontent.com/raw/apache/airflow/pull/54769.patch \
| filterdiff -p1 -i 'task-sdk/src/airflow/*' | patch -p4 -u --verbose
USER airflow

The path to site-packages and the user to switch back to at the end would need adapting to your specific Dockerfile

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Did you check this on different types? can you please add unit tests for that?

I manually tested it with this:

First set the conneciton:

export AIRFLOW_CONN_TEST='{"conn_type": "google_cloud_default", "extra": {"key_path": "/files/airflow-breeze-config/keys2/keys.json", "scope": "https://www.googleapis.com/auth/cloud-platform", "project": "project_id", "num_retries": 6}}

Than I ran this DAG:

@taskdefmy_function() ->None:
conn=Connection.get("test")
print(f"{conn.conn_id=}{conn.password=}{conn.extra_dejson=}{conn=}")
withDAG("test_dag") asdag:
my_function()

@potiuk

Copy link
Copy Markdown
Member

I manually tested it with this:

I will take it from now on @ashb -> thanks for the repro scenarios

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn!

@amoghrajesh

Copy link
Copy Markdown
Contributor

@ashb mypy will need a fix:

 task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:127: error: Argument "value" to "MaskSecret" has incompatible type "str | Iterable[Any]"; expected "JsonValue" [arg-type]
comms.send(MaskSecret(value=secret, name=name))
^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

@potiuk

Copy link
Copy Markdown
Member

^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

I'll copy the PR and fix it -> and I will ask others to help :) .. @ashb will not be available to fix it @amoghrajesh

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Ah... It's in `apache/airflow" so we can push to it directly

@VladaZakharova

Copy link
Copy Markdown
Contributor

Originally, there was also the problem with triggerers with the following errors:

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

do you think we can also fix the types for masks to solve this problem too?

@gopidesupavan

Copy link
Copy Markdown
Member

oh geeee...

@amoghrajesh

Copy link
Copy Markdown
Contributor

@potiuk you can push directly to the PR. No need to copy it.

@potiuk

Copy link
Copy Markdown
Member

@potiuk you can push directly to the PR. No need to copy it.

Yep. Will do

@potiuk

Copy link
Copy Markdown
Member

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

@potiuk

Copy link
Copy Markdown
Member

Let's just push fixups without -f : to not override each-others's changes.

@amoghrajesh

amoghrajesh commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Dropped you a message on slack, looking into mypy failures

@potiuk

Copy link
Copy Markdown
Member

pushed mypy fix :)

@VladaZakharova

VladaZakharova commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

@potiuk

Copy link
Copy Markdown
Member

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

@VladaZakharova

Copy link
Copy Markdown
Contributor

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

agree :)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Agreed too. Let's fix the error at hand first and we can always follow up for the non breaking / burning things...

@potiuk

Copy link
Copy Markdown
Member

fixup with tests is here

@potiuk

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Yeah I do not think it is directly linked to this PR

@kaxil

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

Yeah unrelated

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtask-sdk/tests/task_sdk/definitions/test_secrets_masker.py
@kaxil

Copy link
Copy Markdown
Member

Compat test change: #54776

@potiuk

Copy link
Copy Markdown
Member

Merging it 3.0.5 fails clearly because of yanking :) and #54766 is already on the way.

@potiuk
potiuk merged commit 5aec867 into mainAug 21, 2025
74 of 75 checks passed
@potiuk
potiuk deleted the fix-extra-connection-masking branch August 21, 2025 12:40
github-actionsBot pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-0-test

StatusBranchResult
v3-0-testPR Link

kaxil pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
kaxil pushed a commit that referenced this pull request Aug 22, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#54769)
* Ensure that Connection extra can get masked without causing an error
Fixesapache#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of conn.extra_dejson in Airflow 3.0.5 fails with NotImplementedError error

9 participants

@ashb@VladaZakharova@potiuk@amoghrajesh@gopidesupavan@kaxil@dshvedchenko@Lee-W@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Ensure that Connection extra can get masked without causing an error - #54769

Merged
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking
Aug 21, 2025
Merged

Ensure that Connection extra can get masked without causing an error#54769
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking

Conversation

@ashb

@ashbashb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Fixes#54768

This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because Iterable is too open-ended a type

Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.


^ 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 airflow-core/newsfragments.

Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
@VladaZakharova

Copy link
Copy Markdown
Contributor

Hi
thank you for the fix
Did you check this on different types? can you please add unit tests for that?

@potiukpotiuk left a comment

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.

Damn. Pydantic runtime type usage has side effects - that change looks lile it changes nothing....

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hi thank you for the fix Did you check this on different types? can you please add unit tests for that?

@ashb wrote:

Yes, this should absolutely have unit tests to go with the fix, but a fix is better than nothing, and I'm about to leave on a camping holiday.

I guess we should add unit tests as follow-up without breaking @ashb's plans.

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Anyone hitting this on 3.0.5 andwho doesn't want to rollback to 3.0.4: a work around for now is to manually apply this patch to your Airflow.

If you are using docker to deploy your airflow then something like this should help hopefully:

USER root
RUN apt update && apt install -y patch patchutils
RUN set -ex; \
cd /usr/local/lib/python3.12/site-packages/airflow; \
curl -L https://patch-diff.githubusercontent.com/raw/apache/airflow/pull/54769.patch \
| filterdiff -p1 -i 'task-sdk/src/airflow/*' | patch -p4 -u --verbose
USER airflow

The path to site-packages and the user to switch back to at the end would need adapting to your specific Dockerfile

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Did you check this on different types? can you please add unit tests for that?

I manually tested it with this:

First set the conneciton:

export AIRFLOW_CONN_TEST='{"conn_type": "google_cloud_default", "extra": {"key_path": "/files/airflow-breeze-config/keys2/keys.json", "scope": "https://www.googleapis.com/auth/cloud-platform", "project": "project_id", "num_retries": 6}}

Than I ran this DAG:

@taskdefmy_function() ->None:
conn=Connection.get("test")
print(f"{conn.conn_id=}{conn.password=}{conn.extra_dejson=}{conn=}")
withDAG("test_dag") asdag:
my_function()

@potiuk

Copy link
Copy Markdown
Member

I manually tested it with this:

I will take it from now on @ashb -> thanks for the repro scenarios

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn!

@amoghrajesh

Copy link
Copy Markdown
Contributor

@ashb mypy will need a fix:

 task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:127: error: Argument "value" to "MaskSecret" has incompatible type "str | Iterable[Any]"; expected "JsonValue" [arg-type]
comms.send(MaskSecret(value=secret, name=name))
^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

@potiuk

Copy link
Copy Markdown
Member

^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

I'll copy the PR and fix it -> and I will ask others to help :) .. @ashb will not be available to fix it @amoghrajesh

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Ah... It's in `apache/airflow" so we can push to it directly

@VladaZakharova

Copy link
Copy Markdown
Contributor

Originally, there was also the problem with triggerers with the following errors:

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

do you think we can also fix the types for masks to solve this problem too?

@gopidesupavan

Copy link
Copy Markdown
Member

oh geeee...

@amoghrajesh

Copy link
Copy Markdown
Contributor

@potiuk you can push directly to the PR. No need to copy it.

@potiuk

Copy link
Copy Markdown
Member

@potiuk you can push directly to the PR. No need to copy it.

Yep. Will do

@potiuk

Copy link
Copy Markdown
Member

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

@potiuk

Copy link
Copy Markdown
Member

Let's just push fixups without -f : to not override each-others's changes.

@amoghrajesh

amoghrajesh commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Dropped you a message on slack, looking into mypy failures

@potiuk

Copy link
Copy Markdown
Member

pushed mypy fix :)

@VladaZakharova

VladaZakharova commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

@potiuk

Copy link
Copy Markdown
Member

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

@VladaZakharova

Copy link
Copy Markdown
Contributor

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

agree :)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Agreed too. Let's fix the error at hand first and we can always follow up for the non breaking / burning things...

@potiuk

Copy link
Copy Markdown
Member

fixup with tests is here

@potiuk

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Yeah I do not think it is directly linked to this PR

@kaxil

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

Yeah unrelated

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtask-sdk/tests/task_sdk/definitions/test_secrets_masker.py
@kaxil

Copy link
Copy Markdown
Member

Compat test change: #54776

@potiuk

Copy link
Copy Markdown
Member

Merging it 3.0.5 fails clearly because of yanking :) and #54766 is already on the way.

@potiuk
potiuk merged commit 5aec867 into mainAug 21, 2025
74 of 75 checks passed
@potiuk
potiuk deleted the fix-extra-connection-masking branch August 21, 2025 12:40
github-actionsBot pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-0-test

StatusBranchResult
v3-0-testPR Link

kaxil pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
kaxil pushed a commit that referenced this pull request Aug 22, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#54769)
* Ensure that Connection extra can get masked without causing an error
Fixesapache#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of conn.extra_dejson in Airflow 3.0.5 fails with NotImplementedError error

9 participants

@ashb@VladaZakharova@potiuk@amoghrajesh@gopidesupavan@kaxil@dshvedchenko@Lee-W@eladkal
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Ensure that Connection extra can get masked without causing an error - #54769

Merged
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking
Aug 21, 2025
Merged

Ensure that Connection extra can get masked without causing an error#54769
potiuk merged 4 commits into
mainfrom
fix-extra-connection-masking

Conversation

@ashb

@ashbashb commented Aug 21, 2025

Copy link
Copy Markdown
Member

Fixes#54768

This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because Iterable is too open-ended a type

Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.


^ 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 airflow-core/newsfragments.

Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
@VladaZakharova

Copy link
Copy Markdown
Contributor

Hi
thank you for the fix
Did you check this on different types? can you please add unit tests for that?

@potiukpotiuk left a comment

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.

Damn. Pydantic runtime type usage has side effects - that change looks lile it changes nothing....

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Hi thank you for the fix Did you check this on different types? can you please add unit tests for that?

@ashb wrote:

Yes, this should absolutely have unit tests to go with the fix, but a fix is better than nothing, and I'm about to leave on a camping holiday.

I guess we should add unit tests as follow-up without breaking @ashb's plans.

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Anyone hitting this on 3.0.5 andwho doesn't want to rollback to 3.0.4: a work around for now is to manually apply this patch to your Airflow.

If you are using docker to deploy your airflow then something like this should help hopefully:

USER root
RUN apt update && apt install -y patch patchutils
RUN set -ex; \
cd /usr/local/lib/python3.12/site-packages/airflow; \
curl -L https://patch-diff.githubusercontent.com/raw/apache/airflow/pull/54769.patch \
| filterdiff -p1 -i 'task-sdk/src/airflow/*' | patch -p4 -u --verbose
USER airflow

The path to site-packages and the user to switch back to at the end would need adapting to your specific Dockerfile

@ashb

ashb commented Aug 21, 2025

Copy link
Copy Markdown
MemberAuthor

Did you check this on different types? can you please add unit tests for that?

I manually tested it with this:

First set the conneciton:

export AIRFLOW_CONN_TEST='{"conn_type": "google_cloud_default", "extra": {"key_path": "/files/airflow-breeze-config/keys2/keys.json", "scope": "https://www.googleapis.com/auth/cloud-platform", "project": "project_id", "num_retries": 6}}

Than I ran this DAG:

@taskdefmy_function() ->None:
conn=Connection.get("test")
print(f"{conn.conn_id=}{conn.password=}{conn.extra_dejson=}{conn=}")
withDAG("test_dag") asdag:
my_function()

@potiuk

Copy link
Copy Markdown
Member

I manually tested it with this:

I will take it from now on @ashb -> thanks for the repro scenarios

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn!

@amoghrajesh

Copy link
Copy Markdown
Contributor

@ashb mypy will need a fix:

 task-sdk/src/airflow/sdk/execution_time/secrets_masker.py:127: error: Argument "value" to "MaskSecret" has incompatible type "str | Iterable[Any]"; expected "JsonValue" [arg-type]
comms.send(MaskSecret(value=secret, name=name))
^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

@potiuk

Copy link
Copy Markdown
Member

^~~~~~
task-sdk/src/airflow/sdk/execution_time/supervisor.py:1351: error: Argument 1 to "mask_secret" has incompatible type "list[JsonValue] | dict[str, JsonValue] | str | int | float | None"; expected
"str | dict[Any, Any] | Iterable[Any]" [arg-type]
mask_secret(msg.value, msg.name)
^~~~~~~~~
Found 2 errors in 2 files (checked 126 source files)

I'll copy the PR and fix it -> and I will ask others to help :) .. @ashb will not be available to fix it @amoghrajesh

@potiuk

potiuk commented Aug 21, 2025

Copy link
Copy Markdown
Member

Ah... It's in `apache/airflow" so we can push to it directly

@VladaZakharova

Copy link
Copy Markdown
Contributor

Originally, there was also the problem with triggerers with the following errors:

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

do you think we can also fix the types for masks to solve this problem too?

@gopidesupavan

Copy link
Copy Markdown
Member

oh geeee...

@amoghrajesh

Copy link
Copy Markdown
Contributor

@potiuk you can push directly to the PR. No need to copy it.

@potiuk

Copy link
Copy Markdown
Member

@potiuk you can push directly to the PR. No need to copy it.

Yep. Will do

@potiuk

Copy link
Copy Markdown
Member

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

@potiuk

Copy link
Copy Markdown
Member

Let's just push fixups without -f : to not override each-others's changes.

@amoghrajesh

amoghrajesh commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

Dropped you a message on slack, looking into mypy failures

@potiuk

Copy link
Copy Markdown
Member

pushed mypy fix :)

@VladaZakharova

VladaZakharova commented Aug 21, 2025

Copy link
Copy Markdown
Contributor

We can work together @amoghrajesh -> and add more related fixes possibly (looking at the last comment from @VladaZakharova ).

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

@potiuk

Copy link
Copy Markdown
Member

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

@VladaZakharova

Copy link
Copy Markdown
Contributor

will be happy to discuss and help, because now it looks like if we extend this logic with types, maybe we can also add support of Enums and other types that are not standard. Because the current logic only supports limited number of objects. The problem with triggers was the first one, and now with this masking it's just making clear with connections that it doesn't work as expected

Let's focus on getting unit testing for that one first - and then we can see what we can do with Enums - this one is pretty burning :).

agree :)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Agreed too. Let's fix the error at hand first and we can always follow up for the non breaking / burning things...

@potiuk

Copy link
Copy Markdown
Member

fixup with tests is here

@potiuk

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

@amoghrajesh

Copy link
Copy Markdown
Contributor

Yeah I do not think it is directly linked to this PR

@kaxil

Copy link
Copy Markdown
Member

Also @VladaZakharova -> those seem to be unrelated, it seems that simply we would have to add custom serializer to pydantic to handle those enums and classes that are being serialized. How do you arrive at those errors?

dataplex_data_profile: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataplex_data_quality: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_batch_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
dataproc_spark_deferrable: kills the triggerer (NotImplementedError: Objects of type <enum 'State'> are not supported)
example_gcp_transfer: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_transfer_gcs_to_gcs: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)
example_gcp_transfer_aws: kill the triggerer (NotImplementedError: Objects of type <class 'google.cloud.sotrage_transfer_v1.types.transfer_types.TransferOperation'> are not supported)

Yeah unrelated

@amoghrajeshamoghrajesh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me!

Comment threadtask-sdk/tests/task_sdk/definitions/test_secrets_masker.py
@kaxil

Copy link
Copy Markdown
Member

Compat test change: #54776

@potiuk

Copy link
Copy Markdown
Member

Merging it 3.0.5 fails clearly because of yanking :) and #54766 is already on the way.

@potiuk
potiuk merged commit 5aec867 into mainAug 21, 2025
74 of 75 checks passed
@potiuk
potiuk deleted the fix-extra-connection-masking branch August 21, 2025 12:40
github-actionsBot pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
@github-actions

Copy link
Copy Markdown
Contributor

Backport successfully created: v3-0-test

StatusBranchResult
v3-0-testPR Link

kaxil pushed a commit that referenced this pull request Aug 21, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
kaxil pushed a commit that referenced this pull request Aug 22, 2025
…ng an error (#54769) (#54780)
* Ensure that Connection extra can get masked without causing an error
Fixes#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
(cherry picked from commit 5aec867)
Co-authored-by: Ash Berlin-Taylor <ash@apache.org>
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
mangal-vairalkar pushed a commit to mangal-vairalkar/airflow that referenced this pull request Aug 30, 2025
…pache#54769)
* Ensure that Connection extra can get masked without causing an error
Fixesapache#54768
This was caused by pydantic#9541 and improper testing on my part. Sorry
folks. Thsi happens because `Iterable` is too open-ended a type
Yes, this should absolutely have unit tests to go with the fix, but a fix is
better than nothing, and I'm about to leave on a camping holiday.
* fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! Ensure that Connection extra can get masked without causing an error
* fixup! fixup! fixup! Ensure that Connection extra can get masked without causing an error
---------
Co-authored-by: Jarek Potiuk <jarek@potiuk.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use of conn.extra_dejson in Airflow 3.0.5 fails with NotImplementedError error

9 participants

@ashb@VladaZakharova@potiuk@amoghrajesh@gopidesupavan@kaxil@dshvedchenko@Lee-W@eladkal