search

nmrshll / google-photos-api-client-go

Google photos api client in go
35 stars 12 forks source link

support retry on `/v1/upload` as well #3

Closed tonymet closed 4 years ago

tonymet commented 5 years ago

Fixes issue with nmrshll/gphotos-uploader-cli/pull/45 -- 429s occur

I've added retry support to both v1/upload and batchCreate by refactoring retry logic into a retry() function.

retry() retries fn() depending on its return retryStatus --- will retry gain after retryStatus.retryAfter or exponential backoff if unset stopStatus -- will quit retry with error nil -- will quit with success

tonymet commented 5 years ago

@nmrshll hey there i thought we could chat on skype -- there are a few outstanding PRs and i'd like to get your feedback and coordinate merging them.

tonymet commented 5 years ago

skype = tonymetzynga

nmrshll commented 5 years ago

Sure ! Can we do that tomorrow evening or this weekend ? Maybe if we do this this weekend I'll have a bit of time to catch up with the project and PRs

Also what time zone are you in ? I'm in France at the moment

On Wed, Mar 6, 2019, 21:00 Tony Metzidis notifications@github.com wrote:

skype = tonymetzynga

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nmrshll/google-photos-api-client-go/pull/3#issuecomment-470254531, or mute the thread https://github.com/notifications/unsubscribe-auth/AGw1ND9XJyJFIINzkhYupFcg8clIXI8Xks5vUB5wgaJpZM4bhpGu .

tonymet commented 5 years ago

@nmrshll I'm in pacific time (GMT-8 currently, GMT-7 starting sunday). We can chat any time feel free to ping me.

Here is a list of the PRs and the order that they could be merged for you to review. I would recommend reviewing Batch 1 first, and we can merge . It's less risky and easier to merge . Then let's review Batch 2 and I can rebase if needed.

Batch 1 -- support running linux on a VM

These two together allow you to run gphotos-uploader-cli on a VM without a GUI (e.g ubuntu / debian). they allow overriding the redirectURL so you can direct the browser to a remote VM as needed or use ngrok The side effects are minimial and I designed both to be backward compatible with the current configuration and CLI options (a) nmrshll/gphotos-uploader-cli/pull/41 (b) nmrshll/oauth2-noserver/pull/3

Batch 2 -- Speeding uploads with concurrency.

pull#45 adds a semaphore to support 5 concurrent uploads, and pull#3 addresses the HTTP/429s on v1/upload due to the increased upload rate

Overall medium-level risk, but more of a code change compared to Batch 1. (a) nmrshll/gphotos-uploader-cli/pull/45 (b) nmrshll/google-photos-api-client-go/pull/3

tonymet commented 5 years ago

@nmrshll wanted to check in on this and see if we can work together to get the branches merged. we've had 2-3 tests good so far on the other branches and I think there are a few people that these could help out. Happy to chat and make sure the integration goes along with the overall design.