Skip to content

Commit

Permalink
Improve 404 messages from the API when a plugin is not found. #1458
Browse files Browse the repository at this point in the history
  • Loading branch information
mfeit-internet2 committed Jul 15, 2024
1 parent 025a94e commit 77c636c
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ def archivers():
def archivers_name(name):
return json_query("SELECT json FROM archiver"
" WHERE available AND name = %s",
[name], single=True)
[name], single=True,
not_found_message=f'No archiver "{name}" is available.'
)


# Archiver spec validation
Expand All @@ -42,7 +44,7 @@ def archivers_name_data_is_valid(name):
exists = cursor.fetchone()[0]
cursor.close()
if not exists:
return not_found()
return not_found(f'No archiver "{name}" is available.')

data = request.args.get('data')
if data is None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ def contexts():
def contexts_name(name):
return json_query("SELECT json FROM context"
" WHERE available AND name = %s",
[name], single=True)
[name], single=True,
not_found_message=f'No context "{name}" is available.'
)


# Context spec validation
Expand All @@ -42,11 +44,12 @@ def contexts_name_data_is_valid(name):
exists = cursor.fetchone()[0]
cursor.close()
if not exists:
return not_found()
return not_found(f'No context "{name}" is available.')


data = request.args.get('data')
if data is None:
return bad_request("No archive data provided")
return bad_request("No context data provided")

try:
returncode, stdout, stderr = pscheduler.plugin_invoke(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ def json_dump(dump):
)


def json_query_simple(query, query_args=[], empty_ok=False, key=None):
def json_query_simple(query, query_args=[], empty_ok=False, key=None,
not_found_message=None):
"""Do a SQL query that selects one column and dump those values as
a JSON array"""

Expand All @@ -32,7 +33,7 @@ def json_query_simple(query, query_args=[], empty_ok=False, key=None):
# This is safe to return unsanitized
return ok_json([], sanitize=False)
else:
return not_found()
return not_found(not_found_message)

result = []
for row in cursor:
Expand All @@ -42,7 +43,8 @@ def json_query_simple(query, query_args=[], empty_ok=False, key=None):



def json_query(query, query_args=[], name='name', single=False, key=None):
def json_query(query, query_args=[], name='name', single=False, key=None,
not_found_message=None):
"""Do a SQL query that selects one column containing JSON and dump
the results, honoring the 'expanded' and 'pretty' arguments. If
the 'single' argument is True, the first-returned row will be
Expand All @@ -55,7 +57,7 @@ def json_query(query, query_args=[], name='name', single=False, key=None):

if single and cursor.rowcount == 0:
cursor.close()
return not_found()
return not_found(not_found_message)
result = []
for row in cursor:
this = base_url(None if single else row[0][name])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@

# Responses

def ok(message="OK", mimetype=None):
def ok(message=None, mimetype=None):
if message is None:
message = 'OK'
log.debug("Response 200: %s", message)
return Response(message + '\n',
status=200,
Expand All @@ -41,15 +43,24 @@ def ok_json_sanitize_checked(data, required_key=None):
return forbidden()
return ok_json(data, sanitize=sanitize)

def bad_request(message="Bad request"):
def bad_request(message=None):
if message is None:
message = 'Bad request'

log.debug("Response 400: %s", message)
return Response(message + '\n', status=400, mimetype="text/plain")

def forbidden(message="Forbidden."):
def forbidden(message=None):
if message is None:
message = 'Forbidden'

log.debug("Response 403: %s", message)
return Response(message + "\n", status=403, mimetype="text/plain")

def not_found(message="Resource Not found.", mimetype="text/plain"):
def not_found(message=None, mimetype="text/plain"):
if message is None:
message = 'Resource not found.'

log.debug("Response 404: %s", message)
return Response(message + "\n", status=404, mimetype="text/plain")

Expand All @@ -58,8 +69,10 @@ def not_allowed():
return Response("%s not allowed on this resource\n" % (request.method),
status=405, mimetype="text/plain")

def conflict(message="Request would create a conflict."):
log.debug("Response 409: Conflict")
def conflict(message=None):
if message is None:
message = 'Request would create a conflict.'
log.debug(f'Response 409: {message}')
return Response(message + '\n', status=409, mimetype="text/plain")

def no_can_do(message=None):
Expand All @@ -71,12 +84,16 @@ def no_can_do(message=None):
+ '\n',
status=422, mimetype="text/plain")

def error(message="Unknown internal error"):
def error(message=None):
if message is None:
message = 'Unknown internal error'
log.debug("Response 500: %s", message)
log.error("Internal error %s %s %s: %s", request.remote_addr, request.method, request.base_url, message)
return Response(message + '\n', status=500, mimetype="text/plain")

def not_implemented(message="Not implemented."):
def not_implemented(message=None):
if message is None:
message = 'Not implemented.'
log.debug("Response 501: %s", message)
log.warning("Not implemented %s %s %s: %s", request.remote_addr, request.method, request.base_url, message)
return Response(message + "\n", status=501, mimetype="text/plain")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ def tests():
def tests_name(name):
return json_query("SELECT json FROM test"
" WHERE available AND name = %s",
[name], single=True)
[name], single=True,
not_found_message=f'No test "{name}" is available.'
)


# Derive a spec from command line arguments in 'arg'
Expand All @@ -42,7 +44,7 @@ def tests_name_spec(name):
exists = cursor.fetchone()[0]
cursor.close()
if not exists:
return not_found()
return not_found(f'No test "{name}" is available.')

try:
args = arg_json('args')
Expand Down Expand Up @@ -76,7 +78,7 @@ def tests_name_spec_is_valid(name):
exists = cursor.fetchone()[0]
cursor.close()
if not exists:
return not_found()
return not_found(f'No test "{name}" is available.')

spec = request.args.get('spec')
if spec is None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,6 @@ def tools():
def tools_name(name):
return json_query("SELECT json FROM tool"
" WHERE available AND name = %s",
[name], single=True)
[name], single=True,
not_found_message=f'No tool "{name}" is available.'
)

0 comments on commit 77c636c

Please sign in to comment.