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

cluster/import: read enable_binlog from inventory #652

Merged
merged 2 commits into from
Aug 10, 2020

Conversation

AstroProfundis
Copy link
Contributor

@AstroProfundis AstroProfundis commented Aug 4, 2020

What problem does this PR solve?

The enable_binlog switch in ansible inventory is not imported.

What is changed and how it works?

Read the variable and set tidb config if it is set to True.

Release Note

Supporting import clusters with binlog enabled

@AstroProfundis AstroProfundis added the type/bug-fix Categorizes PR as a bug-fix label Aug 4, 2020
@AstroProfundis AstroProfundis requested a review from lucklove August 4, 2020 08:43
@AstroProfundis AstroProfundis self-assigned this Aug 4, 2020
Copy link
Member

@lucklove lucklove left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 4, 2020
Copy link
Contributor

@july2993 july2993 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Aug 4, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Aug 4, 2020
@codecov-commenter
Copy link

Codecov Report

Merging #652 into master will increase coverage by 0.63%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #652      +/-   ##
==========================================
+ Coverage   56.71%   57.34%   +0.63%     
==========================================
  Files         242      243       +1     
  Lines       17469    17512      +43     
==========================================
+ Hits         9908    10043     +135     
+ Misses       6265     6167      -98     
- Partials     1296     1302       +6     
Flag Coverage Δ
#coverage 57.34% <ø> (+0.63%) ⬆️

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

Impacted Files Coverage Δ
...c/github.com/pingcap/tiup/pkg/cluster/spec/spec.go 90.94% <0.00%> (-2.62%) ⬇️
...hub.com/pingcap/tiup/pkg/cluster/ansible/import.go 48.07% <0.00%> (-1.93%) ⬇️
...m/pingcap/tiup/components/cluster/command/check.go 81.63% <0.00%> (-0.50%) ⬇️
...src/github.com/pingcap/tiup/pkg/cluster/manager.go 70.37% <0.00%> (-0.20%) ⬇️
....com/pingcap/tiup/pkg/cluster/spec/spec_manager.go 67.50% <0.00%> (-0.15%) ⬇️
...c/github.com/pingcap/tiup/pkg/cluster/api/dmapi.go 61.31% <0.00%> (ø)
....com/pingcap/tiup/pkg/cluster/operation/destroy.go 54.48% <0.00%> (ø)
...com/pingcap/tiup/pkg/cluster/spec/server_config.go 65.87% <0.00%> (ø)
....com/pingcap/tiup/pkg/cluster/clusterutil/retry.go
...ub.com/pingcap/tiup/pkg/cliutil/prepare/prepare.go
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80da63c...406d7ed. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/LGT2 Indicates that a PR has LGTM 2. type/bug-fix Categorizes PR as a bug-fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants