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

Allow passing custom client arguments to S3 client #146

Merged
merged 2 commits into from
May 17, 2022

Conversation

shadyvb
Copy link
Contributor

@shadyvb shadyvb commented May 17, 2022

Allows modifying S3 client options via environment variables, so implementations can use options like s3BucketEndpoint to customize the client.

ref https://github.com/humanmade/product-dev/issues/1033 and humanmade/altis-local-server#341

Copy link
Contributor

@roborourke roborourke left a comment

Choose a reason for hiding this comment

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

Looks good, though some client args are nested objects iirc. Do we need more than just whether to do SSL verification and use path style? This does give us some flexibility but might not work for everything.

@shadyvb
Copy link
Contributor Author

shadyvb commented May 17, 2022

This isn't using path style rather bucket endpoint, so it doesn't play with the URL at all. Traefik then adds the bucket endpoint as a path prefix. Quite the workaround! but fits all our cases.

I wouldn't know what else we'd need, but it made sense to allow arbitrary options for the future. Why not was my philosophy for this change.

@roborourke
Copy link
Contributor

Minor version bump seems appropriate here

@shadyvb shadyvb merged commit 428be9d into master May 17, 2022
@shadyvb shadyvb deleted the custom-s3-client-args branch May 17, 2022 14:12
@shadyvb
Copy link
Contributor Author

shadyvb commented May 17, 2022

@roborourke
Copy link
Contributor

Nice, docker image should be available by now then

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

Successfully merging this pull request may close these issues.

2 participants