HTTP response code handling from ADP workforce now API and parse resulting JSON

Question

Is there a way to improve this method that interacts with ADP's workforce-now API?

• 200: capture and parse JSON
• 204: return a None
• 400 and 403: return specific text from JSON
• others: throw an exception

 def get_worker_by_ssn(access_token, ssn):
 
 import json
 
 # try:
 
 headers={
 "Accept": "application/json;masked=false",
 "Content-Type": "application/json",
 "Authorization": f"Bearer {access_token}"
 }
 
 data = {
 "events": [
 {
 "serviceCategoryCode": {"codeValue": "hr"},
 "eventNameCode": {"codeValue": "worker.read"},
 "data": {
 "transform": {
 "queryParameter": f"$filter=person/governmentIDs[0]/idValue eq '{ssn}' and person/governmentIDs[0]/nameCode eq 'SSN'"
 }
 },
 }
 ]
 }
 
 certificate_path = "./certificate.pem"
 private_key_path = "./private_key.key"
 
 response = requests.request(
 'POST',
 'https://api.adp.com/events/hr/v1/worker.read',
 headers=headers,
 data=json.dumps(data) if data else None,
 cert=(certificate_path, private_key_path),
 )
 
 print("response.status_code", response.status_code)
 
 if response.status_code == 200:
 return response.json()
 
 elif response.status_code == 204:
 return None
 
 # Bad Request [400], Forbidden [403]
 elif response.status_code in [400,403]:
 data = response.json()
 
 error_message = data["confirmMessage"]["resourceMessages"][0]['processMessages'][0]["userMessage"]["messageTxt"] if data else None
 # print('error_message',error_message)
 raise ValueError(error_message)
 
 else:
 response.raise_for_status()
 
 # except requests.exceptions.RequestException as err:
 # print(f"Error: {err}")
 # raise(err)

Answer 1

Not much, but here's my suggested improvement:

import json
def get_worker_by_ssn(access_token, ssn):
 # try:
 headers = {
 "Accept": "application/json;masked=false",
 "Content-Type": "application/json",
 "Authorization": f"Bearer {access_token}"
 }
 data = {
 "events": [
 {
 "serviceCategoryCode": {"codeValue": "hr"},
 "eventNameCode": {"codeValue": "worker.read"},
 "data": {
 "transform": {
 "queryParameter": f"$filter=person/governmentIDs[0]/idValue eq '{ssn}' and person/governmentIDs[0]/nameCode eq 'SSN'"
 }
 },
 }
 ]
 }
 certificate_path = "./certificate.pem"
 private_key_path = "./private_key.key"
 response = requests.request(
 'POST',
 'https://api.adp.com/events/hr/v1/worker.read',
 headers=headers,
 data=json.dumps(data),
 cert=(certificate_path, private_key_path),
 )
 print("response.status_code", response.status_code)
 if response.status_code == 200:
 return response.json()
 elif response.status_code == 204:
 return 
 # Bad Request [400], Forbidden [403]
 elif response.status_code in [400, 403]:
 data = response.json()
 error_message = data["confirmMessage"]["resourceMessages"][0]['processMessages'][0]["userMessage"][
 "messageTxt"]
 # print('error_message',error_message)
 raise ValueError(error_message)
 else:
 response.raise_for_status()
 # except requests.exceptions.RequestException as err:
 # print(f"Error: {err}")
 # raise(err)

Primary changes:

• Import json outside the function, rather than inside. You probably don't want the runtime and memory addition due to json being imported every time you call the function.
• As json.dumps(data) and data["confirmMessage"]["resourceMessages"][0]['processMessages'][0]["userMessage"]["messageTxt"] will be None if they are False anyway, there's no need for if data else None.
• No need for return None. Just write return instead, as python will automatically return None if you return from a function but don't provide a return value.

• J_H: Arrrgh, noo! In a function that e.g. promises the caller ... -> dict | None:, we should explicitly return None, so the Gentle Reader can quickly visually verify that we're keeping our promise. OTOH with def foo() -> None: where we're evaluating for side effects, we should leave off the final return so there's an implicit return None, and if we do a conditional early return it should be just return. This lets the Gentle Reader focus on control flow. // Certainly imports belong up top. But 2nd time we import isn't all that expensive, as it enjoys a cache hit.
• Luke L: @J_H I must respectfully disagree with you. Type checking, Gentle Reader, linters, ...etc. are not important at all. They just "help" you debug and find errors, but it seems like these days they create issues for people to solve; that is, I observed that many people went into a lot of effort just to satisfy these checker/linters. That, I concluded, was completely off the point. It defies the original meaning and motive of the people who wrote these software. Plus, OP never asked about anything related to Gentle Reader. So I answered the question correctly.
• J_H: The maintenance engineer we recruit to the team in a few months is the Gentle Reader. Software engineering is always about maintainability in addition to correctness, so that's a focus of the Code Review site. The biggest control flow "whoops!" I typically see from a newly hired maintenance engineer is confusion between returning None vs. raising an exception. We strive to teach good habits here, so currently correct code will continue to be correct even after a new hire has made edits to it.
• Luke L: @J_H I don't get why you are rambling on about Gentle Reader. Just like I said, OP never mentioned Gentle Reader. I've shouldn't have been thinking and writing about something OP never asked for. Also, saying what your company does doesn't mean every company uses this. It's probably just company preference.
• Luke L: @J_H Maintainability of code is absolutely important, but most experienced Python programmers can immediately recognize that return and return None are the same, and I believe that many would agree with me that the former is actually better. Explicit is good, but too over and you get messy code. Again, all experienced python programmers know that if a return value isn't explicitly given, then it's None.

Answer 2

Here are some suggestions:

• Import json once, not on every call.
• If the input is user-provided, begin by validating it.
• Do the extensive error indexing in steps.

data = response.json()
if not data:
 raise ValueError(None)
 
resource_messages = data["confirmMessage"]["resourceMessages"]
process_messages = resource_messages[0]['processMessages']
error_message = process_messages[0]["userMessage"]["messageTxt"]
raise ValueError(error_message)

Hope this helps!

• craig: Why do the error indexing in steps? Any improvements to the status-code-testing logic?
• Justin Chang: I think it is more readable in steps. Also, you could do validation on the response as you go.