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

Enable to configure "host_base" as "S3 Endpoint" in #784

Merged
merged 1 commit into from Sep 9, 2016

Conversation

mozawa
Copy link
Contributor

@mozawa mozawa commented Sep 2, 2016

Currently, if user wants to use an S3 endpoint rather than Amazon S3 default region endpoint, the following entries need to be updated manually after the first configuration using "--configure".
This change is to enable to configure "host_base" as "S3 endpoint" in the first interactive configuration and update "host_bucket" as well so that user can use this command without any changes after the first configuration.

[.s3cfg]
host_base = s3.amazonaws.com
host_bucket = %(bucket)s.s3.amazonaws.com

@mozawa
Copy link
Contributor Author

mozawa commented Sep 7, 2016

@fviard Is it possible to get this feature into the next official release?

@fviard
Copy link
Contributor

fviard commented Sep 7, 2016

Hi, sorry for the delay, i wanted to comment but lacked the time.
The base idea is good I think, but there is 2 points i think:

  • [Minor] Probably it will be useful to have the default value to use to target amazon in a description. And to suggest not to modify it for using with aws.
  • [Major] I'm annoyed because I think that you can't "auto set" the host_bucket base on the "host_value". Because most of custom s3 servers don't use dns based buckets but path style buckets. But people could also be lost in the opposite case if you just set host_bucket=host_base.
    So, in my opinion, it would probably be better to have 2 configuration questions. One for each. And I think that it is important to give the default value for aws s3 and to indicate the "%(bucket)s" and "%(location)s" vars that can be used.

@mozawa
Copy link
Contributor Author

mozawa commented Sep 8, 2016

@fviard Thank you for the advice. I made some changes based on your suggestion. Please review this again. Thanks!

@fviard
Copy link
Contributor

fviard commented Sep 8, 2016

Thank you for being so reactive.

  • Sorry, but I was not thinking about having the "%(bucket)s" automatically set but more that it should have been in the help doc. I think that automatically set it is bad, as most custom configs don't use it.
  • Also, by default you will see amazon url in the "helper", but in fact it shows the value that is already in .s3cfg or otherwise the default value. So if you save the config, and run configure a second time, it is your custom value that will be the default. That's why I'm thinking that it could be good to have the amazon default value in the help doc string also.
  • Finally, for the website_endpoint that change based on use_https, that could have been a good idea for resolving the case of a few people that might use that. But sadly i don't think that we can do that, because by default aws s3 will not support HTTPS traffic for website_endpoint except if your bucket name is correct and you do a complex setup.
  • Also, I think that it is not a common feature, so maybe it is better to not put website_endpoint in the config to not confuse new comers?

@mozawa
Copy link
Contributor Author

mozawa commented Sep 9, 2016

@fviard Thank you for reviewing this again. Made changes which got simpler. Thanks!

@fviard fviard merged commit 89dccc8 into s3tools:master Sep 9, 2016
@fviard
Copy link
Contributor

fviard commented Sep 9, 2016

That is perfect enough like that, thank you!

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.

None yet

2 participants