openapi(dm): add task config template releated openapi #3656

Ehco1996 · 2021-11-29T07:38:05Z

What problem does this PR solve?

subtask of https://github.com/pingcap/ticdc/issues/3583

What is changed and how it works?

as title, need https://github.com/pingcap/ticdc/pull/3620 merged first

Check List

Tests

Unit test

Code changes

Has exported function/method change

Side effects

Possible performance regression
Increased code complexity

Related changes

Need to update the documentation

Release note

`add task config releated openapi`.

ti-chi-bot · 2021-11-29T07:38:07Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

lichunzhu
okJiang

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Ehco1996 · 2021-12-14T09:55:54Z

/hold

need add new batch import api after meeting with PM😅

Ehco1996 · 2021-12-14T10:23:08Z

dm/dm/master/openapi.go

@@ -822,6 +823,123 @@ func (s *Server) DMAPIOperateTableStructure(c *gin.Context, taskName string, sou
 	}
 }

+// DMAPIBatchImportTaskConfig create task_config_template url is: (POST /api/v1/task_configs/batch_import).
+func (s *Server) DMAPIBatchImportTaskConfig(c *gin.Context) {


this api is for this feature

Ehco1996 · 2021-12-14T10:27:19Z

dm/openapi/spec/dm.yaml

+              schema:
+                $ref: "#/components/schemas/ErrorWithMessage"
+
+  /api/v1/task_configs:


maybe use this path pattern ? 🤔

list/create api/vi/tasks/configs?

batch_import api/vi/tasks/configs/batch_import

delete/get/update api/vi/tasks/{task-name}/configs

need suggestion from other reviewers

My point:
list/create api/vi/tasks/configs?
batch_import api/vi/tasks/configs/import
delete/get/update api/vi/tasks/configs/{task-name}

addressed in f585588
ptal

Ehco1996 · 2021-12-14T10:28:20Z

/unhold

could you please take another look for this pr ? thanks 🧡🧡🧡

/cc @glorv @okJiang @lance6716

Ehco1996 · 2021-12-14T10:31:17Z

please check this api spec of task config if the api is sufficient for webui

/cc @zoubingwu

ti-chi-bot · 2021-12-14T10:31:18Z

@Ehco1996: GitHub didn't allow me to request PR reviews from the following users: zoubingwu.

Note that only pingcap members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

please check this api spec of task config if the api is sufficient for webui

/cc @zoubingwu

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 kubernetes/test-infra repository.

codecov-commenter · 2021-12-15T00:09:03Z

Codecov Report

Merging #3656 (f585588) into master (3873d39) will decrease coverage by 2.1798%.
The diff coverage is 61.8644%.

Flag	Coverage Δ
cdc	`58.2864% <61.8644%> (+0.0498%)`	⬆️
dm	`52.0561% <ø> (-3.9785%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@@               Coverage Diff                @@
##             master      #3656        +/-   ##
================================================
- Coverage   57.0741%   54.8943%   -2.1799%     
================================================
  Files           478        484         +6     
  Lines         56551      59671      +3120     
================================================
+ Hits          32276      32756       +480     
- Misses        20978      23596      +2618     
- Partials       3297       3319        +22

okJiang

rest lgtm

dm/openapi/spec/dm.yaml

Ehco1996 · 2021-12-20T06:33:56Z

could you please take another look for this pr ? thanks 🧡🧡🧡

@glorv @lichunzhu @GMHDBJD

lichunzhu

Rest LGTM

lichunzhu · 2021-12-20T09:38:04Z

dm/openapi/spec/dm.yaml

+              schema:
+                $ref: "#/components/schemas/ErrorWithMessage"
+
+  /api/v1/task_configs:


My point:
list/create api/vi/tasks/configs?
batch_import api/vi/tasks/configs/import
delete/get/update api/vi/tasks/configs/{task-name}

dm/openapi/spec/dm.yaml

lichunzhu

LGTM

Ehco1996 · 2021-12-21T05:31:48Z

/merge

ti-chi-bot · 2021-12-21T05:31:50Z

This pull request has been accepted and is ready to merge.

Commit hash: 6634f5f

lance6716 · 2021-12-21T06:11:52Z

/run-integration-test
/run-kafka-integration-test

Ehco1996 · 2021-12-21T07:11:12Z

/run-integration-test
/run-kafka-integration-test

Ehco1996 · 2021-12-21T07:25:08Z

/run-kafka-integration-test

Ehco1996 · 2021-12-21T07:36:00Z

/run-integration-test

Ehco1996 · 2021-12-21T07:36:12Z

/run-kafka-integration-test

Ehco1996 · 2021-12-21T07:47:44Z

/run-kafka-integration-test /tidb=pr/30773
/run-integration-tests /tidb=pr/30773

Ehco1996 added the area/dm Issues or PRs related to DM. label Nov 29, 2021

Ehco1996 mentioned this pull request Nov 29, 2021

Tracking issue for DM WebUI #3583

Closed

14 tasks

Ehco1996 marked this pull request as ready for review December 14, 2021 07:47

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 14, 2021

ti-chi-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 14, 2021

Ehco1996 commented Dec 14, 2021

View reviewed changes

ti-chi-bot requested review from glorv, lance6716 and okJiang December 14, 2021 10:28

ti-chi-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 14, 2021

openapi(dm): add task config api

700f2d5

okJiang reviewed Dec 15, 2021

View reviewed changes

dm/openapi/spec/dm.yaml Show resolved Hide resolved

okJiang approved these changes Dec 16, 2021

View reviewed changes

ti-chi-bot added status/LGT1 Indicates that a PR has LGTM 1. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 16, 2021

Merge branch 'master' into openapi-config-template-add-api

fb7d388

ti-chi-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 20, 2021

Ehco1996 added the status/ptal Could you please take a look? label Dec 20, 2021

Merge branch 'master' into openapi-config-template-add-api

e47ae98

fix failpoint

a3030d2

lichunzhu reviewed Dec 20, 2021

View reviewed changes

address comment

f585588

lichunzhu approved these changes Dec 21, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 21, 2021

Merge branch 'master' into openapi-config-template-add-api

6634f5f

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 21, 2021

Merge branch 'master' into openapi-config-template-add-api

3b6eeaf

Ehco1996 mentioned this pull request Dec 21, 2021

dmctl(dm): add config task-template #3991

Merged

ti-chi-bot merged commit 9ef831f into pingcap:master Dec 21, 2021

zhaoxinyu pushed a commit to zhaoxinyu/ticdc that referenced this pull request Dec 29, 2021

openapi(dm): add task config template releated openapi (pingcap#3656)

52bfb85

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

openapi(dm): add task config template releated openapi #3656

openapi(dm): add task config template releated openapi #3656

Ehco1996 commented Nov 29, 2021 •

edited

Loading

ti-chi-bot commented Nov 29, 2021 •

edited

Loading

Ehco1996 commented Dec 14, 2021

Ehco1996 Dec 14, 2021 •

edited

Loading

Ehco1996 Dec 14, 2021 •

edited

Loading

lichunzhu Dec 20, 2021

Ehco1996 Dec 21, 2021

Ehco1996 commented Dec 14, 2021

Ehco1996 commented Dec 14, 2021

ti-chi-bot commented Dec 14, 2021

codecov-commenter commented Dec 15, 2021 •

edited

Loading

okJiang left a comment

Ehco1996 commented Dec 20, 2021

lichunzhu left a comment

lichunzhu Dec 20, 2021

lichunzhu left a comment

Ehco1996 commented Dec 21, 2021

ti-chi-bot commented Dec 21, 2021

lance6716 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

openapi(dm): add task config template releated openapi #3656

openapi(dm): add task config template releated openapi #3656

Conversation

Ehco1996 commented Nov 29, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Nov 29, 2021 • edited Loading

Ehco1996 commented Dec 14, 2021

Ehco1996 Dec 14, 2021 • edited Loading

Choose a reason for hiding this comment

Ehco1996 Dec 14, 2021 • edited Loading

Choose a reason for hiding this comment

lichunzhu Dec 20, 2021

Choose a reason for hiding this comment

Ehco1996 Dec 21, 2021

Choose a reason for hiding this comment

Ehco1996 commented Dec 14, 2021

Ehco1996 commented Dec 14, 2021

ti-chi-bot commented Dec 14, 2021

codecov-commenter commented Dec 15, 2021 • edited Loading

Codecov Report

okJiang left a comment

Choose a reason for hiding this comment

Ehco1996 commented Dec 20, 2021

lichunzhu left a comment

Choose a reason for hiding this comment

lichunzhu Dec 20, 2021

Choose a reason for hiding this comment

lichunzhu left a comment

Choose a reason for hiding this comment

Ehco1996 commented Dec 21, 2021

ti-chi-bot commented Dec 21, 2021

lance6716 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Dec 21, 2021

Ehco1996 commented Nov 29, 2021 •

edited

Loading

ti-chi-bot commented Nov 29, 2021 •

edited

Loading

Ehco1996 Dec 14, 2021 •

edited

Loading

Ehco1996 Dec 14, 2021 •

edited

Loading

codecov-commenter commented Dec 15, 2021 •

edited

Loading