Skip to content

feat: push expired tiles into job queue and major refactoring#3

Merged
melancholiai merged 9 commits intomasterfrom
melancholiai/issue2
May 3, 2022
Merged

feat: push expired tiles into job queue and major refactoring#3
melancholiai merged 9 commits intomasterfrom
melancholiai/issue2

Conversation

@melancholiai
Copy link
Collaborator

Question Answer
Bug fix
New feature
Breaking change
Deprecations
Documentation
Tests added
Chore

Closes #2

@melancholiai melancholiai requested a review from CptSchnitz March 10, 2022 16:11
@@ -0,0 +1,74 @@
import { Logger } from '@map-colonies/js-logger';
Copy link
Contributor

Choose a reason for hiding this comment

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

file name should be lowercase

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

README.md Outdated

The output of the append command of `osm2pgsql` is an expire list of tiles in requested zoom levels.

The expired tiles list can be uploaded to a source of your choice, `s3` or `queue`.
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. you dont upload to a queue, you push
  2. it should be mentioned that the queue is pgboss
  3. we should mention that its also possible to use to both.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

src/index.ts Outdated
Comment on lines +11 to +14
interface IError {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
error: (...data: any[]) => void;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

better name, and move to common

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

Comment on lines +33 to +34
const httpClientConfig = config.get<object>('httpClient');
const httpClient = httpClientFactory(httpClientConfig);
Copy link
Contributor

Choose a reason for hiding this comment

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

just use axios, there's no need for the factory as its not generic or anything.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

Comment on lines +43 to +55
{ token: ShutdownHandler, provider: { useValue: shutdownHandler } },
{ token: CLI_BUILDER, provider: { useFactory: cliBuilderFactory } },
{ token: CREATE_COMMAND_FACTORY, provider: { useFactory: createCommandFactory } },
{ token: CREATE_MANAGER_FACTORY, provider: { useFactory: createManagerFactory } },
{ token: APPEND_COMMAND_FACTORY, provider: { useFactory: appendCommandFactory } },
{ token: APPEND_MANAGER_FACTORY, provider: { useFactory: appendManagerFactory } },
{ token: SERVICES.CONFIG, provider: { useValue: config } },
{ token: SERVICES.LOGGER, provider: { useValue: logger } },
{ token: SERVICES.TRACER, provider: { useValue: tracer } },
{ token: SERVICES.HTTP_CLIENT, provider: { useValue: httpClient } },
{ token: SERVICES.CONFIG_STORE, provider: { useValue: configStore } },
{ token: EXIT_CODE, provider: { useValue: ExitCodes.SUCCESS } },
];
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this can be ordered better. config/logger should be first.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

helm/README.md Outdated

**postgres:**

the postgres for osm2pgsql
Copy link
Contributor

Choose a reason for hiding this comment

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

i think better phrasing is needed.
something like this?
the postgres database target for osm2pgsql?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

@melancholiai melancholiai marked this pull request as ready for review March 14, 2022 15:23
.gitignore Outdated
jest_html_reporters.html
reports

local.json No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

mb local*.json ?
and new line is missing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

helm/README.md Outdated
*append*
- `cli.append.replicationUrl` - the source of replication
- `cli.append.limit.value` - should limit the amount of appends in a single run
- `cli.append.limit.enabled` - should limit the amount of appends in a single run
Copy link
Contributor

Choose a reason for hiding this comment

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

should? use a more decisive wording.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

package.json Outdated
"@opentelemetry/api": "1.0.1",
"@opentelemetry/api-metrics": "0.23.0",
"@opentelemetry/instrumentation-http": "0.23.0",
"@types/mapbox__sphericalmercator": "^1.1.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

move to dev dep

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

Comment on lines +1 to +71
import fsPromises from 'fs/promises';
import { existsSync } from 'fs';
import { Arguments } from 'yargs';
import { isWebUri } from 'valid-url';
import { AppendArguments, QueueSettings } from '../commands/append/interfaces';
import { validateBySchema, ValidationResponse } from '../../validation/validator';
import { LIMIT_SCHEMA, Limit, AppendEntity, QUEUE_SETTINGS_SCHEMA, APPEND_CONFIG_SCHEMA } from '../../validation/schemas';
import { NOT_FOUND_INDEX } from '../../common/constants';
import { CreateArguments } from '../commands/create/createFactory';
import { DumpSourceType } from '../commands/create/constants';

type InvalidHandler<T> = (validationResponse: ValidationResponse<T>) => void;
type CheckFunc<T> = (args: Arguments<T>) => Promise<boolean> | boolean;

export const limitCheck = (invalidHandler: InvalidHandler<unknown>): CheckFunc<AppendArguments> => {
const check: CheckFunc<AppendArguments> = (args) => {
const { limit } = args;
const validationResponse = validateBySchema<Limit>({ limit }, LIMIT_SCHEMA);
invalidHandler(validationResponse);
return true;
};
return check;
};

export const configCheck = (invalidHandler: InvalidHandler<unknown>): CheckFunc<AppendArguments> => {
const check: CheckFunc<AppendArguments> = async (args) => {
const { config } = args;
const configContent = await fsPromises.readFile(config, 'utf-8');
const configContentAsJson: unknown = JSON.parse(configContent);
const validationResponse = validateBySchema<AppendEntity[]>(configContentAsJson, APPEND_CONFIG_SCHEMA);
invalidHandler(validationResponse);
return true;
};
return check;
};

export const uploadTargetsCheck = (invalidHandler: InvalidHandler<unknown>): CheckFunc<AppendArguments> => {
const check: CheckFunc<AppendArguments> = (args) => {
const { uploadTargets } = args;
if (uploadTargets.indexOf('queue') !== NOT_FOUND_INDEX) {
const { name, minZoom, maxZoom } = args;
const request: QueueSettings = {
name: name as string,
minZoom: minZoom as number,
maxZoom: maxZoom as number,
};
const validationResponse = validateBySchema<QueueSettings>(request, QUEUE_SETTINGS_SCHEMA);
invalidHandler(validationResponse);
}
return true;
};
return check;
};

export const dumpSourceCheck = (): CheckFunc<CreateArguments> => {
const check: CheckFunc<CreateArguments> = (args) => {
const { dumpSourceType, dumpSource } = args;

const errorPrefix = `provided dump source of type ${dumpSourceType} is not valid`;
if (dumpSourceType === DumpSourceType.LOCAL_FILE) {
if (!existsSync(dumpSource)) {
throw new Error(`${errorPrefix}, ${dumpSource} does not exist locally`);
}
} else if (isWebUri(dumpSource) === undefined) {
throw new Error(`${errorPrefix}, ${dumpSource} is not a valid web uri`);
}

return true;
};
return check;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

some documentation on the functions would be nice.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

public constructor(@inject(SERVICES.LOGGER) private readonly logger: Logger) {
this.ajv = new Ajv({ $data: true });
ajvKeywords.default(this.ajv);
export function validateBySchema<T>(content: unknown, schema: JSONSchemaType<T>): ValidationResponse<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

its more of an ajv wrapper, than validate by schema.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done.

@melancholiai melancholiai merged commit 9707577 into master May 3, 2022
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.

insert expired tiles to job queue

2 participants