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.

Sign up for GitHub

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

Jump to bottom

backupccl: stop logging unsanitized backup stmt in schedule executor #99150

Merged
merged 2 commits into from
Mar 22, 2023
Merged

backupccl: stop logging unsanitized backup stmt in schedule executor #99150

merged 2 commits into from
Mar 22, 2023

Conversation

msbutler
Copy link
Collaborator

Informs #99145

Release note: None

@msbutler msbutler requested a review from dt March 21, 2023 20:17
@msbutler msbutler requested a review from a team as a code owner March 21, 2023 20:17
@msbutler msbutler self-assigned this Mar 21, 2023
@blathers-crl
Copy link

blathers-crl bot commented Mar 21, 2023

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

stevendanna
stevendanna approved these changes Mar 22, 2023
View reviewed changes
Copy link
Collaborator

@stevendanna stevendanna left a comment

Choose a reason for hiding this comment

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

👍 Thanks for fixing this up.

If we wanted to go a bit further, we have some code in this file for creating redacted backup statements. We could print a redacted backup statement rather than just the schedule ID if we saw value in having the whole statement.

But, I personally thing that just the ID is fine.

@msbutler msbutler added the backport-23.1.x Flags PRs that need to be backported to 23.1 label Mar 22, 2023
@msbutler
Copy link
Collaborator Author

bors r=stevendanna

@craig
Copy link
Contributor

craig bot commented Mar 22, 2023

Build failed:

@msbutler
Copy link
Collaborator Author

bors r=retry

@craig
Copy link
Contributor

craig bot commented Mar 22, 2023

Build failed (retrying...):

@msbutler
Copy link
Collaborator Author

bors r-

@craig
Copy link
Contributor

craig bot commented Mar 22, 2023

Canceled.

@msbutler
backupccl: scrub backup stmt from return errors
97243f6 
This patch ensures that no unsanitized uris or secret keys get written to the
jobs table if the backup fails.

Informs cockroachdb#99145

Release note: None
@msbutler msbutler mentioned this pull request Mar 22, 2023
Merged
@msbutler
Copy link
Collaborator Author

bors r=stevendanna

@craig
Copy link
Contributor

craig bot commented Mar 22, 2023

Build succeeded:

@craig craig bot merged commit c2460f1 into cockroachdb:master Mar 22, 2023
This was referenced Mar 22, 2023
Closed
Merged
Merged
@msbutler msbutler deleted the butler-redact-backup-stmt-2 branch March 23, 2023 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dt dt dt approved these changes

@stevendanna stevendanna stevendanna approved these changes

Assignees

@msbutler msbutler

Labels
backport-23.1.x Flags PRs that need to be backported to 23.1 T-disaster-recovery
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@msbutler @cockroach-teamcity @dt @stevendanna