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

feat: implement naive cache solution #17

Merged
merged 14 commits into from
Jan 3, 2022
Merged

Conversation

AnguIar
Copy link
Collaborator

@AnguIar AnguIar commented Dec 27, 2021

Question Answer
New feature

@lgtm-com
Copy link

lgtm-com bot commented Dec 29, 2021

This pull request introduces 1 alert when merging 355d6cf into f9d5dc0 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Dec 29, 2021

This pull request introduces 1 alert when merging 14d69da into f9d5dc0 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

@lgtm-com
Copy link

lgtm-com bot commented Dec 29, 2021

This pull request introduces 1 alert when merging bacc684 into f9d5dc0 - view on LGTM.com

new alerts:

  • 1 for Unused variable, import, function or class

Copy link
Contributor

@asafMasa asafMasa left a comment

Choose a reason for hiding this comment

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

👍 good job

src/clients/jobManagerClient.ts Outdated Show resolved Hide resolved
import { ICreateJobBody, ICreateJobResponse, IJobCreationResponse, IWorkerInput } from '../common/interfaces';
import booleanEqual from '@turf/boolean-equal';
import bboxPolygon from '@turf/bbox-polygon';
import { ICreateJobBody, IJobResponse, IUpdateJobBody, OperationStatus } from '@map-colonies/mc-priority-queue';
Copy link
Contributor

Choose a reason for hiding this comment

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

remove old interfaces from /common/interfaces

@@ -19,26 +28,31 @@ export class JobManagerClient extends HttpClient {
this.tilesTaskType = config.get<string>('workerTypes.tiles.taskType');
}

public async createJob(data: IWorkerInput, layer: LayerMetadata): Promise<IJobCreationResponse> {
public async updateJob(jobId: string, payload: IUpdateJobBody<IJobParameters>): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

as you have inserted the "mc-priority-queue" package use the built in "jobManagerClient"
use https://github.com/MapColonies/mc-priority-queue/blob/master/src/jobManagerClient.ts#L137

await this.put(updateJobUrl, payload);
}

public async createJob(data: IWorkerInput): Promise<ICreateJobResponse> {
Copy link
Contributor

Choose a reason for hiding this comment

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

as you have inserted the "mc-priority-queue" package use the built in "jobManagerClient"
use https://github.com/MapColonies/mc-priority-queue/blob/master/src/jobManagerClient.ts#L124

return undefined;
}

private async getJobs(queryParams: IFindJob): Promise<JobResponse[] | undefined> {
Copy link
Contributor

Choose a reason for hiding this comment

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

option a: add "/jobs Get" method to pc-priority-que "JobManagerClient" and use it
option b: inherit the "JobManagerClient" and add this method localy

src/createPackage/models/createPackageManager.ts Outdated Show resolved Hide resolved
src/createPackage/models/createPackageManager.ts Outdated Show resolved Hide resolved
}
}

private async checkForProcessing(jobParams: JobDuplicationParams, addedCallbackUrls: string[]): Promise<ICreateJobResponse | undefined> {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to check for inProgress?

@@ -14,7 +14,7 @@ interface IServerConfig {
port: string;
}

const serverConfig = get<IServerConfig>('server');
const serverConfig = config.get<IServerConfig>('server');
Copy link
Contributor

Choose a reason for hiding this comment

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

add p to boiler plate?

@@ -1,88 +1,285 @@
/* eslint-disable */
import { LayerMetadata } from '@map-colonies/mc-model-types';
import { IWorkerInput } from '../../src/common/interfaces';
import { IJobResponse, OperationStatus } from '@map-colonies/mc-priority-queue';
Copy link
Contributor

Choose a reason for hiding this comment

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

fix prettier

Copy link
Contributor

@asafMasa asafMasa left a comment

Choose a reason for hiding this comment

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

👍

@@ -79,28 +81,39 @@ export class CreatePackageManager {
return processingExists;
}

// For race condition
Copy link
Contributor

Choose a reason for hiding this comment

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

race-condition logic need to change according to the design

@@ -1,9 +1,10 @@
import jsLogger from '@map-colonies/js-logger';
import { OperationStatus } from '@map-colonies/mc-priority-queue';
import { JobManagerClient } from '../../../src/clients/jobManagerClient';
import { JobManagerWrapper } from '../../../src/clients/jobManagerWrapper';
import { IJobParameters, ITaskParameters } from '../../../src/common/interfaces';
Copy link
Contributor

Choose a reason for hiding this comment

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

fix lint errors

@AnguIar AnguIar merged commit 8253e9d into master Jan 3, 2022
@AnguIar AnguIar deleted the naive_cache_implementation branch January 3, 2022 15:02
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