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

proposal: app engine support #113

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

jtolio
Copy link

@jtolio jtolio commented Dec 29, 2016

App Engine unfortunately can't make outbound requests without using
the appengine urlfetch library and a request-specific context.

I don't see a way around this other than to change the signature of
the Provider and Session interfaces.

This really has a huge amount of downsides obviously, but before I
go and fix stuff for providers besides Facebook, G+, and Twitter
(Twitter support requires mrjones/oauth#60
to get merged), I figured I'd open a pull request and discuss the
tradeoffs.

Unfortunately I think we need to do something like this to support
App Engine. Perhaps this is an opportunity to version goth using
gopkg.in or something?

@jtolio
Copy link
Author

jtolio commented Dec 29, 2016

Yeah, there's no way the builder passes. Let me know if I should spend time fixing all the other providers.

jtolio added a commit to go-webhelp/whgoth that referenced this pull request Dec 29, 2016
@jtolio
Copy link
Author

jtolio commented Dec 29, 2016

Of course, the other option is to add methods to all the interfaces. This makes provider implementation harder, but maybe that's worth the backwards compatibility.

@markbates
Copy link
Owner

Backwards compatibility is really important. A lot of people have Goth in production. I don't want to break that. I, honestly, don't really understand the issue, so it's hard for me to help with guidance on a good solution.

@tylerstillwater
Copy link
Collaborator

Perhaps some kind of global Requester interface/object that defaults to what is in there now, but can be overridden by the user-programmer?

@jtolio
Copy link
Author

jtolio commented Dec 29, 2016

@tylerb: that's included in the above patchset. the problem is that the overrideable global requester requires knowing request specific context information. on app engine, you can't make an outbound request without tying it to the request that kicked off your app engine code

@jtolio
Copy link
Author

jtolio commented Dec 29, 2016

I'll refactor to not break backwards compat.

App Engine unfortunately can't make outbound requests without using
the appengine urlfetch library and a request-specific context.

I don't see a way around this other than to change the signature of
the Provider and Session interfaces.

This really has a huge amount of downsides obviously, but before I
go and fix stuff for providers besides Facebook, G+, and Twitter
(Twitter support requires mrjones/oauth#60
to get merged), I figured I'd open a pull request and discuss the
tradeoffs.

Unfortunately I think we need to do something like this to support
App Engine. Perhaps this is an opportunity to version goth using
gopkg.in or something?
@jtolio
Copy link
Author

jtolio commented Dec 30, 2016

Okay, new commit, still only touched Facebook, G+, and Twitter, but should be backwards compatible with downstream libraries.

@jtolio
Copy link
Author

jtolio commented Dec 30, 2016

If there's a chance this approach might be accepted, I'll get to work fixing all the other providers.

jtolio added a commit to go-webhelp/whgoth that referenced this pull request Dec 30, 2016
@asawilliams
Copy link

Is there a work around for getting goth to work with GAE or should we wait until this gets merged in?

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