Feat/wna server#35
Conversation
4d7c708 to
227e07b
Compare
227e07b to
37786f7
Compare
|
|
||
| const adminLogger = createLogger('Admin'); | ||
|
|
||
| export const createAdminRouter = (): express.Router => { |
There was a problem hiding this comment.
those endpoints can be called by smartflows, can we make separate http server for it? We need to consider additional security layer here so no one can randomly stop instance
| }); | ||
|
|
||
| // Perform graceful shutdown after response is sent | ||
| setTimeout(() => { |
There was a problem hiding this comment.
I really dislike this setTimeout. Can you send SIGTERM and add event handler on it?
| }; | ||
| }; | ||
|
|
||
| const getOperatorInfo = async (timeoutMs: number = 5000): Promise<OperatorInfo | undefined> => { |
There was a problem hiding this comment.
move it to different place and export as public method
| | `LOCAL_SOLUTIONS_PATH` | Path to locally hosted NodeRed solution flow files to be used when installing solutions using "local" prefix within WorkLogic field. | `string` | | | ||
| | `SS58_FORMAT` | SS58 Key Format. | `number` (_>0_) | `42` | | ||
| | `PRETTY_PRINT` | Should pretty print logs. If you plan to use Grafana or any other log tooling it's recommended to set it to false. | `'true' \| 'false'` | `'false'` | | ||
| | `LOG_FILE_PATH` | Full path to log file (e.g., /var/log/app.log or ./logs/app.log) | `string` | | |
There was a problem hiding this comment.
If value is not provided it means that log file transport is disabled
| }; | ||
| }; | ||
|
|
||
| const getOperatorInfo = async (timeoutMs: number = 5000): Promise<OperatorInfo | undefined> => { |
There was a problem hiding this comment.
instead of undefined please use null, it's more explicit
| return { | ||
| id: solution.solutionId, | ||
| name, | ||
| status: 'Active', // Assume active if installed |
| solutionGroups, | ||
| }; | ||
| } catch { | ||
| return undefined; |
There was a problem hiding this comment.
at least print error so we know what is going on
| export const getAllInstalledSolutionsWithGroups = async (): Promise<InstalledSolutionDetails[]> => { | ||
| const tabNodes = await getTabNodes(); | ||
|
|
||
| const solutions: Array<InstalledSolutionDetails | null> = await Promise.all( |
There was a problem hiding this comment.
why you need Promise.all ?
| return logPath; | ||
| }; | ||
|
|
||
| const ensureLogDir = (logPath: string): void => { |
| @@ -0,0 +1,46 @@ | |||
| import express from 'express'; | |||
| import { createLogger } from '../util/logger'; | |||
| import { getIsShuttingDown } from '../shutdown'; | |||
There was a problem hiding this comment.
Can you create a middleware that will not accept new requests?
It can be even applied to submit vote endpoint so we don't accept new votes in case
| // Gather configuration (non-sensitive) | ||
| let rpcUrl: string | undefined; | ||
| let workerAddress: string | undefined; | ||
| try { |
There was a problem hiding this comment.
app should not initialize without worker address but it's ok.
I don't see a point to assign rpcUrl in both places, why not just const rpcUrl = MAIN_CONFIG.PALLET_RPC_URL ? We don't even need to reassign it, just use it in return
There was a problem hiding this comment.
| } | ||
|
|
||
| // Get operator information with solution groups | ||
| const operatorInfo = await getOperatorInfo(); |
There was a problem hiding this comment.
This logic does not make sense to me, first there is a try-catch that I assume checks if worker address is present and then you fetch operator info based on worker address without any additional checks?
Please clean up this logic as it's confusing
There was a problem hiding this comment.
updated check top logic
| : {}), | ||
| }; | ||
|
|
||
| const cacheKey = `file:${JSON.stringify(options)}`; |
There was a problem hiding this comment.
Can you make maybe hash out of this JSON stringify? It's not cheap operation
https://www.npmjs.com/package/object-hash
Maybe use this but let's ensure that it will work as expected for our use case
| translateTime: 'SYS:HH:MM:ss', | ||
| }; | ||
|
|
||
| const cacheKey = `pretty:${JSON.stringify(options)}`; |
| () => null, | ||
| ); | ||
|
|
||
| await api.disconnect().catch(() => { |
There was a problem hiding this comment.
It needs to be in some finally block so we are sure it will execute and not lost
| const logger = createLogger('OperatorAddressCache'); | ||
|
|
||
| // In-memory cache: Map<workerAddress, operatorAddress> | ||
| const operatorAddressCache = new Map<string, string>(); |
There was a problem hiding this comment.
Operator of worker might be changed while WNS is still running so it will return incorrect information.
Please add some cache expiration, ideally you could use https://www.npmjs.com/package/cache-manager
|
|
||
| // Extracts solution name, removing UUID suffix if present | ||
| const extractSolutionName = (solutionId: string): string => { | ||
| const lastDot = solutionId.lastIndexOf('.'); |
There was a problem hiding this comment.
What is the use case? It looks like it's for some very specific use case, can you describe that one? In some cases it might lead to undefined-behavior.
There was a problem hiding this comment.
put in func description
| }; | ||
| }; | ||
|
|
||
| export const getSolutionGroupsDetailsStatus = async (): Promise<SolutionGroupsDetailsStatus> => { |
No description provided.