Skip to content
This repository was archived by the owner on Nov 24, 2023. It is now read-only.

feat: add get-task-config command#798

Merged
GMHDBJD merged 14 commits into
pingcap:masterfrom
GMHDBJD:retrieveConfig
Jul 17, 2020
Merged

feat: add get-task-config command#798
GMHDBJD merged 14 commits into
pingcap:masterfrom
GMHDBJD:retrieveConfig

Conversation

@GMHDBJD
Copy link
Copy Markdown
Collaborator

@GMHDBJD GMHDBJD commented Jul 10, 2020 •
edited
Loading

What problem does this PR solve?

close #182

What is changed and how it works?

  • save the task-config-string when start-task
  • remove the task-config-string when stop-task
  • get the task-config-string and write the cfg to file(option)
  • add a step by step contribute example

Tests

  • Unit test
  • Integration test

@GMHDBJD GMHDBJD added priority/normal Minor change, requires approval from ≥1 primary reviewer status/WIP This PR is still work in progress type/feature-request This issue is a feature request type/feature New feature and removed type/feature-request This issue is a feature request labels Jul 10, 2020
@GMHDBJD
Copy link
Copy Markdown
Collaborator Author

GMHDBJD commented Jul 13, 2020

/run-all-tests

@GMHDBJD GMHDBJD added status/PTAL This PR is ready for review. Add this label back after committing new changes and removed status/WIP This PR is still work in progress labels Jul 14, 2020
lance6716
lance6716 reviewed Jul 14, 2020
View reviewed changes
Comment thread dm/ctl/master/get_task_config.go Outdated
Comment thread dm/master/scheduler/scheduler.go
lance6716
lance6716 reviewed Jul 14, 2020
View reviewed changes
Copy link
Copy Markdown
Collaborator

@lance6716 lance6716 left a comment

Choose a reason for hiding this comment

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

LGTM

@lance6716 lance6716 added status/LGT1 One reviewer already commented LGTM and removed status/PTAL This PR is ready for review. Add this label back after committing new changes labels Jul 14, 2020
WangXiangUSTC
WangXiangUSTC reviewed Jul 17, 2020
View reviewed changes
Comment thread step_by_step_contribute.md Outdated
Comment thread step_by_step_contribute.md Outdated
Comment thread step_by_step_contribute.md Outdated
csuzhangxc
csuzhangxc reviewed Jul 17, 2020
View reviewed changes
Comment thread dm/ctl/master/get_task_config.go Outdated
Comment thread dm/master/scheduler/scheduler.go Outdated
Comment thread pkg/terror/error_list.go Outdated
Comment thread step_by_step_contribute.md Outdated
@GMHDBJD
Copy link
Copy Markdown
Collaborator Author

GMHDBJD commented Jul 17, 2020

All comment have been addressed. PTAL again

csuzhangxc
csuzhangxc reviewed Jul 17, 2020
View reviewed changes
Comment thread step_by_step_contribute.md Outdated
Comment thread step_by_step_contribute.md
@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 17, 2020

Codecov Report

Merging #798 into master will decrease coverage by 0.8179%.
The diff coverage is 37.8708%.

@@               Coverage Diff                @@
##             master       #798        +/-   ##
================================================
- Coverage   57.0981%   56.2802%   -0.8180%     
================================================
  Files           205        211         +6     
  Lines         21104      21894       +790     
================================================
+ Hits          12050      12322       +272     
- Misses         7890       8368       +478     
- Partials       1164       1204        +40

csuzhangxc
csuzhangxc approved these changes Jul 17, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

LGTM

@csuzhangxc csuzhangxc added status/LGT2 Two reviewers already commented LGTM, ready for merge and removed status/LGT1 One reviewer already commented LGTM labels Jul 17, 2020
@GMHDBJD GMHDBJD merged commit 2c30a80 into pingcap:master Jul 17, 2020
@GMHDBJD GMHDBJD deleted the retrieveConfig branch July 17, 2020 08:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@lance6716 lance6716 lance6716 left review comments

@WangXiangUSTC WangXiangUSTC WangXiangUSTC left review comments

@csuzhangxc csuzhangxc csuzhangxc approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge type/feature New feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

UCP: retrieve the configuration of the running task from the DM cluster

4 participants

@GMHDBJD @lance6716 @csuzhangxc @WangXiangUSTC