Skip to content

Conversation

@abeyad
Copy link

@abeyad abeyad commented Oct 11, 2016

In 2.x, the S3 repository accepted a / (forward slash) to start
the repositories.s3.base_path, and it used a different string splitting
method that removed the forward slash from the base path, so there
were no issues.

In 5.x, we removed this custom string splitting method in favor of
the JDK's string splitting method, which preserved the leading /.
The AWS SDK does not like the leading / in the key path after the
bucket name, and so it could not find any objects in the S3 repository.

This commit fixes the issue by removing the leading / if it exists
and adding a deprecation notice that leading / will not be supported
in the future in S3 repository's base_path.

the repositories.s3.base_path, and it used a different string splitting
method that removed the forward slash from the base path, so there
were no issues.

In 5.x, we removed this custom string splitting method in favor of
the JDK's string splitting method, which preserved the leading `/`.
The AWS SDK does not like the leading `/` in the key path after the
bucket name, and so it could not find any objects in the S3 repository.

This commit fixes the issue by removing the leading `/` if it exists
and adding a deprecation notice that leading `/` will not be supported
in the future in S3 repository's base_path.
@abeyad
Copy link
Author

abeyad commented Oct 11, 2016

I created #20862 to ensure we have tests against real S3 repositories, which would uncover issues like this going forward.

@rjernst
Copy link
Member

rjernst commented Oct 11, 2016

LGTM

@abeyad
Copy link
Author

abeyad commented Oct 11, 2016

thanks @rjernst

@abeyad abeyad merged commit bbf6e6d into elastic:master Oct 11, 2016
@abeyad abeyad deleted the fix/s3_base_path branch October 11, 2016 15:18
abeyad pushed a commit that referenced this pull request Oct 11, 2016
In 2.x, the S3 repository accepted a `/` (forward slash) to start
the repositories.s3.base_path, and it used a different string splitting
method that removed the forward slash from the base path, so there
were no issues.

In 5.x, we removed this custom string splitting method in favor of
the JDK's string splitting method, which preserved the leading `/`.
The AWS SDK does not like the leading `/` in the key path after the
bucket name, and so it could not find any objects in the S3 repository.

This commit fixes the issue by removing the leading `/` if it exists
and adding a deprecation notice that leading `/` will not be supported
in the future in S3 repository's base_path.
abeyad pushed a commit that referenced this pull request Oct 11, 2016
In 2.x, the S3 repository accepted a `/` (forward slash) to start
the repositories.s3.base_path, and it used a different string splitting
method that removed the forward slash from the base path, so there
were no issues.

In 5.x, we removed this custom string splitting method in favor of
the JDK's string splitting method, which preserved the leading `/`.
The AWS SDK does not like the leading `/` in the key path after the
bucket name, and so it could not find any objects in the S3 repository.

This commit fixes the issue by removing the leading `/` if it exists
and adding a deprecation notice that leading `/` will not be supported
in the future in S3 repository's base_path.
@abeyad
Copy link
Author

abeyad commented Oct 11, 2016

5.0 commit: f2b3b1a
5.x commit: 2c02a97

@s1monw s1monw removed the review label Oct 11, 2016
@s1monw
Copy link
Contributor

s1monw commented Oct 11, 2016

thanks @abeyad

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs v5.0.0 v6.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants