-
Notifications
You must be signed in to change notification settings - Fork 0
feat!: mdr integration and deps update #28
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
Conversation
export interface IMdrClient { | ||
getStatus: () => Promise<EnrollmentStatus>; | ||
postEnrollment: (enrollment: EnrollmentRequestBody) => Promise<void>; | ||
} | ||
|
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.
Just use MdrClient class as a type. No need for separate interface
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.
Using interfaces as a type-safe option isn't redundant.
I find the explicit interface more readable, documented, type-safe, and less error-prone in future development.
InstanceType<typeof MdrClient>
isn't as elaborate as a full interface.
return (server: http.Server): http.Server => { | ||
return createTerminus(server, { | ||
// eslint-disable-next-line @typescript-eslint/naming-convention | ||
healthChecks: { '/liveness': stubHealthcheck }, |
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.
Out of scope for this PR but, can we think of a real healthcheck?
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.
It is possible to check connectivity with the configured database, the problem is code-wise, we don't use a pg client
but wrap the execution with osm2pgsql
.
To have a proper healthcheck, we also need a pg client
which takes and manages database connections, those are already a limited resource.
So today, if for any reason, the connection has gone bad, we will be notified once there is a job to be executed and the connection is off.
Deprecations: k8s cronjob is no longer supported.