-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
#4076 - Queue Monitoring - Schedulers Refactor (SIN validation and loan balance) #4118
#4076 - Queue Monitoring - Schedulers Refactor (SIN validation and loan balance) #4118
Conversation
andrewsignori-aot
commented
Dec 16, 2024
•
edited
Loading
edited
- Refactored SIN validation-related schedulers and student loan balance import.
- Adjusted E2E tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Nice refactor!
...egration/sin-validation-integration/sin-validation-process-response-integration.scheduler.ts
Outdated
Show resolved
Hide resolved
...c-integration/student-loan-balances/student-loan-balances-part-time-integration.scheduler.ts
Outdated
Show resolved
Hide resolved
const results = await this.sinValidationProcessingService.processResponses( | ||
auditUser.id, | ||
); | ||
await summary.info("ESDC SIN validation response files processed."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we have a processSummary.info
with similar context ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we talked, removing the friendly messages from the processor since the BaseQueue
has some similar to indicate the start and end of the processing.
...c-integration/student-loan-balances/student-loan-balances-part-time-integration.scheduler.ts
Show resolved
Hide resolved
...c-integration/student-loan-balances/student-loan-balances-part-time-integration.scheduler.ts
Outdated
Show resolved
Hide resolved
Good job @andrewsignori-aot. Added a few minor comments. |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes. Looks good 👍