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

S3: Add access-style property #2392

Merged
merged 1 commit into from
Sep 7, 2020
Merged

Conversation

laszlovandenhoek
Copy link
Contributor

@laszlovandenhoek laszlovandenhoek commented Sep 3, 2020

References #2171 . In the pull request #2193 (comment) it was noted that it would be better to have a separate access-style property instead of the current tripartite path-style-access one. This PR attempts to improve that situation.

With this change, access-style is the preferred way of setting the property, using either path or virtual.

If the old, deprecated path-style-access is used, that takes precedence, but a warning is logged to instruct the developer to change to the new property.

If the effective access style is path, and endpoint-url is not set (i.e. the S3 provider is AWS), one warning is issued. This is not configurable, but it shouldn't have to be, because using path-style access on AWS S3 is likely going to cause problems sooner or later.

@laszlovandenhoek laszlovandenhoek changed the title Add access-style property S3: Add access-style property Sep 3, 2020
@laszlovandenhoek laszlovandenhoek marked this pull request as draft September 3, 2020 14:34
@laszlovandenhoek laszlovandenhoek marked this pull request as ready for review September 3, 2020 15:36
@laszlovandenhoek laszlovandenhoek force-pushed the access-style branch 3 times, most recently from 639acb6 to e402d13 Compare September 3, 2020 20:13
@laszlovandenhoek laszlovandenhoek marked this pull request as draft September 4, 2020 07:00
Copy link
Member

@ennru ennru left a comment

Choose a reason for hiding this comment

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

LGTM.

@ennru ennru added this to the 2.0.2 milestone Sep 7, 2020
@ennru ennru merged commit f3303a9 into akka:master Sep 7, 2020
@ennru
Copy link
Member

ennru commented Sep 7, 2020

Thank you for suggesting and providing a clean solution to this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants