-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Debug health check #5587
Debug health check #5587
Conversation
@azhavoro , can we customize the error message in the PR? |
cvat-ui/src/utils/enviroment.ts
Outdated
@@ -29,3 +29,7 @@ export function customWaViewHit(pageName?: string, queryString?: string, hashInf | |||
} | |||
} | |||
} | |||
|
|||
export function isPublicInstance(): boolean { |
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.
I don't think that it is a right solution. Probably HealthCheck endpoint can return extra information. Need to think how to implement that. Another solution is to have the message as a variable in settings. Please think and propose the best solution.
@azhavoro , could you please resolve the conflict? |
if (Object.prototype.hasOwnProperty.call(data, checkName) && | ||
checkName !== 'Cache backend: media' && | ||
data[checkName] !== 'working') { | ||
isHealthy = false; |
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.
If it is a temporary workaround, let's keep a comment about it here
Co-authored-by: Boris Sekachev <boris.sekachev@yandex.ru>
Co-authored-by: Boris Sekachev <boris.sekachev@yandex.ru>
Co-authored-by: Boris Sekachev <boris.sekachev@yandex.ru>
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.
From my perspective these changes are fine!
<!-- Raised an issue to propose your change (https://github.com/cvat-ai/cvat/issues). It helps to avoid duplication of efforts from multiple independent contributors. Discuss your ideas with maintainers to be sure that changes will be approved and merged. Read the [CONTRIBUTION](https://github.com/cvat-ai/cvat/blob/develop/CONTRIBUTING.md) guide. --> <!-- Provide a general summary of your changes in the Title above --> ### Implementation & using Regular import from the whole cvat-ui-app, like: ```import consts from 'config';``` To change config for app.cvat.ui, need to specify a new file during webpack build time: ```UI_APP_CONFIG='/home/bsekachev/config.override.tsx' yarn build``` In development: ``UI_APP_CONFIG='/home/bsekachev/config.override.tsx' yarn start:cvat-ui`` Override file content in this case is simple: ```ts import config from './cvat/cvat-ui/src/config'; config.HEALH_CHECK_RETRIES = 5; export default { ...config, NEW_CONFIG_VARIABLE: 'test value', }; ``` Implemented via webpack aliases: ``` resolve { alias: { config$: appConfigFile, } } ``` + No typescript errors in code, because default ``app-config`` exists in ``src``. Co-authored-by: Andrey Zhavoronkov <andrey@cvat.ai>
Motivation and context
How has this been tested?
Checklist
develop
branchcvat-core, cvat-data and cvat-ui)
License
Feel free to contact the maintainers if that's a concern.