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

Fix downloadDirectory method in TransferManager #1321

Closed
jperez-dx opened this issue Sep 29, 2017 · 4 comments
Closed

Fix downloadDirectory method in TransferManager #1321

jperez-dx opened this issue Sep 29, 2017 · 4 comments

Comments

@jperez-dx
Copy link

The current implementation of the downloadDirectory method creates the whole directory structure based on the key of the objects and not only their relative path to the key prefix.

For example, if the downloadDirectory method is called with the key prefix sample-dir/other-dir/ and /Users/jperez/Desktop/dest-dir/ as the destination dir, then all the S3 files with that key prefix in the given bucket would be placed in /Users/jperez/Desktop/dest-dir/sample-dir/other-dir/, when they should be placed under /Users/jperez/Desktop/dest-dir/. Files should be located in the root of the destination dir.

Maybe this can be fixed by replacing this line...

File f = new File(destinationDirectory, summary.getKey());

...with something like

File f = new File(destinationDirectory, summary.getKey().lstrip(keyPrefix));

I don't know if this is implemented this way on purpose, but it makes much more sense to do it this other way.

@millems
Copy link
Contributor

millems commented Sep 29, 2017

Unfortunately we can't change this behavior because it would break backwards compatibility with customers that currently expect the previous behavior, but I agree that your approach is more intuitive.

We have an opportunity to break backwards compatibility a bit with our next major version of the Java SDK (v2.0). I've linked to this issue in our 2.0 repository to make sure we don't miss this suggestion when we're making the updates.

Thanks for the report!

@kevin860
Copy link

@millems understood if this is more of a v2.0 type change, but if you're interested I ran into this as well today and created a pull request that won't break backwards compatibility.

@alecswan
Copy link

@millems, any chance merging @kevin860 backward-compatible change into this repo?

@zoewangg
Copy link
Contributor

Hi all, just a quick update, this feature has been implemented in the transfer manager in Java SDK v2. Note that it's in Developer Preview. You can check out sample code here https://github.com/aws/aws-sdk-java-v2/tree/master/services-custom/s3-transfer-manager.

As always, we welcome feedback, bug reports and feature requests on the https://github.com/aws/aws-sdk-java-v2 repo!:)

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

No branches or pull requests

5 participants