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 1/4 #2934

Open
wants to merge 3 commits into
base: staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 1 addition & 59 deletions packages/common/src/scorekeeper/RegisterHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,8 @@
*
* @function RegisterHandler
*/
import {
ApiHandler,
ChainData,
Config,
logger,
queries,
ScoreKeeper,
} from "../index";
import { ApiHandler, ChainData, Config, logger, queries } from "../index";
import { scorekeeperLabel } from "./scorekeeper";
import { jobStatusEmitter } from "../Events";
import { Job, JobStatus } from "./jobs/JobsClass";

export const registerAPIHandler = (
handler: ApiHandler,
Expand Down Expand Up @@ -52,52 +43,3 @@ export const registerAPIHandler = (
logger.info(`New Session Event: ${sessionIndex}`, scorekeeperLabel);
});
};

export const registerEventEmitterHandler = (scoreKeeper: ScoreKeeper) => {
logger.info(`Registering event emitter handler`, scorekeeperLabel);
jobStatusEmitter.on("jobStarted", (data) => {
// scoreKeeper.updateJobStarted(data);
});

jobStatusEmitter.on("jobRunning", (data) => {
// scoreKeeper.updateJobRunning(data);
});

jobStatusEmitter.on("jobFinished", (data) => {
// scoreKeeper.updateJobFinished(data);
});

jobStatusEmitter.on("jobErrored", (data) => {
// scoreKeeper.updateJobErrored(data);
});

jobStatusEmitter.on("jobProgress", (data) => {
// scoreKeeper.updateJobProgress(data);
});
};

export const registerJobStatusEventEmitterHandler = (job: Job) => {
logger.info(
`Registering event emitter handler for job: ${job.getName()}`,
scorekeeperLabel,
);
jobStatusEmitter.on("jobStarted", (data: JobStatus) => {
job.updateJobStatus(data);
});

jobStatusEmitter.on("jobRunning", (data: JobStatus) => {
job.updateJobStatus(data);
});

jobStatusEmitter.on("jobFinished", (data: JobStatus) => {
job.updateJobStatus(data);
});

jobStatusEmitter.on("jobErrored", (data: JobStatus) => {
job.updateJobStatus(data);
});

jobStatusEmitter.on("jobProgress", (data: JobStatus) => {
job.updateJobStatus(data);
});
};
77 changes: 77 additions & 0 deletions packages/common/src/scorekeeper/jobs/Job.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { startJob } from "./cron/StartCronJobs";
import logger from "../../logger";
import { jobStatusEmitter } from "../../Events";
import { JobStatus, JobConfig, JobRunnerMetadata } from "./types";

export class Job {
protected status: JobStatus;
protected jobConfig: JobConfig;
protected jobRunnerMetadata: JobRunnerMetadata;

// TODO: remove this dependency injection
private startJobFunction: (
metadata: JobRunnerMetadata,
jobConfig: JobConfig,
) => Promise<void>;

// TODO: remove events and use db to handle the state
// then we can decouple scorekeeper and gateway
static events: string[] = [
"jobStarted",
"jobRunning",
"jobFinished",
"jobErrored",
"jobProgress",
];

constructor(jobConfig: JobConfig, jobRunnerMetadata: JobRunnerMetadata) {
this.status = {
name: jobConfig.name,
updated: Date.now(),
status: "Not Running",
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be of type StatusName, right ? can we avoid to have the string Not Running hardcoded ?

Copy link
Author

Choose a reason for hiding this comment

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

I partially answered in the previous comment. Please let me know if you want me to include this in the current PR.
For the renaming issue, there is a dependency at least on scorekeeper-status-ui.
For the hardcoding issue, it's easier, but the plan was to tackle it together.

};
this.jobConfig = jobConfig;
this.jobRunnerMetadata = jobRunnerMetadata;
this.startJobFunction = startJob;
}

private log(message: string) {
logger.info(message, { label: "Job" });
}

// TODO: remove events and use db to handle the state
// then we can decouple scorekeeper and gateway
private registerEventHandlers() {
this.log(`Registering event handlers for ${this.jobConfig.name}`);
Job.events.forEach((event) => {
jobStatusEmitter.on(event, (data: JobStatus) => {
this.updateJobStatus(data);
});
});
}

public async run(): Promise<void> {
this.registerEventHandlers();
this.log(`Starting ${this.getName()}`);
await this.startJobFunction(this.jobRunnerMetadata, this.jobConfig);
}

// TODO: remove this public interface after decoupling with the Gateway
public getName(): string {
return this.jobConfig.name;
}

public updateJobStatus(status: JobStatus) {
if (status.name == this.getName()) {
this.status = { ...this.status, ...status };
}
}

public getStatusAsJson(): string {
return JSON.stringify(this.status);
}

public getStatus(): JobStatus {
return this.status;
}
}
Loading