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

Upgrade from Octokit 1x to 2x to resolve issues with oauth2 #649

Merged
merged 1 commit into from Feb 10, 2014

Conversation

reconbot
Copy link
Contributor

@reconbot reconbot commented Feb 9, 2014

I was having some issues with github and oauth2 when users had 2 factor auth enabled. I wasn't able to track down the exact cause of the errors unfortunately, however I was able to confirm

  • Things work as expected with the new octokit
  • that none of the changes to octokit between the older 1.25 and the 2.7.1 affect our use of it

To reproduce the error I was experiencing simply pop a rails console

user_token = "xxxxx" # taken from your user model
Octokit::Client.new(access_token: user_token).organizations
# Octokit::NotFound: GET https://api.github.com/user/orgs: 404: Not Found

I have a hunch that it has to do with 2 factor auth but I do not yet have facts to back that up.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling fbec305826760f93a6daf9ae3070514bdb3a9eca on wizarddevelopment:upgrade-octokit into 4f4dddb on errbit:master.

@arthurnn
Copy link
Member

👍 for the update..
But tests are not passing.

One breaking change was how to get the urls of objects from the github objects. The only place we used this has been updated.
@reconbot
Copy link
Contributor Author

I just pushed a fix. I spoke too soon about it doing everything we needed and was relying on travis to confirm. I was having issues with my setup to get the tests to run locally, I fixed those, got the tests working locally. Travis is running now.

One thing did change (and was breaking) about how urls are presented in octokit. It's now part of a rels hash and you have to pull it out of there.

@shingara
Copy link
Member

thanks

shingara added a commit that referenced this pull request Feb 10, 2014
Upgrade from Octokit 1x to 2x to resolve issues with oauth2
@shingara shingara merged commit 1c6a130 into errbit:master Feb 10, 2014
@shingara shingara added this to the 0.3.0 milestone Feb 10, 2014
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

4 participants