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

add cdc types #2338

Merged
merged 14 commits into from
May 12, 2020
Merged

add cdc types #2338

merged 14 commits into from
May 12, 2020

Conversation

weekface
Copy link
Contributor

@weekface weekface commented Apr 29, 2020

What problem does this PR solve?

Add ticdc types. Part of #2056

What is changed and how does it work?

Check List

Tests

Code changes

  • Has Go code change

Side effects

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Does this PR introduce a user-facing change?:

Support TiCdc in TidbCluster CR.

@weekface
Copy link
Contributor Author

/run-e2e-tests

@@ -335,6 +339,25 @@ type TiFlashSpec struct {
LogTailer *LogTailerSpec `json:"logTailer,omitempty"`
}

// TiCdcpec contains details of TiCdc members
// +k8s:openapi-gen=true
type TiCdcSpec struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add a TidbClusterRef to indicate this TiCDC's target tidbcluster?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TiCdcSpec is part of TidbCluster, so it doesn't need the TidbClusterRef attribute.

@weekface weekface requested a review from Yisaer May 6, 2020 08:08
Yisaer
Yisaer previously approved these changes May 6, 2020
DanielZhangQD
DanielZhangQD previously approved these changes May 6, 2020
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

@DanielZhangQD
Copy link
Contributor

Can we change Cdc to CDC in the variable and function names? It's a little weird with Cdc.

@weekface weekface dismissed stale reviews from DanielZhangQD and Yisaer via 0fb5768 May 7, 2020 02:20
@weekface weekface requested review from DanielZhangQD and Yisaer May 7, 2020 06:08
weekface and others added 2 commits May 7, 2020 14:20
Co-authored-by: DanielZhangQD <36026334+DanielZhangQD@users.noreply.github.com>
@weekface weekface requested a review from DanielZhangQD May 7, 2020 06:21
DanielZhangQD
DanielZhangQD previously approved these changes May 7, 2020
Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@DanielZhangQD DanielZhangQD left a comment

Choose a reason for hiding this comment

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

LGTM

@weekface
Copy link
Contributor Author

/run-e2e-tests

@weekface
Copy link
Contributor Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented May 12, 2020

Your auto merge job has been accepted, waiting for:

  • 2414
  • 2437
  • 2444
  • 2444

@sre-bot
Copy link
Contributor

sre-bot commented May 12, 2020

/run-all-tests

@sre-bot sre-bot merged commit 4343d28 into pingcap:master May 12, 2020
sre-bot pushed a commit to sre-bot/tidb-operator that referenced this pull request May 12, 2020
@sre-bot sre-bot mentioned this pull request May 12, 2020
@sre-bot
Copy link
Contributor

sre-bot commented May 12, 2020

cherry pick to release-1.1 in PR #2449

cofyc pushed a commit that referenced this pull request May 13, 2020
Co-authored-by: weekface <weekface@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants