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

storage: add openstack_storage_url option #454

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

elthariel
Copy link

In order to support ACL management in my provider (OVH), I need to have users in a different tenant and I need to use another base URL for my storage calls. In the python client it is implemented using the OS_STORAGE_URL.

This is the equivalent option for fog.

I don't really know how to add tests to this, suggestions are highly welcomed

Signed-off-by: Julien 'Lta' BALLET [email protected]

@@ -237,6 +230,19 @@ def authenticate

true
end

protected

Choose a reason for hiding this comment

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

Layout/EmptyLinesAroundAccessModifier: Keep a blank line before and after protected.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Oct 15, 2018

Build succeeded.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Oct 16, 2018

Build succeeded.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Oct 16, 2018

Build succeeded.

@@ -237,6 +230,20 @@ def authenticate

true
end

protected
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless I missed something I believe private should be enough.

@gildub
Copy link
Collaborator

gildub commented Oct 18, 2018

This looks good. Just a question, see above conversation.
Regarding tests, since you have the module utils you should be able to easily unit test #management_url. For a good explanation and example check out https://chriskottom.com/blog/2015/03/testing-ruby-mixins-in-isolation/

@theopenlab-ci
Copy link

theopenlab-ci bot commented Oct 16, 2019

Build succeeded.

@ares
Copy link
Collaborator

ares commented Feb 4, 2022

@ShamoX seems reasonable to me and should be backwards compatible, thoughts?

@ShamoX
Copy link
Contributor

ShamoX commented Feb 4, 2022

Yes, seems good, but adding a test like @gildub proposed would improve confidence.

@elthariel still need help here to do it (even after @gildub proposition) ?

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.

5 participants