Skip to content

fetch-configlet: use logging module instead of print #2188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cmccandless merged 3 commits into from
Mar 8, 2020
Merged

fetch-configlet: use logging module instead of print #2188

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74bcff8
fetch-configlet: use logging module instead of print
cmccandless Mar 6, 2020
978afde
better error logging
cmccandless Mar 6, 2020
35241a3
Update fetch-configlet
cmccandless Mar 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions bin/fetch-configlet
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from datetime import datetime, timezone
import io
import json
import logging
import platform
import sys
import tarfile
Expand All @@ -10,9 +11,13 @@ from urllib.request import urlopen
from urllib.error import HTTPError, URLError


logging.basicConfig(level=logging.INFO)
logger = logging.getLogger(__name__)
Comment thread
yawpitch marked this conversation as resolved.

BACKOFF_TIME = 5 # in seconds
MAX_RETRIES = 5


def download_and_extract(url):
try:
with urlopen(url) as request:
Expand All @@ -22,15 +27,15 @@ def download_and_extract(url):
status = err.code
response = err.reason
except URLError as err:
print(err.reason)
logger.error(err.reason)
time.sleep(BACKOFF_TIME)
return False
if status >= 400:
print(f"Unexpected {status} status from download server.")
logger.error(f"Unexpected {status} status from download server: {response}")
time.sleep(BACKOFF_TIME)
return False

print("extracting...")
logger.info("extracting...")
with tarfile.open(fileobj=io.BytesIO(response), mode="r:*") as file:
file.extractall(path="bin/")
return True
Expand Down Expand Up @@ -62,11 +67,11 @@ def fetch_configlet():
headers = err.headers
response = err.reason
except URLError as err:
print(err.reason)
logger.exception(err)
time.sleep(BACKOFF_TIME)
return 1
if status >= 500:
print(f"Sleeping due to {status} response from API.")
logger.info(f"Sleeping due to {status} response from API: {response}")
time.sleep(BACKOFF_TIME)
return 1
if status == 403:
Expand All @@ -76,18 +81,19 @@ def fetch_configlet():
delta = wait_until - datetime.now(timezone.utc)
seconds = delta.total_seconds()
wait = seconds + 5 if seconds > BACKOFF_TIME else BACKOFF_TIME
print(f"Rate limited, sleeping {wait} seconds.")
logger.info(f"Rate limited, sleeping {wait} seconds.")
time.sleep(wait)
return 1
if status >= 400:
sys.exit(f"Received unexpected {status} response from API.")
logger.error(f"Received unexpected {status} response from API: {response}")
return 1
Comment on lines +88 to +89
Copy link
Copy Markdown
Contributor

@AnAccountForReportingBugs AnAccountForReportingBugs Mar 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, I originally did that as sys.exit() because my thinking was a client error other than a 403 from the API is probably a permanent condition within the lifetime of the script run. Probably doesn't matter either way.

Copy link
Copy Markdown
Contributor Author

@cmccandless cmccandless Mar 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intent noted. It still seems the wrong approach to allow the function itself to trigger a full program exit. If we wanted to keep that short-circuit functionality, the best approach would probably be to raise some sort of unique exception to allow the caller to handle the failure however it wishes.

data = json.loads(response.decode("utf-8"))
version = data["tag_name"]
machine_info = f"{get_os()}-{get_arch()}"
name = f"configlet-{machine_info}.tgz"
for asset in data["assets"]:
if asset["name"] == name:
print(f"Downloading configlet {version} for {machine_info}")
logger.info(f"Downloading configlet {version} for {machine_info}")
for _ in range(MAX_RETRIES):
if download_and_extract(asset["browser_download_url"]):
return 0
Expand Down