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

Implemented upload session APIs (multi-part uploads) #18

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

Conversation

jpillora
Copy link

@jpillora jpillora commented Jul 16, 2016

Changes

  • implemented upload session APIs (multi-part uploads)
  • implemented auto-handled upload session method UploadSession() (see go-dropy)
  • renamed internal download() to content() ("Content-upload endpoints" and "Content-download endpoints")
  • added internal decodeContent() helper
  • fixed PermanentlyDelete endpoint path (not sure if it was incorrect on purpose?)

Notes

  1. Methods should probably use value parameters, as pointer parameters allow property changes after the call has been made. Also, it's good practise to use values where possible. However, I thought it was more important to follow the current method layout.
  2. As an alternate API to reduce the size of files.go, the raw upload session methods (start/append/finish) could be moved to files_upload_session.go and then would become Files.UploadSession.Start(), .Append(), .Finish() - though we'd have to rename UploadSession() to something else.
  3. As an additional feature, we could add a PerformRetries bool so if any chunk failed, it would be retransmitted. This would require a ChunkSize buffer though to hold data in case of failure and maybe the Dropbox API doesn't fail that often so it might not be worth it...

@jpillora
Copy link
Author

# cd .; git clone https://github.com/davecgh/go-spew /home/runner/workspace/src/github.com/davecgh/go-spew
Cloning into '/home/runner/workspace/src/github.com/davecgh/go-spew'...
error: RPC failed; curl 56 GnuTLS recv error (-9): A TLS packet with unexpected length was received.

Strange failure, maybe manually retry?

@jpillora
Copy link
Author

Ah yes, just remembered #2. So maybe the higher-level UploadSession() should go in go-dropy?

@tj
Copy link
Owner

tj commented Jul 17, 2016

niceee, yea as much as possible I think the fancier stuff should be in go-dropy

@jpillora
Copy link
Author

Striped out high-level UploadSession() method, moved to go-dropy, see jpillora/go-dropy@5eee26d - Will PR it over there once this lands.

@jpillora
Copy link
Author

Adding new test with manually managed session upload...

@jpillora
Copy link
Author

Semaphore seems to be a bit buggy, https://semaphoreci.com/tj/go-dropbox/branches/pull-request-18/builds/4 also failed strangely

@jpillora
Copy link
Author

I'll sync with upstream again once #17 tests are fixed

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