Skip to content

Mask DFP connection and variable responses#65704

Merged
vincbeck merged 13 commits into
apache:mainfrom
leeyspaul:fix/dfp-mask-connection-variable-responses
May 4, 2026
Merged

Mask DFP connection and variable responses#65704
vincbeck merged 13 commits into
apache:mainfrom
leeyspaul:fix/dfp-mask-connection-variable-responses

Conversation

@leeyspaul
Copy link
Copy Markdown
Contributor

@leeyspaul leeyspaul commented Apr 23, 2026

Please see context in this comment: #65624 (comment) for justification of this small PR and back porting.

Mask DAG File Processor GetConnection and GetVariable responses so returned secrets are registered with the masker before being sent back to parsing code.

Related: #65624

This is intentionally separate from #65624 so the DFP masking fix can be reviewed and backported independently of the request-handler refactor. We can follow up with a separate PR or add another good first issue to handle the refactor after addressing the immediate fix.


Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: Codex GPT 5.4


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

Comment thread airflow-core/src/airflow/dag_processing/processor.py
Comment thread airflow-core/src/airflow/dag_processing/processor.py
@leeyspaul leeyspaul marked this pull request as ready for review April 23, 2026 04:08
@leeyspaul
Copy link
Copy Markdown
Contributor Author

@ferruzzi added the mask here in this separate PR, previous context: #65624 (comment)

@ferruzzi ferruzzi requested review from o-nikolas and vincbeck April 24, 2026 18:56
@leeyspaul
Copy link
Copy Markdown
Contributor Author

The CI failure started after this PR added mask_secret() calls to the GetConnection and GetVariable branches of _handle_request().

That function already had an older function-local import in the later MaskSecret branch:

from airflow.sdk.log import mask_secret

In Python, an import inside a function is a local assignment for the entire function scope. Because of that later import, Python treated all mask_secret references in _handle_request() as local variables, including the new calls that appear before the import. Those earlier calls then failed with UnboundLocalError, and Ruff reported the same issue as F823.

I fixed it by removing the nested import and use the existing module-level mask_secret import consistently. I'm unsure why it passed first, but then later failed on merges. Something I will look into.

Sorry, this PR is now ready to merge. @o-nikolas @vincbeck @ferruzzi

@vincbeck vincbeck merged commit 1d3e134 into apache:main May 4, 2026
79 checks passed
Sign up for free to 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.

4 participants