Logging, Exception Management Assignment #30
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
naman-TI
reviewed
Dec 14, 2022
| @@ -15,6 +15,7 @@ | |||
|
|
|||
There was a problem hiding this comment.
info logs missing throughout the file
| @@ -12,4 +12,6 @@ | |||
|
|
|||
|
|
|||
| def get_enriched_lead_json(adf_json: dict) -> dict: | |||
| if adf_json == {}: | |||
| logging.warning("Empty JSON") | |||
| pass | |||
There was a problem hiding this comment.
raise KeyError for missing 'adf' key and check if adj_json is valid to raise ValueError
fast_api_als/routers/three_pl.py
Outdated
| @@ -17,10 +17,16 @@ async def reset_authkey(request: Request, token: str = Depends(get_token)): | |||
| body = json.loads(body) | |||
| provider, role = get_user_role(token) | |||
| if role != "ADMIN" and (role != "3PL"): | |||
| logging.info("Role is neither Admin nor 3PL") | |||
There was a problem hiding this comment.
raise http exception with proper status code, add info logs where necessary
| @@ -47,6 +48,7 @@ async def submit(file: Request, apikey: APIKey = Depends(get_api_key)): | |||
|
|
|||
| # check if xml was not parsable, if not return | |||
| if not obj: | |||
| logging.error("XML could not be parsed") | |||
| provider = db_helper_session.get_api_key_author(apikey) | |||
| obj = { | |||
| @@ -95,11 +99,13 @@ async def submit(file: Request, apikey: APIKey = Depends(get_api_key)): | |||
| for future in as_completed(futures): | |||
| result = future.result() | |||
| if result.get('Duplicate_Api_Call', {}).get('status', False): | |||
| logging.warning("Duplicate API call") | |||
There was a problem hiding this comment.
log info for the time taken for request completion
| @@ -199,6 +211,7 @@ async def submit(file: Request, apikey: APIKey = Depends(get_api_key)): | |||
| res = sqs_helper_session.send_message(message) | |||
|
|
|||
| else: | |||
| logging.info("Rejected current response") | |||
| message = { | |||
| 'put_file': { | |||
| 'item': item, | |||
| # throw proper HTTPException | ||
| pass | ||
| logging.error("Lead conversion not updated") | ||
| raise HTTPException(status_code = 406, detail = "Lead conversion not updated") |
There was a problem hiding this comment.
*already updated, status_code should be 400
fast_api_als/database/db_helper.py
Outdated
| @@ -355,3 +355,9 @@ def verify_response(response_code): | |||
|
|
|||
| session = get_boto3_session() | |||
| db_helper_session = DBHelper(session) | |||
|
|
|||
|
|
|||
| def log_res_data(obj : DBHelper): | |||
|
Kindly check the current changes and suggest any if needed |
naman-TI
reviewed
Dec 15, 2022
| logging.info("Role is neither Admin nor 3PL") | ||
| pass | ||
| logging.error("Role required to be admin or 3PL") | ||
| raise HTTPException(status_code = 400, detail = "Role required to be admin or 3PL") |
There was a problem hiding this comment.
400 status code is for a Bad request.
There was a problem hiding this comment.
decide whether this should be 401 or 403.
|
This is Done |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.