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

openstack::core: Don't mutate params between request retries #508

Merged
merged 1 commit into from
Feb 12, 2020

Conversation

avgerin0s
Copy link
Contributor

This PR fixes #507 and potentially #501 and #502

Since f72d2fc (first released in v0.1.20) fog-openstack mutates the
params passed on Fog::Openstack::Core#request by calling
#delete(:headers)1 on the params hash to access it.

In case of a re-authentication the request is re-issued2 but the
headers hash is no longer present. As a result all headers passed by the
user are lost and the request gets just the default headers3.

This commit fixes the aforementioned behavior by not mutating the params
hash in order to access its :headers part.

Since f72d2fc (first released in v0.1.20) fog-openstack mutates the
params passed on `Fog::Openstack::Core#request` by calling
`#delete(:headers)`[1] on the params hash to access it.

In case of a re-authentication the request is re-issued[2] but the
headers hash is no longer present. As a result all headers passed by the
user are lost and the request gets just the default headers[3].

This commit fixes the aforementioned behavior by not mutating the params
hash in order to access its `:headers` part.

[1]: https://github.com/fog/fog-openstack/blob/v1.0.10/lib/fog/openstack/core.rb#L57
[2]: https://github.com/fog/fog-openstack/blob/v1.0.10/lib/fog/openstack/core.rb#L61
[3]: https://github.com/fog/fog-openstack/blob/v1.0.10/lib/fog/openstack/core.rb#L128
@avgerin0s
Copy link
Contributor Author

Forgot to mention the maintainers as the README suggests.

cc @dhague, @Ladas, @seanhandley, @mdarby and @jjasghar.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Nov 27, 2019

Build succeeded.

@avgerin0s
Copy link
Contributor Author

Any news on this one?

@aufi
Copy link
Member

aufi commented Feb 12, 2020

Looks good to me 👍

@aufi aufi added the bug label Feb 12, 2020
@aufi aufi merged commit 997dc67 into fog:master Feb 12, 2020
@avgerin0s
Copy link
Contributor Author

@aufi Thanks for merging this. Are there any plans for a patch version release any time soon?

@aufi
Copy link
Member

aufi commented Feb 13, 2020

I don't have permissions to create a new gem release, but there few new things, so it would make sense to me 👍

@Ladas Would it be possible create a new release?

@avgerin0s
Copy link
Contributor Author

Hope you are well and healthy. Any news on the upcoming release?

@Ladas
Copy link
Contributor

Ladas commented May 13, 2020

@ares are you planning new release soon?

@ares
Copy link
Collaborator

ares commented May 19, 2020

let me forward this question to @domitea, is there already something you want to push? I can do the release if not

@domitea
Copy link

domitea commented May 19, 2020

@ares Nothing right now.

@ares
Copy link
Collaborator

ares commented May 19, 2020

ok, 1.0.11 released, I hope I didn't forget anything

@nhh
Copy link

nhh commented May 28, 2020

We are using the 1.011 gem and the issues #501 and #502 are not fixed. Everything newly created is "application/json" no matter what content_type= we are setting.

Any ideas?

@avgerin0s
Copy link
Contributor Author

@nhh Do you have a sample request/snippet of code that reproduces it?

@nhh
Copy link

nhh commented May 28, 2020

@avgerin0s I'm using it with carrierwave, so there is no direct fog-openstack code. It may be a problem with carrierwave too, but #501 and #502 sounds pretty much like my problem.

CarrierWave::Storage::Fog::File

Calling the CarrierWave::Storage::Fog::File#content_type= method has no effect. It seems that the Fog::Storage::OpenStack::File has no method like #content_type=. Or #metadata=. What is the appropriate method to set the content_type?

@nhh
Copy link

nhh commented May 28, 2020

@avgerin0s I've opened a more detailed issue on carrierwave. It may be useful for you?

carrierwaveuploader/carrierwave#2484

I dont know what gem causes this issue, so sorry for the confusion.

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.

Fog::Openstack::Core#request discards headers upon retries
6 participants