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: add subcommand pull and push to transfer files #1044

Merged
merged 7 commits into from
Jan 8, 2021

Conversation

AstroProfundis
Copy link
Contributor

What problem does this PR solve?

Add a pair of subcommand pull/push to cluster, these adb style commands are used to transfer files from or to servers in the cluster.

Fields in node component specification can be used as template for remote paths.

Check List

Tests

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

Code changes

  • Has exported function/method change

Side effects

  • Increased code complexity

Release notes:

cluster: add `pull`/`push` (hidden) subcommand to transfer files from or to remote servers

@AstroProfundis AstroProfundis added type/new-feature Categorizes pr as related to a new feature. category/usability Categorizes issue or PR as a usability enhancement. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. labels Jan 6, 2021
@AstroProfundis AstroProfundis added this to the v1.4.0 milestone Jan 6, 2021
@AstroProfundis AstroProfundis self-assigned this Jan 6, 2021
@ti-chi-bot ti-chi-bot requested a review from lonng January 6, 2021 11:41
@ti-chi-bot ti-chi-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jan 6, 2021
@codecov-io
Copy link

codecov-io commented Jan 6, 2021

Codecov Report

Merging #1044 (50b5b49) into master (b5fe5e8) will increase coverage by 0.11%.
The diff coverage is 67.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1044      +/-   ##
==========================================
+ Coverage   55.80%   55.92%   +0.11%     
==========================================
  Files         280      282       +2     
  Lines       19759    19860     +101     
==========================================
+ Hits        11026    11106      +80     
- Misses       7029     7032       +3     
- Partials     1704     1722      +18     
Flag Coverage Δ
cluster 44.01% <66.99%> (+0.23%) ⬆️
dm 23.95% <4.28%> (-0.16%) ⬇️
integrate 50.23% <67.92%> (+0.14%) ⬆️
playground 20.31% <ø> (+0.01%) ⬆️
tiup 16.48% <ø> (ø)
unittest 22.24% <21.69%> (-0.04%) ⬇️

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

Impacted Files Coverage Δ
pkg/cluster/manager/transfer.go 55.22% <55.22%> (ø)
components/cluster/command/transfer.go 86.66% <86.66%> (ø)
components/cluster/command/exec.go 86.66% <100.00%> (+0.95%) ⬆️
components/cluster/command/root.go 46.10% <100.00%> (+0.65%) ⬆️
components/dm/command/exec.go 85.71% <100.00%> (+1.09%) ⬆️
pkg/cluster/api/dmapi.go 58.26% <0.00%> (-1.74%) ⬇️
pkg/cluster/api/pdapi.go 60.56% <0.00%> (-1.58%) ⬇️
pkg/cluster/task/builder.go 100.00% <0.00%> (+2.89%) ⬆️
... and 2 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 b5fe5e8...50b5b49. Read the comment docs.

@AstroProfundis AstroProfundis marked this pull request as ready for review January 7, 2021 08:09
@ti-chi-bot 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 Jan 7, 2021
@lucklove
Copy link
Member

lucklove commented Jan 7, 2021

Please add some test for this.

}

func renderSpec(t string, s interface{}, id string) (string, error) {
tpl, err := template.New(id).Option("missingkey=error").Parse(t)
Copy link
Member

Choose a reason for hiding this comment

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

I have write a playground, this should work: https://play.golang.org/p/ZP1fQgRmkFM

@lucklove
Copy link
Member

lucklove commented Jan 8, 2021

/lgtm

@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 8, 2021
@lucklove
Copy link
Member

lucklove commented Jan 8, 2021

/merge

@ti-chi-bot
Copy link
Member

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

Commit hash: 50b5b49

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 8, 2021
@ti-chi-bot ti-chi-bot merged commit 65f061d into pingcap:master Jan 8, 2021
@lucklove lucklove mentioned this pull request Jan 8, 2021
@AstroProfundis AstroProfundis deleted the subcmd-transfer branch January 19, 2021 03:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/usability Categorizes issue or PR as a usability enhancement. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/new-feature Categorizes pr as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants