canonical / s3-integrator

An integrator charm for handling s3 credentials
https://charmhub.io/s3-integrator
Apache License 2.0
1 stars 1 forks source link

Avoid overwriting bucket name if set by config #9

Closed shayancanonical closed 1 year ago

shayancanonical commented 1 year ago

Resolves issue 5

Issue

Solution

Release Notes

Avoid overwriting bucket name if set by config

deusebio commented 1 year ago

Looks very good!

Earlier today, I just wanted to make sure that this change was compliant with the s3 interface here

Indeed, the requirer's behavours requirements reads:

Is expected to tolerate that the Provider may ignore the bucket field in some cases (e.g. S3Proxy or S3 Integrator) and instead use the bucket name received.

Which is what we are doing here. So green light to be merged!

However, while doing this, I realized that the name of the interface we are using is not "s3-credentails" (as in the metadata) but it should be "s3". I suppose we should really change this. We could either change it in this PR (if it is really one liner) or open a dedicated issue.

Up to you @shayancanonical ! I approve the PR, but if this is not addressed in the PR, just please open an issue to be addressed on a dedicated PR.

shayancanonical commented 1 year ago

@deusebio @MiaAltieri as suggested, i renamed the interface from s3-credentials to s3 in 3ec051e. please update the consumer charms (mongodb) to use this new interface name once this PR is merged and this charm is publsihed