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

*: check cluster version #570

Merged
merged 3 commits into from
May 15, 2020
Merged

Conversation

overvenus
Copy link
Member

@overvenus overvenus commented May 13, 2020

What problem does this PR solve?

Add version check when starting CDC server and CDC client. It impls #542 task 9.

It also passes context to PD client and may help #562 .

Fix #564

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Support validate cluster version

Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus
Copy link
Member Author

/run-integration-tests

@overvenus
Copy link
Member Author

/run-integration-tests

1 similar comment
@overvenus
Copy link
Member Author

/run-integration-tests

@codecov-io
Copy link

Codecov Report

Merging #570 into master will increase coverage by 1.4906%.
The diff coverage is 35.2112%.

@@               Coverage Diff                @@
##             master       #570        +/-   ##
================================================
+ Coverage   30.8896%   32.3802%   +1.4906%     
================================================
  Files            71         70         -1     
  Lines          7857       6995       -862     
================================================
- Hits           2427       2265       -162     
+ Misses         5265       4560       -705     
- Partials        165        170         +5     

Signed-off-by: Neil Shen <overvenus@gmail.com>
@overvenus
Copy link
Member Author

/run-all-tests

1 similar comment
@overvenus
Copy link
Member Author

/run-all-tests

req, err := http.NewRequestWithContext(
ctx, http.MethodGet, fmt.Sprintf("%s/pd/api/v1/version", pdHTTP), nil)
if err != nil {
return errors.Annotatef(err, "fail to request PD 1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return errors.Annotatef(err, "fail to request PD 1")
return errors.Annotate(err, "fail to request PD 1")

ditto for other errors follows

Signed-off-by: Neil Shen <overvenus@gmail.com>
Copy link
Contributor

@amyangfei amyangfei left a comment

Choose a reason for hiding this comment

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

LGTM

@amyangfei amyangfei added the LGT1 label May 15, 2020
@overvenus
Copy link
Member Author

/run-integration-tests

@overvenus overvenus merged commit 64df3f2 into pingcap:master May 15, 2020
@overvenus overvenus deleted the cli/versioncheck branch May 15, 2020 08:30
5kbpers pushed a commit to 5kbpers/ticdc that referenced this pull request Aug 24, 2020
Signed-off-by: Neil Shen <overvenus@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

optimization for changefeed remove
3 participants