Adjusting Create Acceleration Flyout Props #289
Conversation
Signed-off-by: Sean Li <[email protected]>
| @@ -87,6 +87,7 @@ export interface CreateAccelerationForm { | |||
| } | |||
|
|
|||
| export enum AsyncQueryStatus { | |||
There was a problem hiding this comment.
Is this state equivalent to DirectQueryLoadingStatus?
| setAccelerationFlyout = () => { | ||
| this.renderCreateAccelerationFlyout(this.props.selectedDatasource[0].label); | ||
| const { loadStatus, startLoading, stopLoading } = catalogCacheRefs.useLoadTableColumnsToCache(); | ||
| this.renderCreateAccelerationFlyout( | ||
| this.props.selectedDatasource[0].label, | ||
| loadStatus, | ||
| startLoading, | ||
| stopLoading | ||
| ); |
There was a problem hiding this comment.
What is this function used for?
cc: @ps48 for more context
There was a problem hiding this comment.
This is used for users coming from URL to see the acceleration flyout open.
| ] = createGetterSetter< | ||
| ( | ||
| dataSource: string, | ||
| loadStatus: any, |
There was a problem hiding this comment.
How should DirectQueryLoadingStatus be brought over from dashboards-observability?
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #289 +/- ##
==========================================
+ Coverage 59.34% 59.49% +0.14%
==========================================
Files 39 40 +1
Lines 1835 1844 +9
Branches 328 328
==========================================
+ Hits 1089 1097 +8
- Misses 633 634 +1
Partials 113 113
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| return () => { | ||
| stopLoading(); | ||
| }; | ||
| }, []); |
| ( | ||
| dataSource: string, | ||
| loadStatus: any, | ||
| startLoading: (dataSourceName: string, databaseName?: string, tableName?: string) => void, |
There was a problem hiding this comment.
I saw there are repeated patterns not only in this PR where we have a same set of interfaces for functions and components but the interfaces are duplicated and redefined repeatedly, feel like we should define common interfaces and reuse them or leverage utility types for composing types and interfaces. It's ok for this PR and release but we should follow up on this later.
|
Fixing with #1598 |
Description
renderCreateAccelerationFlyoutshould match what is updated in the above PRDraft PR for some questions
Issues Resolved
N/A
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.