[llvmlab] Add timeout and retries for fetching builds.

[vsapsai]

Summary

In case of network problems, time out after 5 seconds instead of relying on OS
to close the connection which can take more than 20 minutes. Timeout value is
measured between successfully transmitted packets, not for entire transaction.
So we don't require big files to be downloaded in 5 seconds but we require not
to get stuck for more than 5 seconds.

[JDevlieghere]

llvmbisect/llvmlab/gcs.py

15 ↗

(On Diff #116681)

Maybe we should call this HttpSession to emphasize that we will need a new instance each time?

[vsapsai]

llvmbisect/llvmlab/gcs.py

15 ↗

(On Diff #116681)

In this code it doesn't really matter if you create a new instance or reuse the old one. In most places I decided to create a new instance because it is simpler.

I've chosen name HttpClient because client.get(...) seems more natural compared to session.get(...). I have to acknowledge my preferences are influenced by Java (e.g. org.apache.http.impl.client.CloseableHttpClient, okhttp3.OkHttpClient). Though looks like in Python "client" is also popular according to renaming httplib to http.client.

[JDevlieghere]

llvmbisect/llvmlab/gcs.py

15 ↗

(On Diff #116681)

Fair enough, if it doesn't matter I'm totally fine with naming it client!

53 ↗

(On Diff #116681)

Any argument against moving this up and having all the functions reusing this single instance?

[vsapsai]

llvmbisect/llvmlab/gcs.py

53 ↗

(On Diff #116681)

No argument against it. I'll move and test.

HttpClient was intended to be stateful so I tried to avoid sharing. But based on current implementation we can safely share it and if necessary, it is possible to have a separate client.

[cmatthews]

Looks good to me. Please address JDevlieghere's concerns and commit.

[vsapsai]

• Address JDevlieghere feedback: use shared HttpClient in all methods.

zorg/trunk/llvmbisect/llvmlab/gcs.py

 """Integration with Google Cloud Storage.
 
 """
 import os
 import requests
+import urllib3
 
 # Root URL to use for our queries.
 GCS = "https://www.googleapis.com/storage/v1/"
 
 DEFAULT_BUCKET = "llvm-build-artifacts"
 
 BUCKET = os.getenv("BUCKET", DEFAULT_BUCKET)
 
+class HttpClient(object):
+    def __init__(self):
+        self.session = requests.Session()
+        # Retry after 0s, 0.2s, 0.4s.
+        retry = urllib3.util.retry.Retry(total=3, backoff_factor=0.1)
+        adapter = requests.adapters.HTTPAdapter(max_retries=retry)
+        self.session.mount('https://', adapter)
+
+    def get(self, url, **kwargs):
+        if "timeout" not in kwargs:
+            kwargs["timeout"] = 5  # seconds
+        return self.session.get(url, **kwargs)
+
+HTTP_CLIENT = HttpClient()
+
 
 def fetch_builders():
     """Each build kind is stored as a folder in the GCS bucket.
     List all the folders in the bucket, which is our list of possible
     compilers.
     """
     params = {'delimiter': "/", 'fields': "prefixes,nextPageToken"}
-    r = requests.get(GCS + "b/" + BUCKET + "/o", params=params)
+    r = HTTP_CLIENT.get(GCS + "b/" + BUCKET + "/o", params=params)
     r.raise_for_status()
     reply_data = r.json()
     assert "nextPageToken" not in reply_data.keys(), "Too many builders!"
     folders = reply_data['prefixes']
     no_slashes = [x.replace("/", "") for x in folders]
     return no_slashes
 
 
 def fetch_builds(project):
     """Given a builder name, get the list of all the files stored for that
     builder.
     """
     assert project is not None
     all_data = {'items':[]}
     params = {'delimiter': "/",
              "fields": "nextPageToken,kind,items(name, mediaLink)",
              'prefix': project + "/"}
-    r = requests.get(GCS + "b/" + BUCKET + "/o", params=params)
+    r = HTTP_CLIENT.get(GCS + "b/" + BUCKET + "/o", params=params)
     r.raise_for_status()
     reply_data = r.json()
     all_data['items'].extend(reply_data['items'])
     while reply_data.get('nextPageToken'):
         params['pageToken'] = reply_data['nextPageToken']
-        r = requests.get(GCS + "b/" + BUCKET + "/o", params=params)
+        r = HTTP_CLIENT.get(GCS + "b/" + BUCKET + "/o", params=params)
         r.raise_for_status()
         reply_data = r.json()
         all_data['items'].extend(reply_data['items'])
     return all_data
 
 #  Dunno what this could be moved up to?
 CHUNK_SIZE = 5124288
 
 
 def get_compiler(url, filename):
     """Get the compiler at the url, and save to filename."""
-    r = requests.get(url)
+    r = HTTP_CLIENT.get(url)
     r.raise_for_status()
     with open(filename, 'wb') as fd:
         for chunk in r.iter_content(CHUNK_SIZE):
             fd.write(chunk)
     return filename