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

check for empty params #91

Merged
merged 1 commit into from Mar 15, 2014
Merged

check for empty params #91

merged 1 commit into from Mar 15, 2014

Conversation

Asmod4n
Copy link
Contributor

@Asmod4n Asmod4n commented Mar 4, 2014

Am writing a etcd client, the server throws a http 5** error when a questionmark is added without params.

So this is needed.

@Asmod4n
Copy link
Contributor Author

Asmod4n commented Mar 4, 2014

Oh and, would be nice if you could release 0.5.1 then asap ^^

@tarcieri
Copy link
Member

tarcieri commented Mar 4, 2014

Hmm, looks like the build is breaking on 1.8?

@sferik as soon as we get the #close bug licked I really think we should release 0.16. It's been too long since a release IMO.

@sferik
Copy link
Contributor

sferik commented Mar 4, 2014

@tarcieri If you look at the build failures, it’s failing because it can’t install celluloid on Ruby 1.8. This is odd because guard-rspec (which requires guard, which requires listen, which requires celluloid) is excluded from Ruby 1.8 in the Gemfile.

As for the release v0.6 release, we have a v0.6 milestone with 5 open issues. If you want to move any of those issues out of the milestone, we should talk about it. IMHO, these are all blockers that need to be addressed before we can ship the next release.

@jwinter
Copy link
Contributor

jwinter commented Mar 12, 2014

I can't repro this issue, even on 0-5-stable. I've tried:

HTTP.get('http://www.google.com/?').to_s
HTTP.get('http://www.google.com/?', params: {}).to_s                                                             
HTTP.get('http://www.google.com/', params: {}).to_s

@Asmod4n can you provide either a test or some example code that fails?

@ixti
Copy link
Member

ixti commented Mar 12, 2014

@jwinter Issue is specific to etcd, which responds with HTTP 5XX when query has traling ?.

@jwinter
Copy link
Contributor

jwinter commented Mar 12, 2014

Thanks @ixti. It looks like 83a7d15b adds this check. Can this can be closed then?

@Asmod4n
Copy link
Contributor Author

Asmod4n commented Mar 12, 2014

Some etcd API methods 5XX when you add a empty query param, this is a fix for the 0.5 release, so if you '~> 0.5' http you'll get the fix.

Von einem mobilen Gerät gesendet

Am 12.03.2014 um 14:50 schrieb "Aleksey V. Zapparov" notifications@github.com:

@jwinter Issue is specific to etcd, which responds with HTTP 5XX when query has traling ?.


Reply to this email directly or view it on GitHub.

@Asmod4n
Copy link
Contributor Author

Asmod4n commented Mar 12, 2014

@jwinter not for the 0.5 branch.

@tarcieri
Copy link
Member

@Asmod4n need to ship 0.6 soon ;)

@ixti
Copy link
Member

ixti commented Mar 15, 2014

@sferik @tarcieri While we should "ship 0.6 soon", I think it worth merging this PR (it's against 0.5-stable branch) so that those using that branch will get benefit.

@sferik
Copy link
Contributor

sferik commented Mar 15, 2014

@ixti Are you suggesting we merge this into 0-5-stable, ship 0.5.1, and then git cherry-pick this commit into master? Just want to confirm.

@Asmod4n
Copy link
Contributor Author

Asmod4n commented Mar 15, 2014

@sferik this is already resolved in master, was just missing in 0.5

@sferik
Copy link
Contributor

sferik commented Mar 15, 2014

I see.

sferik added a commit that referenced this pull request Mar 15, 2014
@sferik sferik merged commit 8a148d4 into httprb:0-5-stable Mar 15, 2014
@ixti
Copy link
Member

ixti commented Mar 15, 2014

@Asmod4n beat me to 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.

None yet

5 participants