Skip to content

*: redact arguments for Error#1051

Merged
bb7133 merged 5 commits intopingcap:masterfrom
SunRunAway:logredact
Oct 21, 2020
Merged

*: redact arguments for Error#1051
bb7133 merged 5 commits intopingcap:masterfrom
SunRunAway:logredact

Conversation

@SunRunAway
Copy link
Contributor

@SunRunAway SunRunAway commented Oct 13, 2020 •
edited
Loading

What problem does this PR solve?

part of pingcap/tidb#18566

What is changed and how it works?

  • changes in "github.com/pingcap/errors" Redact arguments for Error errors#27
    • redact errors in Error.GenWithStackByArgs & Error.FastGenByArgs
  • changes in "github.com/pingcap/parser" *: redact arguments for Error #1051
    • define mysql.Message to bind redactArgs which means which arguments of the error need to be redacted with each error message
    • MySQLErrName should be defined with mysql.Message.
    • ErrClass.NewStd considers redactArgs
    • All errors uses ErrClass.NewStd or mysql.Message to consider error redaction
  • changes in "github.com/pingcap/tidb" *: redact arguments for Error tidb#20436
    • inherit ErrClass.NewStd
    • All errors uses ErrClass.NewStd or mysql.Message to consider error redaction

Check List

Tests

  • Unit test
  • Manual test (add detailed scripts or steps below)

Code changes

  • Has exported function/method change

@SunRunAway SunRunAway marked this pull request as ready for review October 14, 2020 11:40
crazycs520
crazycs520 reviewed Oct 15, 2020
View reviewed changes
Copy link
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

lgtm

@ti-srebot ti-srebot added the status/LGT1 LGT1 label Oct 15, 2020
ichn-hu
ichn-hu previously approved these changes Oct 15, 2020
View reviewed changes
Copy link
Contributor

@ichn-hu ichn-hu left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot
Copy link
Contributor

ti-srebot commented Oct 15, 2020

@ichn-hu, Thanks for your review. The bot only counts LGTMs from Reviewers and higher roles, but you're still welcome to leave your comments. See the corresponding SIG page for more information. Related SIG: ddl(slack).

@SunRunAway SunRunAway mentioned this pull request Oct 19, 2020
Merged
@ti-srebot ti-srebot removed the status/LGT1 LGT1 label Oct 21, 2020
@ti-srebot ti-srebot added the status/LGT2 LGT2 label Oct 21, 2020
@bb7133 bb7133 merged commit 783a032 into pingcap:master Oct 21, 2020
@SunRunAway SunRunAway deleted the logredact branch October 21, 2020 06:25
lonng pushed a commit that referenced this pull request Nov 10, 2020
@SunRunAway @lonng
*: redact arguments for Error (#1051)
5fa9f22
@lonng lonng mentioned this pull request Nov 10, 2020
Merged
crazycs520 pushed a commit that referenced this pull request Nov 10, 2020
@lonng @SunRunAway
*: redact arguments for Error (#1051) (#1083)
77ed358 
Co-authored-by: Feng Liyuan <darktemplar.f@gmail.com>
@ti-srebot ti-srebot mentioned this pull request Nov 17, 2020
Closed
tiancaiamao pushed a commit to tiancaiamao/parser that referenced this pull request Apr 27, 2021
@SunRunAway
*: redact arguments for Error (pingcap#1051)
35280b8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crazycs520 crazycs520 crazycs520 left review comments

@bb7133 bb7133 bb7133 approved these changes

@ti-srebot ti-srebot ti-srebot approved these changes

@ichn-hu ichn-hu ichn-hu left review comments

Assignees

No one assigned

Labels

status/LGT2 LGT2

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@SunRunAway @ti-srebot @bb7133 @crazycs520 @ichn-hu