Skip to content

Support unauthenticated rate limited requests #2

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

Closed
wants to merge 3 commits into from
Closed

Support unauthenticated rate limited requests #2

wants to merge 3 commits into from

Conversation

sqs
Copy link
Contributor

@sqs sqs commented Jun 3, 2013

This PR adds support for unauthenticated rate limited requests (http://developer.github.com/v3/#unauthenticated-rate-limited-requests), which are subject to the higher rate limit of your OAuth application but are not authenticated with any user's Github credentials.

If you are interested in merging this, let me know what kind of tests would you like to see and I will add them to this PR.

(Credit: I copied cloneRequest from code.google.com/p/goauth2/oauth.)

…quests subject to your application's rate limit (as opposed to the lower rate limit for anonymous requests)
@willnorris
Copy link
Collaborator

whoops, looks like I wasn't watching my own repo, so I didn't get the email notification for your pull request :)

This looks great. Yes, please do add tests... the main thing to test for is that, given a properly configured transport, the client_id and client_secret parameters are being appended to the request as expected. Do something similar to TestDo where you setup an http handler on "/", and inside that handler check that the client_id and client_secret request parameters have the expected values (you can see samples of this kind of checking in places like this).

The setup() method will create a client for you, but for this particular test, you'll of course need to create a new client with an unauthenticated transport.

@sqs
Copy link
Contributor Author

sqs commented Jun 11, 2013

Thanks for the comments. I've fixed the docs and will write some tests soon.

@sqs
Copy link
Contributor Author

sqs commented Jun 15, 2013

OK, I pushed a test that checks that the querystring params are set. I believe this addresses all of your comments. Let me know if it needs anything else. Thanks!

@sqs
Copy link
Contributor Author

sqs commented Jun 15, 2013

Hmm, Travis CI failed on this error:

--- FAIL: TestRepositoriesService_CreateStatus (0.00 seconds)
repos_test.go:385:  Repositories.CreateStatus returned error: POST http://127.0.0.1:59270/repos/o/r/statuses/r: 404 
repos_test.go:390:  Repositories.CreateStatus returned &{ID:0 State: TargetURL: Description: Creator:<nil> CreatedAt:<nil> UpdatedAt:<nil>}, want &{ID:1 State: TargetURL: Description: Creator:<nil> CreatedAt:<nil> UpdatedAt:<nil>}

I'm unable to repro it locally with or without merging, and it seems unrelated.

@willnorris
Copy link
Collaborator

merged in 03fb3ab

@willnorris willnorris closed this Jun 16, 2013
bubg-dev pushed a commit to bubg-dev/go-github that referenced this pull request Jun 16, 2017
Adding omitempty to base_tree in CreateTree API
jmcampanini added a commit to jmcampanini/go-github that referenced this pull request Sep 6, 2018
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.

2 participants