-
Notifications
You must be signed in to change notification settings - Fork 18
feat: add partitions progress bar to table info #3206
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
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.
6 files reviewed, 1 comment
src/containers/Tenant/Diagnostics/Overview/TableInfo/prepareTableInfo.tsx
Outdated
Show resolved
Hide resolved
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.
7 files reviewed, no comments
src/containers/Tenant/Diagnostics/Overview/TableInfo/PartitionsProgress/helpers.ts
Outdated
Show resolved
Hide resolved
|
|
||
| // Feature flag: show partitions progress only if WINDOW_SHOW_TABLE_SETTINGS is truthy | ||
| const isPartitionsProgressEnabled = Boolean( | ||
| (window as unknown as {WINDOW_SHOW_TABLE_SETTINGS?: unknown}).WINDOW_SHOW_TABLE_SETTINGS, |
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 think we could add type to window.d.ts to avoid "as unknown"
astandrik
left a comment
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.
Left couple of questions
src/containers/Tenant/Diagnostics/Overview/TableInfo/i18n/en.json
Outdated
Show resolved
Hide resolved
src/containers/Tenant/Diagnostics/Overview/TableInfo/PartitionsProgress/helpers.ts
Outdated
Show resolved
Hide resolved
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.
8 files reviewed, no comments
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.
8 files reviewed, no comments
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.
8 files reviewed, no comments
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.
10 files reviewed, no comments
| const width = `${value}%`; | ||
|
|
||
| return ( | ||
| <div |
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 decided not to use Progress component, because when we have only main progress bar and if the value of current partitions is quite small, we should fix min-width in 20 px for filling part (designer said). Progress component doesn't allow to do it.
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.
10 files reviewed, no comments
| partitionProgressConfig, | ||
| } = React.useMemo(() => prepareTableInfo(data, type), [data, type]); | ||
|
|
||
| // Feature flag: show partitions progress only if WINDOW_SHOW_TABLE_SETTINGS is truthy |
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.
How is it intended to be set to truthy on prod?
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 want that users won't see these changes until all work of this task is finished...
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 suppose we do things like that via uiFactory
search codebase for
export interface UIFactory<H extends string = CommonIssueType, T extends string = string> {
I think these examples look like what you may use
hasAccess?: boolean;
hideGrantAccess?: boolean;
useDatabaseId?: boolean;
useMetaProxy?: boolean;
useClusterDomain?: boolean;
The partitions progress bar is hidden behind a window flag for now.
To enable it in the browser, run in DevTools console:
window.WINDOW_SHOW_TABLE_SETTINGS = true;Stand: https://nda.ya.ru/t/0KO6gq_k7PvGjP
In this database you can see different tables with different number of partitions (with min, max, less then min, with no max)
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Test Changes Summary ⏭️2
⏭️ Skipped Tests (2)
Bundle Size: 🔺
Current: 62.54 MB | Main: 62.52 MB
Diff: +0.02 MB (0.03%)
ℹ️ CI Information