sql/schmachanger: use more friendly language for fallback #98112
Conversation
|
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. |
|
This change is |
fqazi
force-pushed
the
scLessSevereMessages
branch
from
46c1f5c to
49192c1
Compare
| @@ -75,7 +75,7 @@ func (el EventLogger) HandlePanicAndLogError(ctx context.Context, err *error) { | |||
| log.InfofDepth(ctx, depth, "done %s in %s", el.msg, redact.Safe(timeutil.Since(el.start))) | |||
| } | |||
| case HasNotImplemented(*err): | |||
| log.InfofDepth(ctx, depth, "failed %s with error: %v", el.msg, *err) | |||
| log.InfofDepth(ctx, depth, "unsuccessful in %s with message: %v", el.msg, *err) | |||
There was a problem hiding this comment.
How about something more straightforward likedeclarative schema changer fallback %s with message:, unsuccessful still sounds erroneous heh.
There was a problem hiding this comment.
Adjusted to get rid of unsuccessful (the way the strings are joined doesn't help):
falling back from building declarative schema change targets for GRANT with message: ‹*tree.Grant not implemented in the new schema changer›
There was a problem hiding this comment.
Can we just get rid of this log message altogether? I don't think we get a ton of value from it. How about:
log.VEventfDepth(ctx, depth, 1, "declarative schema changer does not support %s: %v", el.msg, *err)
There was a problem hiding this comment.
I'll go with that approach.
fqazi
force-pushed
the
scLessSevereMessages
branch
from
49192c1 to
160d005
Compare
Previously, the declarative schema changer used slightly alarming language when falling back. To address this, this patch removes the word failure and uses softer language. Also the logging level is reduced to remove this message by default. Fixes: cockroachdb#97922 Release note: None
fqazi
force-pushed
the
scLessSevereMessages
branch
from
160d005 to
420a0aa
Compare
There was a problem hiding this comment.
Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @chengxiong-ruan)
There was a problem hiding this comment.
Let's backport this
Reviewable status:
|
@ajwerner TFTR! bors r+ |
|
Build succeeded: |
|
blathers backport 22.2 |
Previously, the declarative schema changer used slightly
alarming language when falling back. To address this,
this patch removes the word failure and uses softer
language. Also, the logging level is reduced to remove
this message by default.
Fixes: #97922
Release note: None