Skip to content
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

Scorekeeper Refactoring 2/4 #2937

Open
wants to merge 5 commits into
base: staging
Choose a base branch
from

Conversation

vovacha
Copy link

@vovacha vovacha commented May 19, 2024

First part #2934

Current PR changes:

  • Localize behavior by bringing cron job initialization into the Job class.
  • Remove cron/StartCronJobs/, cron/SetupCronJobs/.
  • Remove the JobNames enum.
  • Add enums: JobStatus, JobEvent, JobKey.
  • Clean up job modules.
  • Add new tests.

Comments:

  • There is almost no need for the events now, since CronJob is a part of the Job class. The only event that makes sense is the JobEvent.Progress event. Job chaining is something to check; that could be a reason for having other event handlers.
  • There is no consistency in the logging strategy when working with labels and logging levels.
  • There is a redundancy in the exception handling in the job functions.
  • Not sure if it's the correct behavior. jobFinished event will immediately overwrite jobErrored status here https://github.com/w3f/1k-validators-be/blob/master/packages/common/src/scorekeeper/jobs/cron/SetupCronJob.ts#L63-L73

Impact:

  • Package common
  • Package scorekeeper-status-ui

Potential next steps (to be discussed):

  • Scorekeeper 1. First part of refactoring.
  • Scorekeeper 2. Current PR.
  • Scorekeeper 3. Add DB queries to store job status.
  • Gateway. Update views to retrieve job statuses from the DB.
  • Gateway. Remove Scorekeeper dependency.
  • Core. Update Gateway initialization without Scorekeeper.
  • Scorekeeper 4. Remove status updates via events and eliminate public interfaces related to the Gateway.
  • Gateway. Reduce abstraction layers and code duplication in controllers.
  • Gateway. Refactor dynamic routes (e.g., onlyHealth flag).
  • Deployment Update. Update deployment scripts to run services separately: Scorekeeper, Gateway, and Telemetry.

@vovacha vovacha mentioned this pull request May 19, 2024
10 tasks
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.

1 participant