Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dm: support dump task mode in openapi #11729

Merged
merged 9 commits into from
Nov 14, 2024
Merged

Conversation

River2000i
Copy link
Contributor

@River2000i River2000i commented Nov 7, 2024

What problem does this PR solve?

Issue Number: close #9230

What is changed and how it works?

  1. generate go client and server by make dm_generate_openapi
  2. add opanapi test from dump task

Check List

Tests

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

Questions

Will it cause performance regression or break compatibility?
Do you need to update user documentation, design documentation or monitoring documentation?

Release note

Please refer to [Release Notes Language Style Guide](https://pingcap.github.io/tidb-dev-guide/contribute-to-tidb/release-notes-style-guide.html) to write a quality release note.

If you don't think this PR needs a release note then fill it with `None`.

@ti-chi-bot ti-chi-bot bot added do-not-merge/needs-linked-issue release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. area/dm Issues or PRs related to DM. contribution This PR is from a community contributor. first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Nov 7, 2024
Copy link
Contributor

ti-chi-bot bot commented Nov 7, 2024

Hi @River2000i. Thanks for your PR.

I'm waiting for a pingcap member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@ti-chi-bot ti-chi-bot bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Nov 7, 2024
@River2000i River2000i changed the title [WIP]dm: add openapi test in dump and load&sync mode [WIP]dm: add dump and load&sync task mode in openapi Nov 8, 2024
@River2000i River2000i changed the title [WIP]dm: add dump and load&sync task mode in openapi dm: support dump and load&sync task mode in openapi Nov 8, 2024
@ti-chi-bot ti-chi-bot bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 8, 2024
@River2000i
Copy link
Contributor Author

@GMHDBJD @lance6716 @D3Hunter PTAL~

dm/openapi/spec/dm.yaml Outdated Show resolved Hide resolved
@lance6716
Copy link
Contributor

/ok-to-test

@ti-chi-bot ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Nov 11, 2024
@ti-chi-bot ti-chi-bot bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Nov 11, 2024
@River2000i River2000i changed the title dm: support dump and load&sync task mode in openapi dm: support dump task mode in openapi Nov 11, 2024
@River2000i
Copy link
Contributor Author

ut #11361 failed, rerun it

@River2000i
Copy link
Contributor Author

/retest

Copy link

codecov bot commented Nov 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 55.4474%. Comparing base (64decf6) to head (6657afa).
Report is 3 commits behind head on master.

Additional details and impacted files
Components Coverage Δ
cdc 60.0726% <ø> (+0.4924%) ⬆️
dm 50.1461% <ø> (+0.0787%) ⬆️
engine 53.2562% <ø> (+0.0225%) ⬆️
Flag Coverage Δ
unit 55.4474% <ø> (+0.3116%) ⬆️

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

@@               Coverage Diff                @@
##             master     #11729        +/-   ##
================================================
+ Coverage   55.1358%   55.4474%   +0.3116%     
================================================
  Files          1001       1002         +1     
  Lines        136521     137743      +1222     
================================================
+ Hits          75272      76375      +1103     
- Misses        55740      55843       +103     
- Partials       5509       5525        +16     

Copy link
Contributor

@GMHDBJD GMHDBJD left a comment

Choose a reason for hiding this comment

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

rest LGTM

"query-status $task_name" \
"\"stage\": \"Running\"" 1

# wait dump task finish
Copy link
Contributor

Choose a reason for hiding this comment

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

can you check it with get_task_status_check via openapi instead of dmctl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Validating json fields in python is a bit complex, I'll create an issue to improve it later...

@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Nov 14, 2024
@ti-chi-bot ti-chi-bot bot added the lgtm label Nov 14, 2024
Copy link
Contributor

ti-chi-bot bot commented Nov 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: D3Hunter, GMHDBJD

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Nov 14, 2024
Copy link
Contributor

ti-chi-bot bot commented Nov 14, 2024

[LGTM Timeline notifier]

Timeline:

  • 2024-11-14 06:05:38.966616833 +0000 UTC m=+509101.157485823: ☑️ agreed by GMHDBJD.
  • 2024-11-14 06:36:38.043065833 +0000 UTC m=+510960.233934830: ☑️ agreed by D3Hunter.

@River2000i
Copy link
Contributor Author

/hold

@ti-chi-bot ti-chi-bot bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 14, 2024
@River2000i
Copy link
Contributor Author

/unhold

@ti-chi-bot ti-chi-bot bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 14, 2024
@ti-chi-bot ti-chi-bot bot merged commit 68c22f3 into pingcap:master Nov 14, 2024
28 checks passed
@River2000i River2000i deleted the fixOpenapi branch November 14, 2024 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved area/dm Issues or PRs related to DM. contribution This PR is from a community contributor. first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. lgtm ok-to-test Indicates a PR is ready to be tested. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

engine: support new dm task mode
4 participants