Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

User can choose to use get http method #276

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
5 participants

xinuc commented Aug 29, 2012

There are a lot of use cases where using get is acceptable.
Using get makes it easy for us to use http caching like varnish etc

Review on Reviewable

This pull request fails (merged d332677 into 8f33061).

This pull request fails (merged c10cf2e into 8f33061).

Member

alindeman commented Oct 22, 2012

The build is failing. Any idea why?

xinuc commented Oct 23, 2012

travis ci keeps telling

RuntimeError:
Factory can only create an instance once!

Pretty sure this is not caused by my commits.
The specs pass on my local machine.

Member

alindeman commented Nov 20, 2012

I think having the configuration default to post is better. And instead of a conditional, we just __send__ the verb specified. Does this make sense?

xinuc commented Nov 21, 2012

Yes, the default is still using post

I think we can't use __send__ cause rsolr have a slightly different syntax for them.
post uses data option, while get uses params.

To change the http method, user have to add

Sunspot.config.solr.http_method = :get

somewhere.

I'm not sure this is the best api though.

Any idea?

Member

alindeman commented Nov 22, 2012

Can we have http_method default to get? See https://github.com/sunspot/sunspot/blob/master/sunspot/lib/sunspot/configuration.rb for examples of defaults.

xinuc commented Nov 26, 2012

Of course.
I think using get as default method is a sensible configuration.

xinuc commented Nov 26, 2012

finally, good to merge 👍

Member

rywall commented Sep 10, 2013

This pull request has conflicts. Can you rebase it?

Member

joneslee85 commented Mar 3, 2015

@xinuc I am going to close it since it is so out of date. Feel free to reopen if you are active again.

@joneslee85 joneslee85 closed this Mar 3, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment