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

New dropbox-v2 provider which uses Dropbox V2 API #244

Merged
merged 4 commits into from Sep 24, 2017
Merged

Conversation

@klaronix
Copy link
Contributor

klaronix commented Jul 18, 2016

The existing Dropbox provider uses Dropbox API V1 which has been deprecated in April 2016. New API V2 uses "RPC" style calls (i.e. POST) for requesting user account information. So far all providers are requesting account information by GET calls. There is a new provider config option now ("providerMethod") which can be set to 'post' in order to get account information by POST request (defaults to 'get').

https://www.dropbox.com/developers/documentation/http/documentation#users-get_current_account

…precated in April 2016)
@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Jul 26, 2016

This looks good on the first go. Maybe since you clearly pointed out that our existing dropbox implementation is no longer the preferred way, we can change this rather than adding a new dropbox-v2.

I depend on your to let us know if it breaks existing clients to do so, so that I know if this will be a major release or not.

What would make it a breaking release is if the profile response looks any different than before for example or if it supports different parameters than before. I would edge towards saying that yes it breaks existing clients and there is no issue for us to make a major release.

@klaronix

This comment has been minimized.

Copy link
Contributor Author

klaronix commented Jul 26, 2016

Unfortunately Dropbox V2 api breaks clients because of different profile response. So I would go for a '-v2' provider rather than urging bell users to migrate on next update. The old api will be switched off on 06/28/2017; maybe until then we should keep the two providers.

@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Aug 20, 2016

I'll merge this in 8 days but if you could make the following modifications (i'll do them otherwise).

Use Wreck.request and pass the method instead of your current way. As well as default to 'get' internally (along with the current default in the Joi schema).

Also, some documentation on the v2 provider would be appreciated.

@ldesplat ldesplat added the feature label Aug 20, 2016
@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Sep 7, 2016

I have not forgotten about this PR. I've got some work in progress that will incorporate it :)

Arno Klein
@klaronix

This comment has been minimized.

Copy link
Contributor Author

klaronix commented Mar 31, 2017

@ldesplat: Do you think, we get this PR integrated? Because I have another one ;=) It's commit 36bd1f1 from today, which adds support to pass a custom Wreck agent (this allows to authenticate behind a proxy).

@ldesplat

This comment has been minimized.

Copy link
Contributor

ldesplat commented Apr 1, 2017

Can you remove the custom agent from that branch and make another PR. I'll merge this when that's done.

Arno Klein added 2 commits Apr 1, 2017
Arno Klein
This reverts commit 36bd1f1.
@klaronix

This comment has been minimized.

Copy link
Contributor Author

klaronix commented Apr 1, 2017

OK. PR is now "agent free".

@ldesplat ldesplat added this to the 8.8.0 milestone Sep 24, 2017
@ldesplat ldesplat merged commit d188400 into hapijs:master Sep 24, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.