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

Dropbox 8.x support #402

Closed
wants to merge 7 commits into from
Closed

Conversation

danni
Copy link

@danni danni commented Oct 5, 2017

Fixes support for recent versions of the Dropbox SDK.

The API was radically different to the previous SDK, but the mocks used in the unit tests made it so the unit tests appeared to continue passing. This updates the backend to use the return types in the new SDK.

Fixes #396 and #397.

@codecov-io
Copy link

codecov-io commented Oct 5, 2017

Codecov Report

Merging #402 into master will decrease coverage by 0.35%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #402      +/-   ##
==========================================
- Coverage    76.1%   75.74%   -0.36%     
==========================================
  Files          11       11              
  Lines        1578     1567      -11     
==========================================
- Hits         1201     1187      -14     
- Misses        377      380       +3
Impacted Files Coverage Δ
storages/backends/dropbox.py 92.39% <88.88%> (-5.42%) ⬇️
storages/backends/s3boto.py 87.41% <0%> (-0.05%) ⬇️
storages/backends/s3boto3.py 86.54% <0%> (-0.05%) ⬇️
storages/backends/gcloud.py 95.7% <0%> (+0.9%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea5649f...ae53ed4. Read the comment docs.

@patroqueeet
Copy link

@jschneier when will this be merged and released?

@zuck
Copy link
Contributor

zuck commented Apr 12, 2018

@jschneier Please, merge this PR. I currently have 6 apps in production blocked by this issue...

@4levels
Copy link

4levels commented Oct 16, 2018

Same here, please merge this as Dropbox deprecated the old API for Business accounts

@sww314
Copy link
Contributor

sww314 commented Oct 16, 2018

@danni Can you resolve conflicts?

@danni
Copy link
Author

danni commented Oct 16, 2018

I don't have a project running on this backend any more. I won't be able to test the result. Is someone else able to merge it?

@sww314
Copy link
Contributor

sww314 commented Oct 17, 2018

@4levels or @zuck can your verify this fix? None of the maintainers are currently using the Dropbox integration. If the conflicts are fixed and someone will integration test, we can get it merged.

@4levels
Copy link

4levels commented Oct 17, 2018

Hi @sww314,

I'd love to but I'm in no position to do so since my Python skills are very limited.
I also don't understand the merge conflicts listed here, all changes in this PR seem very straightforward.

So far I've managed to get around this by creating a new class as extension, called DropBoxTeamStorage - I needed this anyway since the current DropBoxStorage class cannot handle the extra headers required for Dropbox Team accounts. I then override the listdir method in the new class, making use of the newer Dropbox API response types so I'm not blocked.

PS. I'm curerntly creating a new PR to implement the DropBoxTeam solution I'm using.

Kind regards,

@matburnham
Copy link

I can confirm that this seems to work insofar as I can do a listdir which was failing before (#567). I'm not particularly au-fait with git, but have done as much as I can to figure out the merge conflicts.

I could possibly put together another commit to resolve them but I'm not sure I'll then be able to push it anywhere.

  • requirements-tests.txt appears to be new so I don't understand why it is conflicting?
  • test/test_dropbox.py has mostly minor changes - only two appear to be significant but I don't know how to properly run this test and/or interpret the results
  • tox.ini appears to be irrelevant whitespace changes; the only thing significant is pinning dropbox and some other libraries to a particular version; they weren't there before, they can probably be ignored.

If someone can point me to somewhere I can understand about how the tests are supposed to work I can try and figure that out.

@jschneier
Copy link
Owner

Ended up merging #724. Thanks.

@jschneier jschneier closed this Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

using Dropbox 8.1 MetaData is not subscriptable
8 participants