Skip to content

Apply changes from kates doc#170

Merged
uzairharoon20 merged 5 commits intomainfrom
apply-changes-from-kates-doc
Aug 11, 2025
Merged

Apply changes from kates doc#170
uzairharoon20 merged 5 commits intomainfrom
apply-changes-from-kates-doc

Conversation

@uzairharoon20
Copy link
Contributor

Lambda requested changes.docx

Applied all P1 changes in the attached word document

logging.error("No recipients for batch ID: %s", batch_id)
except oracledb.Error as e:
logging.error("Error fetching recipients: %s", e)
recipients_results = oracle_database.get_recipients(batch_id) # []
Copy link
Contributor

Choose a reason for hiding this comment

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

What's that comment for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, this was my mistake. I think I put that there during development as a reminder to myself that this could return an empty list. This was just to stop myself from switching files back and forth to check. I've removed it now

batch_id = generate_batch_id()
routing_plan_id = get_routing_plan_id(batch_id)
recipients = None
batch_id = generate_batch_id()
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of interest, why are we removing the try blocks and not logging the Oracle error here any more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So this was one of the suggestions from the document. It stated that since we already catch the Oracle Error in the function in the oracle db py file, then there's no need to re raise it, catch it and log it again.

Instead it was suggested that returning None and logging that nothing was returned, from the calling function was better.

Also, it keeps Oracle specific code, e.g. catching an oracle error, in one oracle implementation file

response_codes = []
for data in json_data.get('data', [{}]):
def record_message_statuses(json_data: dict) -> dict[str, int]:
response_counts = {"0": 0, "non_zero": 0}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
response_counts = {"0": 0, "non_zero": 0}
response_counts = {"zero": 0, "non_zero": 0}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied this improvement

response_code = record_message_status(data)
response_codes.append(response_code)
if response_code == 0:
response_counts["0"] += 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
response_counts["0"] += 1
response_counts["zero"] += 1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Applied this improvement

@dnimmo
Copy link
Contributor

dnimmo commented Aug 11, 2025

Looks like maybe your IDE has done some auto-formatting - you'll need to lint this repo I'm afraid, it's currently failing the linting job.

@uzairharoon20 uzairharoon20 added this pull request to the merge queue Aug 11, 2025
Merged via the queue into main with commit 1706bb7 Aug 11, 2025
10 checks passed
@uzairharoon20 uzairharoon20 deleted the apply-changes-from-kates-doc branch August 11, 2025 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants