brie/: add GetVersion function for tidbGlueSession #22731
Conversation
ti-srebot
added
the
first-time-contributor
|
New Glue: // Glue is an abstraction of TiDB function calls used in BR.
type Glue interface {
GetDomain(store kv.Storage) (*domain.Domain, error)
CreateSession(store kv.Storage) (Session, error)
Open(path string, option pd.SecurityOption) (kv.Storage, error)
// OwnsStorage returns whether the storage returned by Open() is owned
// If this method returns false, the connection manager will never close the storage.
OwnsStorage() bool
StartProgress(ctx context.Context, cmdName string, total int64, redirectLog bool) Progress
// Record records some information useful for log-less summary.
Record(name string, value uint64)
// GetVersion gets the code version to run backup/restore job
GetVersion() string
} |
|
/cc 3pointer,kennytm |
| @@ -465,3 +466,7 @@ func (gs *tidbGlueSession) Record(name string, value uint64) { | |||
| gs.info.archiveSize = value | |||
| } | |||
| } | |||
|
|
|||
| func (gs *tidbGlueSession) GetVersion() string { | |||
There was a problem hiding this comment.
Please add a test to make sure it has consistent format in the future release.
|
could you refer the change of BR side here |
Do you mean this? |
br implementation of this method |
I paste the draft PR here. Without TiDB's update br can't merge this PR. https://github.com/pingcap/br/pull/734/files#diff-d811c98bfebfd509023db7c21e45d168e4866b44730fba1623cce73c165753e1R30 |
I think the unit test could also ensure there's rest LGTM |
|
@overvenus, 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: execution(slack). |
|
I want to make sure the version info is added in backupmeta. Besides, we can add this test in pingcap/br#734. |
I mean as in PR description "distinguish TiDB and BR", should check first line there's enough message to distinguish updated: could test in integration test of BR's PR |
|
/lgtm |
|
@lance6716, 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: execution(slack). |
|
/cc crazycs520,AilinKid |
|
/merge |
|
@lichunzhu: It seems you want to merge this PR, I will help you trigger all the tests: /run-all-tests You only need to trigger If you have any questions about the PR merge process, please refer to pr process. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
|
This pull request has been accepted and is ready to merge. Commit hash: ab944e2
|
ti-chi-bot
added
the
status/can-merge
ti-chi-bot
added
the
size/M
Co-authored-by: tangenta <[email protected]>
|
/merge |
|
@lichunzhu: It seems you want to merge this PR, I will help you trigger all the tests: /run-all-tests You only need to trigger If you have any questions about the PR merge process, please refer to pr process. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes. |
|
/test unit-test |
|
/run-unit-test |
|
@lichunzhu: Your PR has out-of-dated, I have automatically updated it for you. At the same time I will also trigger all tests for you: /run-all-tests Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository. |
|
/label needs-cherry-pick-5.0-rc |
|
/label needs-cherry-pick-4.0 |
Signed-off-by: ti-srebot <[email protected]>
|
cherry pick to release-5.0-rc in PR #23142 |
Signed-off-by: ti-srebot <[email protected]>
|
cherry pick to release-4.0 in PR #23143 |
What problem does this PR solve?
Issue Number: pingcap/br#698
Problem Summary: BR will add backup tool version in backup meta. We need a
GetVersionmethod to distinguish TiDB and BR.What is changed and how it works?
What's Changed: Add GetVersion method
Related changes
Check List
Tests
Release note