Skip to content

Commit

Permalink
Bug 1633158 - raise the retries r=Bebe
Browse files Browse the repository at this point in the history
This change tries to address the intermittents  we get
when getting indexed artifacts by raising the retries
and how long we pause between retries.

Differential Revision: https://phabricator.services.mozilla.com/D73022
  • Loading branch information
billrest committed Apr 30, 2020
1 parent aa9c97b commit e8eef80
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 18 deletions.
6 changes: 3 additions & 3 deletions testing/condprofile/condprof/android.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ def prepare(self, profile, logfile):
# See android_emulator_pgo.py run_tests for more
# details on why test_root must be /sdcard/tests
# for android pgo due to Android 4.3.
self.device = ADBDevice(verbose=self.verbose,
logger_name="adb",
test_root='/sdcard/tests')
self.device = ADBDevice(
verbose=self.verbose, logger_name="adb", test_root="/sdcard/tests"
)
except Exception:
logger.error("Cannot initialize device")
raise
Expand Down
46 changes: 39 additions & 7 deletions testing/condprofile/condprof/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,34 @@

from condprof import check_install # NOQA
from condprof import progress
from condprof.util import download_file, TASK_CLUSTER, logger, ArchiveNotFound
from condprof.util import (
download_file,
TASK_CLUSTER,
logger,
check_exists,
ArchiveNotFound,
)
from condprof.changelog import Changelog


ROOT_URL = "https://firefox-ci-tc.services.mozilla.com/api/index"
TC_SERVICE = "https://firefox-ci-tc.services.mozilla.com"
ROOT_URL = TC_SERVICE + "/api/index"
INDEX_PATH = "gecko.v2.%(repo)s.latest.firefox.condprof-%(platform)s"
PUBLIC_DIR = "artifacts/public/condprof"
TC_LINK = ROOT_URL + "/v1/task/" + INDEX_PATH + "/" + PUBLIC_DIR + "/"
ARTIFACT_NAME = "profile-%(platform)s-%(scenario)s-%(customization)s.tgz"
CHANGELOG_LINK = (
ROOT_URL + "/v1/task/" + INDEX_PATH + "/" + PUBLIC_DIR + "/changelog.json"
)
DIRECT_LINK = "https://taskcluster-artifacts.net/%(task_id)s/0/public/condprof/"
ARTIFACTS_SERVICE = "https://taskcluster-artifacts.net"
DIRECT_LINK = ARTIFACTS_SERVICE + "/%(task_id)s/0/public/condprof/"
CONDPROF_CACHE = "~/.condprof-cache"
RETRIES = 3
RETRY_PAUSE = 30
RETRY_PAUSE = 45


class ServiceUnreachableError(Exception):
pass


class ProfileNotFoundError(Exception):
Expand All @@ -42,6 +54,21 @@ class RetriesError(Exception):
pass


def _check_service(url):
"""Sanity check to see if we can reach the service root url.
"""

def _check():
exists, _ = check_exists(url, all_types=True)
if not exists:
raise ServiceUnreachableError(url)

try:
return _retries(_check)
except RetriesError:
raise ServiceUnreachableError(url)


def _check_profile(profile_dir):
"""Checks for prefs we need to remove or set.
"""
Expand Down Expand Up @@ -74,6 +101,8 @@ def _clean_pref_file(name):

def _retries(callable, onerror=None):
retries = 0
pause = RETRY_PAUSE

while retries < RETRIES:
try:
return callable()
Expand All @@ -82,7 +111,9 @@ def _retries(callable, onerror=None):
onerror(e)
logger.info("Failed, retrying")
retries += 1
time.sleep(RETRY_PAUSE)
time.sleep(pause)
pause *= 1.5

# If we reach that point, it means all attempts failed
logger.error("All attempt failed")
raise RetriesError()
Expand Down Expand Up @@ -114,8 +145,10 @@ def get_profile(
filename = ARTIFACT_NAME % params
if task_id is None:
url = TC_LINK % params + filename
_check_service(TC_SERVICE)
else:
url = DIRECT_LINK % params + filename
_check_service(ARTIFACTS_SERVICE)

logger.info("preparing download dir")
if not download_cache:
Expand All @@ -135,7 +168,6 @@ def _get_profile():
archive = download_file(url, target=downloaded_archive)
except ArchiveNotFound:
raise ProfileNotFoundError(url)

try:
with tarfile.open(archive, "r:gz") as tar:
logger.info("Extracting the tarball content in %s" % target_dir)
Expand Down Expand Up @@ -174,7 +206,7 @@ def onerror(error):

try:
return _retries(_get_profile, onerror)
except Exception:
except RetriesError:
raise ProfileNotFoundError(url)


Expand Down
6 changes: 4 additions & 2 deletions testing/condprofile/condprof/tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

from mozprofile.prefs import Preferences

from condprof.client import get_profile, ROOT_URL
from condprof.client import get_profile, TC_SERVICE, ROOT_URL
from condprof.util import _DEFAULT_SERVER


Expand Down Expand Up @@ -53,6 +53,8 @@ def setUp(self):
status=200,
)

responses.add(responses.HEAD, TC_SERVICE, body="", status=200)

secret = {"secret": {"username": "user", "password": "pass"}}
secret = json.dumps(secret)
for pattern in (SECRETS, SECRETS_PROXY):
Expand Down Expand Up @@ -97,7 +99,7 @@ def test_cache(self):

# and do a single extra HEAD call, everything else is cached,
# even the TC secret
self.assertEqual(len(responses.calls), 1)
self.assertEqual(len(responses.calls), 2)

prefs_js = os.path.join(self.target, "prefs.js")
prefs = Preferences.read_prefs(prefs_js)
Expand Down
24 changes: 20 additions & 4 deletions testing/condprofile/condprof/tests/test_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,23 @@
import shutil
import re
import asyncio
import tarfile

import responses

from condprof.runner import main
from condprof.client import ROOT_URL
from condprof.main import main
from condprof.client import ROOT_URL, TC_SERVICE
from condprof import client


client.RETRIES = 1
client.RETRY_PAUSE = 0
GECKODRIVER = os.path.join(os.path.dirname(__file__), "fakegeckodriver.py")
FIREFOX = os.path.join(os.path.dirname(__file__), "fakefirefox.py")
CHANGELOG = re.compile(ROOT_URL + "/.*/changelog.json")
FTP = "https://ftp.mozilla.org/pub/firefox/nightly/latest-mozilla-central/"
PROFILE = re.compile(ROOT_URL + "/.*/.*tgz")

with open(os.path.join(os.path.dirname(__file__), "profile.tgz"), "rb") as f:
PROFILE_DATA = f.read()

with open(os.path.join(os.path.dirname(__file__), "ftp_mozilla.html")) as f:
FTP_PAGE = f.read()
Expand Down Expand Up @@ -47,6 +50,17 @@ def setUp(self):
responses.GET, FTP, content_type="text/html", body=FTP_PAGE, status=200
)

profile_tgz = os.path.join(os.path.dirname(__file__), "profile.tgz")
profile = os.path.join(os.path.dirname(__file__), "profile")

with tarfile.open(profile_tgz, "w:gz") as tar:
tar.add(profile, arcname=".")

with open(profile_tgz, "rb") as f:
PROFILE_DATA = f.read()

os.remove(profile_tgz)

responses.add(
responses.GET,
FTP_ARCHIVE,
Expand Down Expand Up @@ -81,6 +95,8 @@ def setUp(self):
responses.HEAD, ADDON, body="", headers={"content-length": "1"}, status=200
)

responses.add(responses.HEAD, TC_SERVICE, body="", status=200)

def tearDown(self):
shutil.rmtree(self.archive_dir)

Expand Down
8 changes: 6 additions & 2 deletions testing/condprofile/condprof/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def get_firefox_download_link():
raise Exception()


def check_exists(archive, server=None):
def check_exists(archive, server=None, all_types=False):
if server is not None:
archive = server + "/" + archive
try:
Expand All @@ -178,7 +178,11 @@ def check_exists(archive, server=None):
return check_exists(resp.headers["Location"])

# see Bug 1574854
if resp.status_code == 200 and "text/html" in resp.headers["Content-Type"]:
if (
not all_types
and resp.status_code == 200
and "text/html" in resp.headers["Content-Type"]
):
logger.info("Got an html page back")
exists = False
else:
Expand Down

0 comments on commit e8eef80

Please sign in to comment.