Skip to content

Add utilities for copying files. #375

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 17, 2018
Merged

Add utilities for copying files. #375

merged 2 commits into from
Oct 17, 2018

Conversation

brendandburns
Copy link
Contributor

Closes #353

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Sep 27, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns

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

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 27, 2018
@brendandburns brendandburns mentioned this pull request Sep 27, 2018
Closed
@karthikkondapally
Copy link
Contributor

@brendandburns
looks good to me.
may be we can add todo statements for

  1. is tar available.
  2. and can function names be like copyFileFrom/copyDirectoryFrom so that in future we can also add some thing like copyFileTo/copyDirectoryTo ?

adding copyFileTo Pod may not be appropriate as it defeats the purpose of docker / kubernetes. but may be for testing people can use it.

@brendandburns
Copy link
Contributor Author

Comments addressed, please re-check.

Thanks!

@brendandburns
Copy link
Contributor Author

@kondapally1989 I added you as a member of the repo, so when it looks good you should be able to say '/lgtm' and it will then merge automatically.

Thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 29, 2018
karthikkondapally
karthikkondapally reviewed Sep 29, 2018
View reviewed changes
util/src/main/java/io/kubernetes/client/Copy.java Outdated
public static void copyFile(String namespace, String pod, String srcPath, Path dest)
throws ApiException, IOException {
Copy c = new Copy();
InputStream is = c.copyFile(namespace, pod, null, srcPath);
Copy link
Contributor

@karthikkondapally karthikkondapally Sep 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

i think travis build is throwing error here , string to Path.
is it copyFileFromPath ?

karthikkondapally
karthikkondapally reviewed Sep 29, 2018
View reviewed changes
util/src/main/java/io/kubernetes/client/Copy.java Outdated

public InputStream copyFileFromPod(String namespace, String pod, String srcPath)
throws ApiException, IOException {
return copyFile(namespace, pod, null, srcPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

is it copyFileFromPod ?

karthikkondapally
karthikkondapally reviewed Sep 29, 2018
View reviewed changes
util/src/main/java/io/kubernetes/client/Copy.java Outdated
}

public InputStream copyFileFromPod(V1Pod pod, String srcPath) throws ApiException, IOException {
return copyFile(pod, null, srcPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

is it copyFileFromPod ?

karthikkondapally
karthikkondapally reviewed Sep 29, 2018
View reviewed changes
util/src/main/java/io/kubernetes/client/Copy.java Outdated

public InputStream copyFileFromPod(V1Pod pod, String container, String srcPath)
throws ApiException, IOException {
return copyFile(
Copy link
Contributor

Choose a reason for hiding this comment

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

is it copyFileFromPod ?

karthikkondapally
karthikkondapally reviewed Sep 29, 2018
View reviewed changes
util/src/main/java/io/kubernetes/client/Copy.java
}
}

public void copyDirectoryFromPod(V1Pod pod, String srcPath, Path destination)
Copy link
Contributor

Choose a reason for hiding this comment

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

is it copyDirectoryToPod ?

karthikkondapally
karthikkondapally reviewed Sep 29, 2018
View reviewed changes
util/src/main/java/io/kubernetes/client/Copy.java Outdated

public void copyDirectoryFromPod(String namespace, String pod, String srcPath, Path destination)
throws ApiException, IOException {
copyDirectoryToPod(namespace, pod, null, srcPath, destiation);
Copy link
Contributor

Choose a reason for hiding this comment

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

destination

karthikkondapally
karthikkondapally reviewed Sep 29, 2018
View reviewed changes
util/src/main/java/io/kubernetes/client/Copy.java Outdated
public static void copyFile(String namespace, String pod, String srcPath, Path dest)
throws ApiException, IOException {
Copy c = new Copy();
InputStream is = c.copyFile(namespace, pod, null, srcPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

copyFileFromPod?

@karthikkondapally
Copy link
Contributor

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 29, 2018
@karthikkondapally
Copy link
Contributor

/hold cancel
i did /lgtm in one of previous comment but deleted the comment as ci build broke and added hold.

@brendandburns once you fix those compile issues, i think it will automatically merge

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 2, 2018
@brendandburns brendandburns mentioned this pull request Oct 16, 2018
Closed
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 16, 2018
@brendandburns
Copy link
Contributor Author

@kondapally1989 updated, thanks for the patience!

@karthikkondapally
Copy link
Contributor

@brendandburns format issue in compile time.

@brendandburns
Copy link
Contributor Author

@kondapally1989 sorry for the noise, I swear I fixed the formatting, but apparently not. Please take another look.

@karthikkondapally
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 17, 2018
@k8s-ci-robot k8s-ci-robot merged commit a13defe into kubernetes-client:master Oct 17, 2018
@rohanKanojia rohanKanojia mentioned this pull request Mar 25, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbohlool mbohlool Awaiting requested review from mbohlool

1 more reviewer

@karthikkondapally karthikkondapally karthikkondapally left review comments

Reviewers whose approvals may not affect merge requirements
Assignees

@karthikkondapally karthikkondapally

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@brendandburns @k8s-ci-robot @karthikkondapally