search

shotgunsoftware / python-api

A Python-based library for accessing Flow Production Tracking API.
https://developer.shotgridsoftware.com/python-api
Other
306 stars 198 forks source link

Add timeout support on upload operations #226

Open greenwolffr opened 4 years ago

greenwolffr commented 4 years ago

Adds the support for timeout on upload request.

According to this old post: https://groups.google.com/a/shotgunsoftware.com/d/msg/shotgun-dev/P-YXTWs9mp0/tzXtkIALt1sJ

It was not added to shotgun API due to a backward compatibility necessity with python 2.4. Since python 2.6 is now a requirement for the API to work it does not create an issue anymore and can be added.

It has become a priority due to the COVID situation, Shotgun creates a lot of timeouts when uploading thumbnails and playblasts.

greenwolffr commented 4 years ago

I am waiting for some test, but I will also add a SSLError except to raise a ShotgunError instead of a Fault.

jfboismenu commented 4 years ago

Hi @greenwolffr , what kinds of timeouts are you seeing exactly? It is when uploading large or small files? If there a specific time one day this is happening?

greenwolffr commented 4 years ago

Hi @jfboismenu , we are experiencing timeouts when uploading medium playblast video files size (~3MB).

This happens every day like 10-20 times a day according to our wranglers, mostly during business time when there is a lot of traffics. We already confirmed that it's due to an HTTP timeout issue, but since upload does not have time out configured, the request never ends and scripts hangs forever.

This patch fixed the issue on our side, with this modification we can now detect timeouts and retry the request.

jfboismenu commented 4 years ago

Hi! Alright, I'll discuss this with the team, this sounds totally fair!

jfboismenu commented 4 years ago

Hi again, we're quite busy at the moment and we want to properly evaluate the impact and QA your fix under different conditions. We're going to look into how widespread this issue is for our clients at the moment, which will raise or lower the priority for us to merge this.

If I'm not wrong everything is under control on your end now that you have this fix, correct?

We'll keep you posted.

greenwolffr commented 4 years ago

Hi, thanks for considering the request.

Yes for now things seems to be in control on our side. The only pitfall we had for now, is that having such a patch make difficult for us to update safely the API.