Remove 50 item limit from uploadMultiple

mcpengelly commented 5 years ago

Motivation

The :batchCreate api which is used by uploadMultiple() only supports a maximum of 50 media items per request as per the warning on this page and the following error response message when using the api:

Error: StatusCodeError: 400 - {"error":{"code":400,"message":"Request must have less than 50 items.","status":"INVALID_ARGUMENT"}}

To work around this I capped mediaItems at 50 items per :batchCreate call and iteratively call :batchCreate until the list of images is exhausted. This allows users to input any amount of images for upload.

Manual Testing

I tested this with a sample of 2500 images works as intended. Although the timeout im forced to use right now slows the uploads down significantly, it does a maximum of 5 uploads/second right now due to the timeout alone. I was able to upload 2500 images to a google album with these changes in ~750 seconds.

caveats

google is not happy with ~400 requests to upload per minute. However, this is required for each image to upload to get an upload token that gets used with :batchCreate. So far i worked around it with a timeout and i know thats not ideal but that opens the field for discussion i think... here is the error message it is not listed on the limits and quotas page
```
(node:75041) UnhandledPromiseRejectionWarning: Error: Error: StatusCodeError: 429 - "{\n  \"code\": 8,\n  \"message\": \"Quota exceeded for quota metric \\u0027Upload requests\\u0027 and limit \\u0027Upload requests per minute per user\\u0027 of service \\u0027photoslibrary.googleapis.com\\u0027 for consumer \\u0027project_number:5775087294\\u0027.\"\n}"
```
possible solutions:
we can leave uploadMultiple as only capable of handling 50 images/files at a time, and leave it to the user who consumes the library to batch calls to this function, the user would then have to handle the logistics themselves (batching, t avoiding overloading the google api) or increase the limit of their requests per minute quota (if thats even possible for requests per minute). or
we can handle it in some way either with a timeout like i've done or through some other means I've not yet thought of. Im not sure what the best approach to this is yet but it works right now. I'm looking for input in case you've got any ideas.

more...

this also uses lodash chunk does this bother you @roopakv?

roopakv commented 4 years ago

Thank you for the PR. I will take a look tomorrow evening or saturday morning worst case :) sorry for the delays.

mcpengelly commented 4 years ago

No worries at all @roopakv I expect you're busy! I just like to keep PRs as an ongoing process so you (or anyone) can leave feedback whenever they have a chance. If it seems like i'm impatient thats not the case!

roopakv commented 4 years ago

thank you for the PR!

thoughts

love the auto chunking and spreading over time. in the future we could make that number customizeable instead of 10000 so that people can tune as needed
lets use lodash.chunk

LGTM otherwise

mcpengelly commented 4 years ago

love the auto chunking and spreading over time. in the future we could make that number customizeable instead of 10000 so that people can tune as needed

I like this. A default value that is the bare minimum would work and open up more flexibility if the users the google app is somehow able to get more then 500-600 requests per minute (ie if theyre a google partner)

lets use lodash.chunk

sounds good

mcpengelly commented 4 years ago

should be good now, addressed feedback and gave it another test with 150 images and it seemed to go fine! I should really have provided a test case or something though

roopakv / google-photos