Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Don't fail to log if we can't redact something #16118

Merged
merged 1 commit into from Jun 7, 2021

Conversation

ashb
Copy link
Member

@ashb ashb commented May 27, 2021

Rather than dying with an exception, catch it and warn about that, asking users to report it to us.

Closes #16071 in the general case, and in the specific write-only IO object that caused the exception.


^ Add meaningful description above

Read the Pull Request Guidelines for more information.
In case of fundamental code change, 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 UPDATING.md.

@ashb ashb requested review from jhtimmins, kaxil and potiuk May 27, 2021 14:54
@ashb ashb added this to the Airflow 2.1.1 milestone May 27, 2021
@ashb
Copy link
Member Author

ashb commented May 27, 2021

/cc @madison-ookla

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Nice. Redacting NonRedactable!

@github-actions
Copy link

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label May 27, 2021
@madison-ookla
Copy link
Contributor

Awesome, thanks @ashb!

@ashb ashb force-pushed the log-masking-non-str-args branch from 9763858 to ed7cc4e Compare June 1, 2021 12:36
Rather than dying with an exception, catch it and warn about that,
asking users to report it to us.

Additionally handle the specific case where a file handle/IO object is
logged -- we definitely don't want to iterate over that!
@ashb ashb force-pushed the log-masking-non-str-args branch from ed7cc4e to 3bf83df Compare June 5, 2021 10:02
@ashb ashb added the type:bug-fix Changelog: Bug Fixes label Jun 5, 2021
@ashb ashb merged commit 57bd6fb into apache:main Jun 7, 2021
@ashb ashb deleted the log-masking-non-str-args branch June 7, 2021 08:27
jhtimmins pushed a commit that referenced this pull request Jun 10, 2021
Rather than dying with an exception, catch it and warn about that,
asking users to report it to us.

Additionally handle the specific case where a file handle/IO object is
logged -- we definitely don't want to iterate over that!

(cherry picked from commit 57bd6fb)
ashb added a commit that referenced this pull request Jun 22, 2021
Rather than dying with an exception, catch it and warn about that,
asking users to report it to us.

Additionally handle the specific case where a file handle/IO object is
logged -- we definitely don't want to iterate over that!

(cherry picked from commit 57bd6fb)
kaxil pushed a commit to astronomer/airflow that referenced this pull request Jun 22, 2021
Rather than dying with an exception, catch it and warn about that,
asking users to report it to us.

Additionally handle the specific case where a file handle/IO object is
logged -- we definitely don't want to iterate over that!

(cherry picked from commit 57bd6fb)
(cherry picked from commit 7603ef6)
kaxil pushed a commit to astronomer/airflow that referenced this pull request Jun 23, 2021
Rather than dying with an exception, catch it and warn about that,
asking users to report it to us.

Additionally handle the specific case where a file handle/IO object is
logged -- we definitely don't want to iterate over that!

(cherry picked from commit 57bd6fb)
(cherry picked from commit 7603ef6)
(cherry picked from commit 41ae709)
kaxil pushed a commit to astronomer/airflow that referenced this pull request Jun 23, 2021
Rather than dying with an exception, catch it and warn about that,
asking users to report it to us.

Additionally handle the specific case where a file handle/IO object is
logged -- we definitely don't want to iterate over that!

(cherry picked from commit 57bd6fb)
(cherry picked from commit 7603ef6)
(cherry picked from commit 41ae709)
(cherry picked from commit ae0d119)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:logging full tests needed We need to run full set of tests for this PR to merge type:bug-fix Changelog: Bug Fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Secret masking fails on io objects
4 participants