Skip to content

Conversation

@niedfelj
Copy link
Contributor

Make headers persistent to current thread so they can be set on a per-request basis instead of globally.

@tamird
Copy link
Contributor

tamird commented May 24, 2013

What does this solve? Can you provide a failing test?

@niedfelj
Copy link
Contributor Author

Provide a failing test for this? It's pretty clear that the headers variable is not threadsafe as it is implemented in the current active resource code. Not sure where you want a failing test, but you can emulate it by setting different headers on two different resources, and noticing that they will have the headers of that last one set.

guilleiguaran added a commit that referenced this pull request Jul 15, 2013
@guilleiguaran guilleiguaran merged commit 6101794 into rails:master Jul 15, 2013
@ches
Copy link
Contributor

ches commented Jan 22, 2014

This was sort of only half a solution -- say I have this:

class MyServiceResource < ActiveResource::Base
  self.headers['Accept'] = 'application/json, application/vnd.myapp+json; version=1'
end

I use something like this as a base class for other ARes models that talk to the same API. With the solution in this merged change, only the thread where class definition of MyServiceResource is first evaluated (i.e. where it is required) gets a thread-local hash containing my Accept header. In any other thread, the lazy initialization of a thread-local means that MyServiceResource.headers is just an empty hash.

@bvogel
Copy link

bvogel commented Jun 21, 2016

Hi @niedfelj, could you explain why due to this change I'm seeing #219 and what I would do to work around this issue? I'm using (almost) official sanctioned code but this commit will break it.

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