Monitoring: add logic to pick the available stack for a service #3105
Conversation
…tech/tfgrid-sdk-ts into development_support_multiple_stacks
0oM4R
requested review from
AhmedHanafy725,
zaelgohary,
maayarosama,
MohamedElmdary,
Mahmoud-Emad,
amiraabouhadid,
AlaaElattar,
mohamedamer453 and
samaradel
as code owners
| private rmbValidatesChain = false; | ||
| private rmbTFchainUrls: string[]; |
There was a problem hiding this comment.
should we group all related rmb options into one object
rmb: {
urls: string[],
chainUrls?: string[],
validateChainUrl = false,
}
| "red", | ||
| ); | ||
| } | ||
| throw new Error(` Failed to reach ${service.serviceName()} on all provided stacks`); |
There was a problem hiding this comment.
Allow option silent to return null instead of error
There was a problem hiding this comment.
by default silent: false
| if (this[ServiceName.GraphQl]) { | ||
| if (this[ServiceName.GraphQl]?.length == 0) | ||
| result[ServiceName.GraphQl] = this.handleSilent( | ||
| "Can't validate GraphQL stacks; There is GraphQL urls provided", | ||
| ); | ||
| else { | ||
| result[ServiceName.GraphQl] = this.getAvailableServiceStack( | ||
| this[ServiceName.GraphQl], | ||
| new GraphQLMonitor(this[ServiceName.GraphQl][0]), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
should I extract this logic to a separated function like RMB?
| * @throws {Error} - Throws an error if no reachable service URL is found after checking all URLs. | ||
| * | ||
| */ | ||
| async getAvailableServiceStack(urls: string[], service: ILivenessChecker): Promise<ServiceUrl<N>> { |
There was a problem hiding this comment.
Here we should pass service instance, i don't like it but this will reduce code redundancy and handle all types of ILivenessChecker
| const gridproxy = ["test", "hello", "https://gridproxy.dev.grid.tf/"]; | ||
| const rmb = ["wsefews://tfin.dev.grid.tf/ws", "wss://relay.dev.grid.tf"]; | ||
| const graphql = ["https://graphql.dev.grid.tf/graphql"]; | ||
| const tfChain = ["wss://tfchain.dev.grid.tf/ws", "wsss://tfchain.dev.grid.tf/ws"]; |
There was a problem hiding this comment.
that don't look like valid urls, you should use a valid url but not reachable
| serviceName() { | ||
| return this.name; | ||
| } | ||
| serviceUrl() { | ||
| return this.url; | ||
| } | ||
| updateUrl(url: string) { | ||
| this.url = url; | ||
| } |
There was a problem hiding this comment.
what about using setters and getters here instead?
it would look better service.url instead of service.serviceUrl and so on...
| async getAvailableServiceStack(urls: string[], service: ILivenessChecker): Promise<ServiceUrl<N>> { | ||
| let error: Error | string = ""; | ||
| for (let i = 0; i < urls.length; i++) { | ||
| if (i != 0) await service.updateUrl(urls[i]); |
There was a problem hiding this comment.
| if (i != 0) await service.updateUrl(urls[i]); | |
| if (i != 0) service.updateUrl(urls[i]); |
- rename stats and activation classes
…into development_support_multiple_stacks
| constructor(activationServiceUrl?: string) { | ||
| if (activationServiceUrl) this.url = activationServiceUrl; | ||
| } | ||
| public get Name() { |
There was a problem hiding this comment.
the private property should start with _ and the getter should be lowercase
| if (!this.url) throw new Error("Can't access before initialization"); | ||
| return this.url; |
There was a problem hiding this comment.
no need to check if it's defined or not here. just return whatever the value
| this.URL = param.url; | ||
| } | ||
| async isAlive(): Promise<ServiceStatus> { | ||
| return resolveServiceStatus(sendRequest(this.url, { method: "Get" })); |
There was a problem hiding this comment.
you need here to make sure the URL is defined or not
|
#3134 all changes applied on |
Changes
ServiceUrlManagerpackage.jsonRelated Issues
sdk-ts: Support multiple stacks per network #3078Documentation PR
For UI changes, Please provide the Documetation PR on info_grid
Checklist