From 56d4d13d4d787f7fd28dc6bd2a7ea9ec81f111ba Mon Sep 17 00:00:00 2001 From: A_D Date: Mon, 6 Jul 2020 21:26:55 +0200 Subject: [PATCH 01/14] Fixed line spacing and comment length This adds newlines after blocks and in other logical places to assist with reading the code. Additionally, comments that were too long to remain inline within a 120 character limit have been moved above the line they reference. --- companion.py | 163 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 116 insertions(+), 47 deletions(-) diff --git a/companion.py b/companion.py index 30c01106..74a2760e 100644 --- a/companion.py +++ b/companion.py @@ -22,10 +22,14 @@ import zlib from config import appname, appversion, config from protocol import protocolhandler +from typing import TYPE_CHECKING +if TYPE_CHECKING: + _ = lambda x: x # noqa -holdoff = 60 # be nice -timeout = 10 # requests timeout -auth_timeout = 30 # timeout for initial auth + +holdoff = 60 # be nice +timeout = 10 # requests timeout +auth_timeout = 30 # timeout for initial auth # Currently the "Elite Dangerous Market Connector (EDCD/Athanasius)" one in # Athanasius' Frontier account @@ -47,7 +51,7 @@ category_map = { 'Narcotics' : 'Legal Drugs', 'Slaves' : 'Slavery', 'Waste ' : 'Waste', - 'NonMarketable' : False, # Don't appear in the in-game market so don't report + 'NonMarketable' : False # Don't appear in the in-game market so don't report } commodity_map = {} @@ -105,47 +109,62 @@ ship_map = { # In practice these arrays aren't very sparse so just convert them to lists with any 'gaps' holding None. def listify(thing): if thing is None: - return [] # data is not present + return [] # data is not present + elif isinstance(thing, list): - return list(thing) # array is not sparse + return list(thing) # array is not sparse + elif isinstance(thing, dict): retval = [] - for k,v in thing.items(): + for k, v in thing.items(): idx = int(k) + if idx >= len(retval): retval.extend([None] * (idx - len(retval))) retval.append(v) else: retval[idx] = v + return retval + else: - assert False, thing # we expect an array or a sparse array - return list(thing) # hope for the best + assert False, thing # we expect an array or a sparse array + return list(thing) # hope for the best class ServerError(Exception): def __init__(self, *args): - self.args = args if args else (_('Error: Frontier server is down'),) # Raised when cannot contact the Companion API server + # Raised when cannot contact the Companion API server + self.args = args if args else (_('Error: Frontier server is down'),) + class ServerLagging(Exception): def __init__(self, *args): - self.args = args if args else (_('Error: Frontier server is lagging'),) # Raised when Companion API server is returning old data, e.g. when the servers are too busy + # Raised when Companion API server is returning old data, e.g. when the servers are too busy + self.args = args if args else (_('Error: Frontier server is lagging'),) + class SKUError(Exception): def __init__(self, *args): - self.args = args if args else (_('Error: Frontier server SKU problem'),) # Raised when the Companion API server thinks that the user has not purchased E:D. i.e. doesn't have the correct 'SKU' + # Raised when the Companion API server thinks that the user has not purchased E:D + # i.e. doesn't have the correct 'SKU' + self.args = args if args else (_('Error: Frontier server SKU problem'),) + class CredentialsError(Exception): def __init__(self, *args): self.args = args if args else (_('Error: Invalid Credentials'),) + class CmdrError(Exception): def __init__(self, *args): - self.args = args if args else (_('Error: Wrong Cmdr'),) # Raised when the user has multiple accounts and the username/password setting is not for the account they're currently playing OR the user has reset their Cmdr and the Companion API server is still returning data for the old Cmdr + # Raised when the user has multiple accounts and the username/password setting is not for + # the account they're currently playing OR the user has reset their Cmdr and the Companion API + # server is still returning data for the old Cmdr + self.args = args if args else (_('Error: Wrong Cmdr'),) class Auth(object): - def __init__(self, cmdr): self.cmdr = cmdr self.session = requests.Session() @@ -166,19 +185,23 @@ class Auth(object): 'client_id': CLIENT_ID, 'refresh_token': tokens[idx], } + r = self.session.post(SERVER_AUTH + URL_TOKEN, data=data, timeout=auth_timeout) if r.status_code == requests.codes.ok: data = r.json() tokens[idx] = data.get('refresh_token', '') config.set('fdev_apikeys', tokens) - config.save() # Save settings now for use by command-line app + config.save() # Save settings now for use by command-line app return data.get('access_token') + else: print('Auth\tCan\'t refresh token for %s' % self.cmdr) self.dump(r) + except: print('Auth\tCan\'t refresh token for %s' % self.cmdr) print_exc() + else: print('Auth\tNo token for %s' % self.cmdr) @@ -193,23 +216,26 @@ class Auth(object): def authorize(self, payload): # Handle OAuth authorization code callback. Returns access token if successful, otherwise raises CredentialsError - if not '?' in payload: + if '?' not in payload: print('Auth\tMalformed response "%s"' % payload) - raise CredentialsError() # Not well formed + raise CredentialsError() # Not well formed data = urllib.parse.parse_qs(payload[payload.index('?')+1:]) if not self.state or not data.get('state') or data['state'][0] != self.state: print('Auth\tUnexpected response "%s"' % payload) - raise CredentialsError() # Unexpected reply + raise CredentialsError() # Unexpected reply if not data.get('code'): print('Auth\tNegative response "%s"' % payload) if data.get('error_description'): raise CredentialsError('Error: %s' % data['error_description'][0]) + elif data.get('error'): raise CredentialsError('Error: %s' % data['error'][0]) + elif data.get('message'): raise CredentialsError('Error: %s' % data['message'][0]) + else: raise CredentialsError() @@ -222,6 +248,7 @@ class Auth(object): 'code': data['code'][0], 'redirect_uri': protocolhandler.redirect, } + r = self.session.post(SERVER_AUTH + URL_TOKEN, data=data, timeout=auth_timeout) data = r.json() if r.status_code == requests.codes.ok: @@ -232,8 +259,9 @@ class Auth(object): tokens = tokens + [''] * (len(cmdrs) - len(tokens)) tokens[idx] = data.get('refresh_token', '') config.set('fdev_apikeys', tokens) - config.save() # Save settings now for use by command-line app + config.save() # Save settings now for use by command-line app return data.get('access_token') + except: print('Auth\tCan\'t get token for %s' % self.cmdr) print_exc() @@ -244,10 +272,13 @@ class Auth(object): self.dump(r) if data.get('error_description'): raise CredentialsError('Error: %s' % data['error_description']) + elif data.get('error'): raise CredentialsError('Error: %s' % data['error']) + elif data.get('message'): raise CredentialsError('Error: %s' % data['message']) + else: raise CredentialsError() @@ -260,7 +291,7 @@ class Auth(object): tokens = tokens + [''] * (len(cmdrs) - len(tokens)) tokens[idx] = '' config.set('fdev_apikeys', tokens) - config.save() # Save settings now for use by command-line app + config.save() # Save settings now for use by command-line app def dump(self, r): print('Auth\t' + r.url, r.status_code, r.reason and r.reason or 'None', r.text) @@ -270,7 +301,6 @@ class Auth(object): class Session(object): - STATE_INIT, STATE_AUTH, STATE_OK = list(range(3)) def __init__(self): @@ -278,10 +308,10 @@ class Session(object): self.credentials = None self.session = None self.auth = None - self.retrying = False # Avoid infinite loop when successful auth / unsuccessful query + self.retrying = False # Avoid infinite loop when successful auth / unsuccessful query # yuck suppress InsecurePlatformWarning under Python < 2.7.9 which lacks SNI support - if sys.version_info < (2,7,9): + if sys.version_info < (2, 7, 9): from requests.packages import urllib3 urllib3.disable_warnings() @@ -289,16 +319,20 @@ class Session(object): # Returns True if login succeeded, False if re-authorization initiated. if not CLIENT_ID: raise CredentialsError() + if not cmdr or is_beta is None: # Use existing credentials if not self.credentials: - raise CredentialsError() # Shouldn't happen + raise CredentialsError() # Shouldn't happen + elif self.state == Session.STATE_OK: - return True # already logged in + return True # already logged in + else: credentials = {'cmdr': cmdr, 'beta': is_beta} if self.credentials == credentials and self.state == Session.STATE_OK: - return True # already logged in + return True # already logged in + else: # changed account or retrying login during auth self.close() @@ -307,11 +341,13 @@ class Session(object): self.server = self.credentials['beta'] and SERVER_BETA or SERVER_LIVE self.state = Session.STATE_INIT self.auth = Auth(self.credentials['cmdr']) + access_token = self.auth.refresh() if access_token: self.auth = None self.start(access_token) return True + else: self.state = Session.STATE_AUTH return False @@ -320,14 +356,16 @@ class Session(object): # Callback from protocol handler def auth_callback(self): if self.state != Session.STATE_AUTH: - raise CredentialsError() # Shouldn't be getting a callback + raise CredentialsError() # Shouldn't be getting a callback + try: self.start(self.auth.authorize(protocolhandler.lastpayload)) self.auth = None + except: - self.state = Session.STATE_INIT # Will try to authorize again on next login or query + self.state = Session.STATE_INIT # Will try to authorize again on next login or query self.auth = None - raise # Bad thing happened + raise # Bad thing happened def start(self, access_token): self.session = requests.Session() @@ -339,11 +377,13 @@ class Session(object): if self.state == Session.STATE_INIT: if self.login(): return self.query(endpoint) + elif self.state == Session.STATE_AUTH: raise CredentialsError() try: r = self.session.get(self.server + endpoint, timeout=timeout) + except: if __debug__: print_exc() raise ServerError() @@ -355,26 +395,31 @@ class Session(object): self.retrying = False self.login() raise CredentialsError() + elif 500 <= r.status_code < 600: # Server error. Typically 500 "Internal Server Error" if server is down self.dump(r) raise ServerError() try: - r.raise_for_status() # Typically 403 "Forbidden" on token expiry - data = r.json() # May also fail here if token expired since response is empty + r.raise_for_status() # Typically 403 "Forbidden" on token expiry + data = r.json() # May also fail here if token expired since response is empty + except: print_exc() self.dump(r) self.close() + if self.retrying: # Refresh just succeeded but this query failed! Force full re-authentication self.invalidate() self.retrying = False self.login() raise CredentialsError() + elif self.login(): # Maybe our token expired. Re-authorize in any case self.retrying = True return self.query(endpoint) + else: self.retrying = False raise CredentialsError() @@ -382,6 +427,7 @@ class Session(object): self.retrying = False if 'timestamp' not in data: data['timestamp'] = time.strftime('%Y-%m-%dT%H:%M:%SZ', parsedate(r.headers['Date'])) + return data def profile(self): @@ -391,20 +437,24 @@ class Session(object): data = self.query(URL_QUERY) if data['commander'].get('docked'): services = data['lastStarport'].get('services', {}) + if services.get('commodities'): + marketdata = self.query(URL_MARKET) - if (data['lastStarport']['name'] != marketdata['name'] or - int(data['lastStarport']['id']) != int(marketdata['id'])): + if (data['lastStarport']['name'] != marketdata['name'] or int(data['lastStarport']['id']) != int(marketdata['id'])): raise ServerLagging() + else: data['lastStarport'].update(marketdata) + if services.get('outfitting') or services.get('shipyard'): shipdata = self.query(URL_SHIPYARD) - if (data['lastStarport']['name'] != shipdata['name'] or - int(data['lastStarport']['id']) != int(shipdata['id'])): + if (data['lastStarport']['name'] != shipdata['name'] or int(data['lastStarport']['id']) != int(shipdata['id'])): raise ServerLagging() + else: data['lastStarport'].update(shipdata) + return data def close(self): @@ -412,8 +462,10 @@ class Session(object): if self.session: try: self.session.close() + except: if __debug__: print_exc() + self.session = None def invalidate(self): @@ -425,14 +477,15 @@ class Session(object): print('cAPI\t' + r.url, r.status_code, r.reason and r.reason or 'None', r.text) -# Returns a shallow copy of the received data suitable for export to older tools - English commodity names and anomalies fixed up +# Returns a shallow copy of the received data suitable for export to older tools +# English commodity names and anomalies fixed up def fixup(data): - if not commodity_map: # Lazily populate for f in ['commodity.csv', 'rare_commodity.csv']: with open(join(config.respath, f), 'r') as csvfile: reader = csv.DictReader(csvfile) + for row in reader: commodity_map[row['symbol']] = (row['category'], row['name']) @@ -446,24 +499,33 @@ def fixup(data): if not isinstance(commodity.get(thing), numbers.Number): if __debug__: print('Invalid "%s":"%s" (%s) for "%s"' % (thing, commodity.get(thing), type(commodity.get(thing)), commodity.get('name', ''))) break + else: - if not category_map.get(commodity['categoryname'], True): # Check not marketable i.e. Limpets + # Check not marketable i.e. Limpets + if not category_map.get(commodity['categoryname'], True): pass - elif commodity['demandBracket'] == 0 and commodity['stockBracket'] == 0: # Check not normally stocked e.g. Salvage + + # Check not normally stocked e.g. Salvage + elif commodity['demandBracket'] == 0 and commodity['stockBracket'] == 0: pass - elif commodity.get('legality'): # Check not prohibited + elif commodity.get('legality'): # Check not prohibited pass + elif not commodity.get('categoryname'): if __debug__: print('Missing "categoryname" for "%s"' % commodity.get('name', '')) + elif not commodity.get('name'): if __debug__: print('Missing "name" for a commodity in "%s"' % commodity.get('categoryname', '')) + elif not commodity['demandBracket'] in range(4): if __debug__: print('Invalid "demandBracket":"%s" for "%s"' % (commodity['demandBracket'], commodity['name'])) + elif not commodity['stockBracket'] in range(4): if __debug__: print('Invalid "stockBracket":"%s" for "%s"' % (commodity['stockBracket'], commodity['name'])) + else: # Rewrite text fields - new = dict(commodity) # shallow copy + new = dict(commodity) # shallow copy if commodity['name'] in commodity_map: (new['categoryname'], new['name']) = commodity_map[commodity['name']] elif commodity['categoryname'] in category_map: @@ -488,22 +550,27 @@ def fixup(data): # Return a subset of the received data describing the current ship def ship(data): - def filter_ship(d): filtered = {} for k, v in d.items(): if v == []: - pass # just skip empty fields for brevity + pass # just skip empty fields for brevity + elif k in ['alive', 'cargo', 'cockpitBreached', 'health', 'oxygenRemaining', 'rebuilds', 'starsystem', 'station']: - pass # noisy + pass # noisy + elif k in ['locDescription', 'locName'] or k.endswith('LocDescription') or k.endswith('LocName'): - pass # also noisy, and redundant + pass # also noisy, and redundant + elif k in ['dir', 'LessIsGood']: - pass # dir is not ASCII - remove to simplify handling + pass # dir is not ASCII - remove to simplify handling + elif hasattr(v, 'items'): filtered[k] = filter_ship(v) + else: filtered[k] = v + return filtered # subset of "ship" that's not noisy @@ -515,11 +582,13 @@ def ship_file_name(ship_name, ship_type): name = str(ship_name or ship_map.get(ship_type.lower(), ship_type)).strip() if name.endswith('.'): name = name[:-1] + if name.lower() in ['con', 'prn', 'aux', 'nul', 'com1', 'com2', 'com3', 'com4', 'com5', 'com6', 'com7', 'com8', 'com9', 'lpt1', 'lpt2', 'lpt3', 'lpt4', 'lpt5', 'lpt6', 'lpt7', 'lpt8', 'lpt9']: name = name + '_' - return name.translate({ ord(x): u'_' for x in ['\0', '<', '>', ':', '"', '/', '\\', '|', '?', '*'] }) + + return name.translate({ord(x): u'_' for x in ['\0', '<', '>', ':', '"', '/', '\\', '|', '?', '*']}) # singleton From 765dc0bcf31e6d5a7a13b987b329ee12257accba Mon Sep 17 00:00:00 2001 From: A_D Date: Mon, 6 Jul 2020 21:31:04 +0200 Subject: [PATCH 02/14] Replaced oneline ifs with multiline ifs Oneline ifs are in general a bad idea in whitespace significant languages, this replaces them with their multi-line equivalents --- companion.py | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/companion.py b/companion.py index 74a2760e..19d191c9 100644 --- a/companion.py +++ b/companion.py @@ -260,12 +260,15 @@ class Auth(object): tokens[idx] = data.get('refresh_token', '') config.set('fdev_apikeys', tokens) config.save() # Save settings now for use by command-line app + return data.get('access_token') except: print('Auth\tCan\'t get token for %s' % self.cmdr) print_exc() - if r: self.dump(r) + if r: + self.dump(r) + raise CredentialsError() print('Auth\tCan\'t get token for %s' % self.cmdr) @@ -385,7 +388,9 @@ class Session(object): r = self.session.get(self.server + endpoint, timeout=timeout) except: - if __debug__: print_exc() + if __debug__: + print_exc() + raise ServerError() if r.url.startswith(SERVER_AUTH): @@ -464,7 +469,8 @@ class Session(object): self.session.close() except: - if __debug__: print_exc() + if __debug__: + print_exc() self.session = None @@ -497,7 +503,8 @@ def fixup(data): # But also see https://github.com/Marginal/EDMarketConnector/issues/32 for thing in ['buyPrice', 'sellPrice', 'demand', 'demandBracket', 'stock', 'stockBracket']: if not isinstance(commodity.get(thing), numbers.Number): - if __debug__: print('Invalid "%s":"%s" (%s) for "%s"' % (thing, commodity.get(thing), type(commodity.get(thing)), commodity.get('name', ''))) + if __debug__: + print('Invalid "%s":"%s" (%s) for "%s"' % (thing, commodity.get(thing), type(commodity.get(thing)), commodity.get('name', ''))) break else: @@ -512,16 +519,20 @@ def fixup(data): pass elif not commodity.get('categoryname'): - if __debug__: print('Missing "categoryname" for "%s"' % commodity.get('name', '')) + if __debug__: + print('Missing "categoryname" for "%s"' % commodity.get('name', '')) elif not commodity.get('name'): - if __debug__: print('Missing "name" for a commodity in "%s"' % commodity.get('categoryname', '')) + if __debug__: + print('Missing "name" for a commodity in "%s"' % commodity.get('categoryname', '')) elif not commodity['demandBracket'] in range(4): - if __debug__: print('Invalid "demandBracket":"%s" for "%s"' % (commodity['demandBracket'], commodity['name'])) + if __debug__: + print('Invalid "demandBracket":"%s" for "%s"' % (commodity['demandBracket'], commodity['name'])) elif not commodity['stockBracket'] in range(4): - if __debug__: print('Invalid "stockBracket":"%s" for "%s"' % (commodity['stockBracket'], commodity['name'])) + if __debug__: + print('Invalid "stockBracket":"%s" for "%s"' % (commodity['stockBracket'], commodity['name'])) else: # Rewrite text fields From 7260dff9a297d02dce46aeb7164b08f339221115 Mon Sep 17 00:00:00 2001 From: A_D Date: Mon, 6 Jul 2020 21:56:23 +0200 Subject: [PATCH 03/14] Replaced modulo formatting with .format calls Modulo formatting is py2 (and C printf) style, its arcane and incredibly hard to read for large formats. I used keyed .formats where there were more than a few format specifiers --- companion.py | 82 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 30 deletions(-) diff --git a/companion.py b/companion.py index 19d191c9..b03e326c 100644 --- a/companion.py +++ b/companion.py @@ -33,7 +33,8 @@ auth_timeout = 30 # timeout for initial auth # Currently the "Elite Dangerous Market Connector (EDCD/Athanasius)" one in # Athanasius' Frontier account -CLIENT_ID = os.getenv('CLIENT_ID') or 'fb88d428-9110-475f-a3d2-dc151c2b9c7a' # Obtain from https://auth.frontierstore.net/client/signup +# Obtain from https://auth.frontierstore.net/client/signup +CLIENT_ID = os.getenv('CLIENT_ID') or 'fb88d428-9110-475f-a3d2-dc151c2b9c7a' SERVER_AUTH = 'https://auth.frontierstore.net' URL_AUTH = '/auth' URL_TOKEN = '/token' @@ -168,7 +169,7 @@ class Auth(object): def __init__(self, cmdr): self.cmdr = cmdr self.session = requests.Session() - self.session.headers['User-Agent'] = 'EDCD-%s-%s' % (appname, appversion) + self.session.headers['User-Agent'] = 'EDCD-{}-{}'.format(appname, appversion) self.verifier = self.state = None def refresh(self): @@ -195,15 +196,15 @@ class Auth(object): return data.get('access_token') else: - print('Auth\tCan\'t refresh token for %s' % self.cmdr) + print('Auth\tCan\'t refresh token for {}'.format(self.cmdr)) self.dump(r) except: - print('Auth\tCan\'t refresh token for %s' % self.cmdr) + print('Auth\tCan\'t refresh token for {}'.format(self.cmdr)) print_exc() else: - print('Auth\tNo token for %s' % self.cmdr) + print('Auth\tNo token for {}'.format(self.cmdr)) # New request print('Auth\tNew authorization request') @@ -212,29 +213,43 @@ class Auth(object): s = random.SystemRandom().getrandbits(8 * 32) self.state = self.base64URLEncode(s.to_bytes(32, byteorder='big')) # Won't work under IE: https://blogs.msdn.microsoft.com/ieinternals/2011/07/13/understanding-protocols/ - webbrowser.open('%s%s?response_type=code&audience=frontier&scope=capi&client_id=%s&code_challenge=%s&code_challenge_method=S256&state=%s&redirect_uri=%s' % (SERVER_AUTH, URL_AUTH, CLIENT_ID, self.base64URLEncode(hashlib.sha256(self.verifier).digest()), self.state, protocolhandler.redirect)) + webbrowser.open( + + '{server_auth}{url_auth}?response_type=code&audience=frontier&scope=capi&client_id={client_id}&code_challenge={challenge}&code_challenge_method=S256&state={state}&redirect_uri={redirect}'.format( # noqa: E501 + server_auth=SERVER_AUTH, + url_auth=URL_AUTH, + client_id=CLIENT_ID, + challenge=self.base64URLEncode(hashlib.sha256(self.verifier).digest()), + state=self.state, + redirect=protocolhandler.redirect + ) + ) def authorize(self, payload): - # Handle OAuth authorization code callback. Returns access token if successful, otherwise raises CredentialsError + # Handle OAuth authorization code callback. + # Returns access token if successful, otherwise raises CredentialsError if '?' not in payload: - print('Auth\tMalformed response "%s"' % payload) + print('Auth\tMalformed response {!r}'.format(payload)) raise CredentialsError() # Not well formed - data = urllib.parse.parse_qs(payload[payload.index('?')+1:]) + data = urllib.parse.parse_qs(payload[payload.index('?') + 1:]) if not self.state or not data.get('state') or data['state'][0] != self.state: - print('Auth\tUnexpected response "%s"' % payload) + print('Auth\tUnexpected response {!r}'.format(payload)) raise CredentialsError() # Unexpected reply if not data.get('code'): - print('Auth\tNegative response "%s"' % payload) + print('Auth\tNegative response {!r}'.format(payload)) + + # TODO(A_D): there should be a cleaner way to do this rather than raising in every if + # Additionally, this is basically the same code as seen below, helper method perhaps? if data.get('error_description'): - raise CredentialsError('Error: %s' % data['error_description'][0]) + raise CredentialsError('Error: {!r}'.format(data['error_description'][0])) elif data.get('error'): - raise CredentialsError('Error: %s' % data['error'][0]) + raise CredentialsError('Error: {!r}'.format(data['error'][0])) elif data.get('message'): - raise CredentialsError('Error: %s' % data['message'][0]) + raise CredentialsError('Error: {!r}'.format(data['message'][0])) else: raise CredentialsError() @@ -252,7 +267,7 @@ class Auth(object): r = self.session.post(SERVER_AUTH + URL_TOKEN, data=data, timeout=auth_timeout) data = r.json() if r.status_code == requests.codes.ok: - print('Auth\tNew token for %s' % self.cmdr) + print('Auth\tNew token for {}'.format(self.cmdr)) cmdrs = config.get('cmdrs') idx = cmdrs.index(self.cmdr) tokens = config.get('fdev_apikeys') or [] @@ -264,30 +279,30 @@ class Auth(object): return data.get('access_token') except: - print('Auth\tCan\'t get token for %s' % self.cmdr) + print('Auth\tCan\'t get token for {}'.format(self.cmdr)) print_exc() if r: self.dump(r) - raise CredentialsError() + raise CredentialsError() # TODO(A_D): Probably give more info about the error here - print('Auth\tCan\'t get token for %s' % self.cmdr) + print('Auth\tCan\'t get token for {}'.format(self.cmdr)) self.dump(r) if data.get('error_description'): - raise CredentialsError('Error: %s' % data['error_description']) + raise CredentialsError('Error: {!r}'.format(data['error_description'])) elif data.get('error'): - raise CredentialsError('Error: %s' % data['error']) + raise CredentialsError('Error: {!r}'.format(data['error'])) elif data.get('message'): - raise CredentialsError('Error: %s' % data['message']) + raise CredentialsError('Error: {!r}'.format(data['message'])) else: raise CredentialsError() @staticmethod def invalidate(cmdr): - print('Auth\tInvalidated token for %s' % cmdr) + print('Auth\tInvalidated token for {}'.format(cmdr)) cmdrs = config.get('cmdrs') idx = cmdrs.index(cmdr) tokens = config.get('fdev_apikeys') or [] @@ -297,7 +312,7 @@ class Auth(object): config.save() # Save settings now for use by command-line app def dump(self, r): - print('Auth\t' + r.url, r.status_code, r.reason and r.reason or 'None', r.text) + print('Auth\t' + r.url, r.status_code, r.reason if r.reason else 'None', r.text) def base64URLEncode(self, text): return base64.urlsafe_b64encode(text).decode().replace('=', '') @@ -372,8 +387,8 @@ class Session(object): def start(self, access_token): self.session = requests.Session() - self.session.headers['Authorization'] = 'Bearer %s' % access_token - self.session.headers['User-Agent'] = 'EDCD-%s-%s' % (appname, appversion) + self.session.headers['Authorization'] = 'Bearer {}'.format(access_token) + self.session.headers['User-Agent'] = 'EDCD-{appname}-{version}'.format(appname=appname, version=appversion) self.state = Session.STATE_OK def query(self, endpoint): @@ -504,7 +519,14 @@ def fixup(data): for thing in ['buyPrice', 'sellPrice', 'demand', 'demandBracket', 'stock', 'stockBracket']: if not isinstance(commodity.get(thing), numbers.Number): if __debug__: - print('Invalid "%s":"%s" (%s) for "%s"' % (thing, commodity.get(thing), type(commodity.get(thing)), commodity.get('name', ''))) + print( + 'Invalid {!r}:{!r} ({}) for {!r}'.format( + thing, + commodity.get(thing), + type(commodity.get(thing)), + commodity.get('name', '') + ) + ) break else: @@ -520,19 +542,19 @@ def fixup(data): elif not commodity.get('categoryname'): if __debug__: - print('Missing "categoryname" for "%s"' % commodity.get('name', '')) + print('Missing "categoryname" for {!r}'.format(commodity.get('name', ''))) elif not commodity.get('name'): if __debug__: - print('Missing "name" for a commodity in "%s"' % commodity.get('categoryname', '')) + print('Missing "name" for a commodity in {!r}'.format(commodity.get('categoryname', ''))) elif not commodity['demandBracket'] in range(4): if __debug__: - print('Invalid "demandBracket":"%s" for "%s"' % (commodity['demandBracket'], commodity['name'])) + print('Invalid "demandBracket":{!r} for {!r}'.format(commodity['demandBracket'], commodity['name'])) elif not commodity['stockBracket'] in range(4): if __debug__: - print('Invalid "stockBracket":"%s" for "%s"' % (commodity['stockBracket'], commodity['name'])) + print('Invalid "stockBracket":{!r} for {!r}'.format(commodity['stockBracket'], commodity['name'])) else: # Rewrite text fields From 31e0758ee43f6561939260703190a4024da9aea1 Mon Sep 17 00:00:00 2001 From: A_D Date: Mon, 6 Jul 2020 22:19:23 +0200 Subject: [PATCH 04/14] Replaced x in $list construct with x in $tuple Tuples are smaller and immutable, therefore using a tuple is cleaner. --- companion.py | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/companion.py b/companion.py index b03e326c..785f9ba5 100644 --- a/companion.py +++ b/companion.py @@ -4,27 +4,25 @@ from builtins import object import base64 import csv import requests -from http.cookiejar import LWPCookieJar # No longer needed but retained in case plugins use it +from http.cookiejar import LWPCookieJar # No longer needed but retained in case plugins use it from email.utils import parsedate import hashlib -import json import numbers import os -from os.path import dirname, isfile, join +from os.path import join import random import sys import time from traceback import print_exc import urllib.parse import webbrowser -import zlib from config import appname, appversion, config from protocol import protocolhandler from typing import TYPE_CHECKING if TYPE_CHECKING: - _ = lambda x: x # noqa + _ = lambda x: x # noqa # to make flake8 stop complaining that the hacked in _ method doesnt exist holdoff = 60 # be nice @@ -214,8 +212,7 @@ class Auth(object): self.state = self.base64URLEncode(s.to_bytes(32, byteorder='big')) # Won't work under IE: https://blogs.msdn.microsoft.com/ieinternals/2011/07/13/understanding-protocols/ webbrowser.open( - - '{server_auth}{url_auth}?response_type=code&audience=frontier&scope=capi&client_id={client_id}&code_challenge={challenge}&code_challenge_method=S256&state={state}&redirect_uri={redirect}'.format( # noqa: E501 + '{server_auth}{url_auth}?response_type=code&audience=frontier&scope=capi&client_id={client_id}&code_challenge={challenge}&code_challenge_method=S256&state={state}&redirect_uri={redirect}'.format( # noqa: E501 # I cant make this any shorter server_auth=SERVER_AUTH, url_auth=URL_AUTH, client_id=CLIENT_ID, @@ -503,7 +500,7 @@ class Session(object): def fixup(data): if not commodity_map: # Lazily populate - for f in ['commodity.csv', 'rare_commodity.csv']: + for f in ('commodity.csv', 'rare_commodity.csv'): with open(join(config.respath, f), 'r') as csvfile: reader = csv.DictReader(csvfile) @@ -514,9 +511,11 @@ def fixup(data): for commodity in data['lastStarport'].get('commodities') or []: # Check all required numeric fields are present and are numeric - # Catches "demandBracket": "" for some phantom commodites in ED 1.3 - https://github.com/Marginal/EDMarketConnector/issues/2 + # Catches "demandBracket": "" for some phantom commodites in + # ED 1.3 - https://github.com/Marginal/EDMarketConnector/issues/2 + # # But also see https://github.com/Marginal/EDMarketConnector/issues/32 - for thing in ['buyPrice', 'sellPrice', 'demand', 'demandBracket', 'stock', 'stockBracket']: + for thing in ('buyPrice', 'sellPrice', 'demand', 'demandBracket', 'stock', 'stockBracket'): if not isinstance(commodity.get(thing), numbers.Number): if __debug__: print( @@ -589,13 +588,14 @@ def ship(data): if v == []: pass # just skip empty fields for brevity - elif k in ['alive', 'cargo', 'cockpitBreached', 'health', 'oxygenRemaining', 'rebuilds', 'starsystem', 'station']: + elif k in ('alive', 'cargo', 'cockpitBreached', 'health', 'oxygenRemaining', + 'rebuilds', 'starsystem', 'station'): pass # noisy - elif k in ['locDescription', 'locName'] or k.endswith('LocDescription') or k.endswith('LocName'): + elif k in ('locDescription', 'locName') or k.endswith('LocDescription') or k.endswith('LocName'): pass # also noisy, and redundant - elif k in ['dir', 'LessIsGood']: + elif k in ('dir', 'LessIsGood'): pass # dir is not ASCII - remove to simplify handling elif hasattr(v, 'items'): @@ -616,12 +616,12 @@ def ship_file_name(ship_name, ship_type): if name.endswith('.'): name = name[:-1] - if name.lower() in ['con', 'prn', 'aux', 'nul', + if name.lower() in ('con', 'prn', 'aux', 'nul', 'com1', 'com2', 'com3', 'com4', 'com5', 'com6', 'com7', 'com8', 'com9', - 'lpt1', 'lpt2', 'lpt3', 'lpt4', 'lpt5', 'lpt6', 'lpt7', 'lpt8', 'lpt9']: + 'lpt1', 'lpt2', 'lpt3', 'lpt4', 'lpt5', 'lpt6', 'lpt7', 'lpt8', 'lpt9'): name = name + '_' - return name.translate({ord(x): u'_' for x in ['\0', '<', '>', ':', '"', '/', '\\', '|', '?', '*']}) + return name.translate({ord(x): u'_' for x in ('\0', '<', '>', ':', '"', '/', '\\', '|', '?', '*')}) # singleton From 9363b1457dd02f0534e8f00a42f95b5e534fed74 Mon Sep 17 00:00:00 2001 From: A_D Date: Tue, 7 Jul 2020 16:59:03 +0200 Subject: [PATCH 05/14] Simplified if ladder Removed a large if ladder with a simpler construct that does the same thing --- companion.py | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/companion.py b/companion.py index 785f9ba5..39674403 100644 --- a/companion.py +++ b/companion.py @@ -236,20 +236,10 @@ class Auth(object): if not data.get('code'): print('Auth\tNegative response {!r}'.format(payload)) - - # TODO(A_D): there should be a cleaner way to do this rather than raising in every if - # Additionally, this is basically the same code as seen below, helper method perhaps? - if data.get('error_description'): - raise CredentialsError('Error: {!r}'.format(data['error_description'][0])) - - elif data.get('error'): - raise CredentialsError('Error: {!r}'.format(data['error'][0])) - - elif data.get('message'): - raise CredentialsError('Error: {!r}'.format(data['message'][0])) - - else: - raise CredentialsError() + error = next( + (data[k] for k in ('error_description', 'error', 'message') if k in data), ('',) + ) + raise CredentialsError('Error: {!r}'.format(error)[0]) try: r = None @@ -285,17 +275,8 @@ class Auth(object): print('Auth\tCan\'t get token for {}'.format(self.cmdr)) self.dump(r) - if data.get('error_description'): - raise CredentialsError('Error: {!r}'.format(data['error_description'])) - - elif data.get('error'): - raise CredentialsError('Error: {!r}'.format(data['error'])) - - elif data.get('message'): - raise CredentialsError('Error: {!r}'.format(data['message'])) - - else: - raise CredentialsError() + error = next((data[k] for k in ('error_description', 'error', 'message') if k in data), ('',)) + raise CredentialsError('Error: {!r}'.format(error)[0]) @staticmethod def invalidate(cmdr): From 34b963620a43765e8b77b4c2d37a8ec6be303dfc Mon Sep 17 00:00:00 2001 From: A_D Date: Tue, 7 Jul 2020 18:21:48 +0200 Subject: [PATCH 06/14] Removed bare except clauses Bare except clauses are a fantastic way to find your HTTP requests eating your ^C. I replaced all bare excepts with Exception if I could not find a list of exceptions that could be thrown. --- companion.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/companion.py b/companion.py index 39674403..edf6b78d 100644 --- a/companion.py +++ b/companion.py @@ -197,7 +197,7 @@ class Auth(object): print('Auth\tCan\'t refresh token for {}'.format(self.cmdr)) self.dump(r) - except: + except Exception: print('Auth\tCan\'t refresh token for {}'.format(self.cmdr)) print_exc() @@ -265,13 +265,13 @@ class Auth(object): return data.get('access_token') - except: + except Exception as e: print('Auth\tCan\'t get token for {}'.format(self.cmdr)) print_exc() if r: self.dump(r) - raise CredentialsError() # TODO(A_D): Probably give more info about the error here + raise CredentialsError('unable to get token') from e print('Auth\tCan\'t get token for {}'.format(self.cmdr)) self.dump(r) @@ -358,7 +358,7 @@ class Session(object): self.start(self.auth.authorize(protocolhandler.lastpayload)) self.auth = None - except: + except Exception: self.state = Session.STATE_INIT # Will try to authorize again on next login or query self.auth = None raise # Bad thing happened @@ -380,11 +380,11 @@ class Session(object): try: r = self.session.get(self.server + endpoint, timeout=timeout) - except: + except Exception as e: if __debug__: print_exc() - raise ServerError() + raise ServerError('unable to get endpoint {}'.format(endpoint)) from e if r.url.startswith(SERVER_AUTH): # Redirected back to Auth server - force full re-authentication @@ -403,7 +403,7 @@ class Session(object): r.raise_for_status() # Typically 403 "Forbidden" on token expiry data = r.json() # May also fail here if token expired since response is empty - except: + except (requests.HTTPError, ValueError) as e: print_exc() self.dump(r) self.close() @@ -412,7 +412,7 @@ class Session(object): self.invalidate() self.retrying = False self.login() - raise CredentialsError() + raise CredentialsError('query failed after refresh') from e elif self.login(): # Maybe our token expired. Re-authorize in any case self.retrying = True @@ -420,7 +420,7 @@ class Session(object): else: self.retrying = False - raise CredentialsError() + raise CredentialsError('Invalid JSON or HTTP error') from e self.retrying = False if 'timestamp' not in data: @@ -461,7 +461,7 @@ class Session(object): try: self.session.close() - except: + except Exception: if __debug__: print_exc() @@ -492,7 +492,7 @@ def fixup(data): for commodity in data['lastStarport'].get('commodities') or []: # Check all required numeric fields are present and are numeric - # Catches "demandBracket": "" for some phantom commodites in + # Catches "demandBracket": "" for some phantom commodites in # ED 1.3 - https://github.com/Marginal/EDMarketConnector/issues/2 # # But also see https://github.com/Marginal/EDMarketConnector/issues/32 From 5c4c9fd3a79ab8a4d270019b416dce0811ca678b Mon Sep 17 00:00:00 2001 From: A_D Date: Tue, 7 Jul 2020 18:30:29 +0200 Subject: [PATCH 07/14] Added error messages to exceptions where possible Adds messages to exceptions where they may be useful and I could figure out what they should be --- companion.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/companion.py b/companion.py index edf6b78d..4c76dd49 100644 --- a/companion.py +++ b/companion.py @@ -227,12 +227,12 @@ class Auth(object): # Returns access token if successful, otherwise raises CredentialsError if '?' not in payload: print('Auth\tMalformed response {!r}'.format(payload)) - raise CredentialsError() # Not well formed + raise CredentialsError('malformed payload') # Not well formed data = urllib.parse.parse_qs(payload[payload.index('?') + 1:]) if not self.state or not data.get('state') or data['state'][0] != self.state: print('Auth\tUnexpected response {!r}'.format(payload)) - raise CredentialsError() # Unexpected reply + raise CredentialsError('Unexpected response from authorization {!r}'.format(payload)) # Unexpected reply if not data.get('code'): print('Auth\tNegative response {!r}'.format(payload)) @@ -314,12 +314,12 @@ class Session(object): def login(self, cmdr=None, is_beta=None): # Returns True if login succeeded, False if re-authorization initiated. if not CLIENT_ID: - raise CredentialsError() + raise CredentialsError('cannot login without a valid client ID') if not cmdr or is_beta is None: # Use existing credentials if not self.credentials: - raise CredentialsError() # Shouldn't happen + raise CredentialsError('Missing credentials') # Shouldn't happen elif self.state == Session.STATE_OK: return True # already logged in @@ -352,7 +352,8 @@ class Session(object): # Callback from protocol handler def auth_callback(self): if self.state != Session.STATE_AUTH: - raise CredentialsError() # Shouldn't be getting a callback + # Shouldn't be getting a callback + raise CredentialsError('Got an auth callback while not doing auth') try: self.start(self.auth.authorize(protocolhandler.lastpayload)) @@ -375,7 +376,7 @@ class Session(object): return self.query(endpoint) elif self.state == Session.STATE_AUTH: - raise CredentialsError() + raise CredentialsError('cannot make a query when unauthorized') try: r = self.session.get(self.server + endpoint, timeout=timeout) @@ -397,7 +398,7 @@ class Session(object): elif 500 <= r.status_code < 600: # Server error. Typically 500 "Internal Server Error" if server is down self.dump(r) - raise ServerError() + raise ServerError('Received error {} from server'.format(r.status_code)) try: r.raise_for_status() # Typically 403 "Forbidden" on token expiry From e7399230833bcf68d44af1bcd47dcc6d21aa4f80 Mon Sep 17 00:00:00 2001 From: A_D Date: Tue, 7 Jul 2020 18:37:05 +0200 Subject: [PATCH 08/14] Moved User Agent to constant This is one of those things that really should be a constant anyway --- companion.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/companion.py b/companion.py index 4c76dd49..501b56bd 100644 --- a/companion.py +++ b/companion.py @@ -35,7 +35,9 @@ auth_timeout = 30 # timeout for initial auth CLIENT_ID = os.getenv('CLIENT_ID') or 'fb88d428-9110-475f-a3d2-dc151c2b9c7a' SERVER_AUTH = 'https://auth.frontierstore.net' URL_AUTH = '/auth' -URL_TOKEN = '/token' +URL_TOKEN = '/token' + +USER_AGENT = 'EDCD-{}-{}'.format(appname, appversion) SERVER_LIVE = 'https://companion.orerve.net' SERVER_BETA = 'https://pts-companion.orerve.net' @@ -167,7 +169,7 @@ class Auth(object): def __init__(self, cmdr): self.cmdr = cmdr self.session = requests.Session() - self.session.headers['User-Agent'] = 'EDCD-{}-{}'.format(appname, appversion) + self.session.headers['User-Agent'] = USER_AGENT self.verifier = self.state = None def refresh(self): @@ -367,7 +369,7 @@ class Session(object): def start(self, access_token): self.session = requests.Session() self.session.headers['Authorization'] = 'Bearer {}'.format(access_token) - self.session.headers['User-Agent'] = 'EDCD-{appname}-{version}'.format(appname=appname, version=appversion) + self.session.headers['User-Agent'] = USER_AGENT self.state = Session.STATE_OK def query(self, endpoint): From 4aa8e2ca0e3fbcb80cf07b308d7f8cde843a42e2 Mon Sep 17 00:00:00 2001 From: A_D Date: Tue, 7 Jul 2020 18:42:16 +0200 Subject: [PATCH 09/14] Added variable for re-used accesses The last starport ID and name was repeatedly accessed out of a dict, making lines longer and harder to parse --- companion.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/companion.py b/companion.py index 501b56bd..3909199b 100644 --- a/companion.py +++ b/companion.py @@ -439,10 +439,12 @@ class Session(object): if data['commander'].get('docked'): services = data['lastStarport'].get('services', {}) - if services.get('commodities'): + last_starport_name = data['lastStarport']['name'] + last_starport_id = int(data['lastStarport']['id']) + if services.get('commodities'): marketdata = self.query(URL_MARKET) - if (data['lastStarport']['name'] != marketdata['name'] or int(data['lastStarport']['id']) != int(marketdata['id'])): + if (last_starport_name != marketdata['name'] or last_starport_id != int(marketdata['id'])): raise ServerLagging() else: @@ -450,7 +452,7 @@ class Session(object): if services.get('outfitting') or services.get('shipyard'): shipdata = self.query(URL_SHIPYARD) - if (data['lastStarport']['name'] != shipdata['name'] or int(data['lastStarport']['id']) != int(shipdata['id'])): + if (last_starport_name != shipdata['name'] or last_starport_id != int(shipdata['id'])): raise ServerLagging() else: From 0b6ef97f410d9bfef86c8657f81bfb9cef4a5adb Mon Sep 17 00:00:00 2001 From: A_D Date: Tue, 7 Jul 2020 18:46:23 +0200 Subject: [PATCH 10/14] Made dict copy clearer While `dict(somedict)` does the same thing as `somedict.copy()`, the .copy is clearer as to intent --- companion.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/companion.py b/companion.py index 3909199b..87f3d351 100644 --- a/companion.py +++ b/companion.py @@ -480,7 +480,6 @@ class Session(object): def dump(self, r): print('cAPI\t' + r.url, r.status_code, r.reason and r.reason or 'None', r.text) - # Returns a shallow copy of the received data suitable for export to older tools # English commodity names and anomalies fixed up def fixup(data): @@ -560,8 +559,8 @@ def fixup(data): commodities.append(new) # return a shallow copy - datacopy = dict(data) - datacopy['lastStarport'] = dict(data['lastStarport']) + datacopy = data.copy() + datacopy['lastStarport'] = data['lastStarport'].copy() datacopy['lastStarport']['commodities'] = commodities return datacopy From dcb9853b68ba83139710625933d09ce4d3f356f2 Mon Sep 17 00:00:00 2001 From: A_D Date: Tue, 7 Jul 2020 21:52:30 +0200 Subject: [PATCH 11/14] Removed python2 specific code This entire branch is py3 only, and even with this code the actual file isn't py2 compatible syntax wise --- companion.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/companion.py b/companion.py index 87f3d351..6321e8ef 100644 --- a/companion.py +++ b/companion.py @@ -308,11 +308,6 @@ class Session(object): self.auth = None self.retrying = False # Avoid infinite loop when successful auth / unsuccessful query - # yuck suppress InsecurePlatformWarning under Python < 2.7.9 which lacks SNI support - if sys.version_info < (2, 7, 9): - from requests.packages import urllib3 - urllib3.disable_warnings() - def login(self, cmdr=None, is_beta=None): # Returns True if login succeeded, False if re-authorization initiated. if not CLIENT_ID: From 2890d33d03d0469e491dc963492f6118987ee9c6 Mon Sep 17 00:00:00 2001 From: A_D Date: Wed, 8 Jul 2020 18:05:43 +0200 Subject: [PATCH 12/14] Address review comments Added a missing comma to the category_map literal Added parens around an index that does math beforehand Clarified HTTP/JSON error CredentialsError --- companion.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/companion.py b/companion.py index 6321e8ef..84f25209 100644 --- a/companion.py +++ b/companion.py @@ -52,7 +52,7 @@ category_map = { 'Narcotics' : 'Legal Drugs', 'Slaves' : 'Slavery', 'Waste ' : 'Waste', - 'NonMarketable' : False # Don't appear in the in-game market so don't report + 'NonMarketable' : False, # Don't appear in the in-game market so don't report } commodity_map = {} @@ -231,7 +231,7 @@ class Auth(object): print('Auth\tMalformed response {!r}'.format(payload)) raise CredentialsError('malformed payload') # Not well formed - data = urllib.parse.parse_qs(payload[payload.index('?') + 1:]) + data = urllib.parse.parse_qs(payload[(payload.index('?') + 1):]) if not self.state or not data.get('state') or data['state'][0] != self.state: print('Auth\tUnexpected response {!r}'.format(payload)) raise CredentialsError('Unexpected response from authorization {!r}'.format(payload)) # Unexpected reply @@ -311,7 +311,7 @@ class Session(object): def login(self, cmdr=None, is_beta=None): # Returns True if login succeeded, False if re-authorization initiated. if not CLIENT_ID: - raise CredentialsError('cannot login without a valid client ID') + raise CredentialsError('cannot login without a valid Client ID') if not cmdr or is_beta is None: # Use existing credentials @@ -418,7 +418,7 @@ class Session(object): else: self.retrying = False - raise CredentialsError('Invalid JSON or HTTP error') from e + raise CredentialsError('HTTP error or invalid JSON') from e self.retrying = False if 'timestamp' not in data: From 09f5dfff2353a02e21b4853c82040858a48ab975 Mon Sep 17 00:00:00 2001 From: A_D Date: Wed, 8 Jul 2020 18:10:28 +0200 Subject: [PATCH 13/14] Replaced assert False with a clearer raise previously this would assert False, which always raises an AssertionError, now we raise a saner ValueError --- companion.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/companion.py b/companion.py index 84f25209..1cd202bb 100644 --- a/companion.py +++ b/companion.py @@ -129,8 +129,7 @@ def listify(thing): return retval else: - assert False, thing # we expect an array or a sparse array - return list(thing) # hope for the best + raise ValueError("expected an array or sparse array") class ServerError(Exception): From 1481c3d13e1842fcb4400c46f33a7e995ace151b Mon Sep 17 00:00:00 2001 From: A_D Date: Wed, 8 Jul 2020 21:24:15 +0200 Subject: [PATCH 14/14] Added todo, removed import Added a TODO on the incase plugins need it line, and removed the unused sys import --- companion.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/companion.py b/companion.py index 1cd202bb..f6496125 100644 --- a/companion.py +++ b/companion.py @@ -4,6 +4,8 @@ from builtins import object import base64 import csv import requests + +# TODO: see https://github.com/EDCD/EDMarketConnector/issues/569 from http.cookiejar import LWPCookieJar # No longer needed but retained in case plugins use it from email.utils import parsedate import hashlib @@ -11,7 +13,6 @@ import numbers import os from os.path import join import random -import sys import time from traceback import print_exc import urllib.parse @@ -129,7 +130,7 @@ def listify(thing): return retval else: - raise ValueError("expected an array or sparse array") + raise ValueError("expected an array or sparse array, got {!r}".format(thing)) class ServerError(Exception):