You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/07/15 00:48:47 UTC
[GitHub] [superset] ktmud opened a new pull request #15699: perf(dashboard): make loading datasets non-blocking
ktmud opened a new pull request #15699:
URL: https://github.com/apache/superset/pull/15699
### SUMMARY
This will be one of the many PRs to address perf issues raised in #15518 .
Currently the Dashboard page will wait for both `datasets` and `charts` data to start rendering, however the datasets data are only used in filter indicators and filter boxes and should not block us from rendering other charts. In our case, the `datasets` endpoint is often much slower because we have a giant single-source-of-truth datasource with thousands of metrics and columns.
Unblocking rendering from the datasets endpoint should speed up the page rendering a little bit. In our use case, it's often a matter of a full second.
It's not easy to simply remove `datasources` from Dashboard state as that would involve letting the filter indicators and filter box fetching datasource metadata themselves.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A
### TESTING INSTRUCTIONS
Go to dashboard page, make sure everything still renders correctly.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #15699:
URL: https://github.com/apache/superset/pull/15699#discussion_r670046995
##########
File path: superset-frontend/src/explore/components/Control.tsx
##########
@@ -43,53 +43,38 @@ export type ControlProps = {
renderTrigger?: boolean;
};
-export default class Control extends React.PureComponent<
Review comment:
Refactor from class component to functional. Not related to this PR but may be useful in the future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #15699:
URL: https://github.com/apache/superset/pull/15699#discussion_r670078905
##########
File path: superset-frontend/src/dashboard/actions/datasources.ts
##########
@@ -16,26 +16,44 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { Dispatch } from 'redux';
import { SupersetClient } from '@superset-ui/core';
-import { getClientErrorObject } from '../../utils/getClientErrorObject';
+import { Datasource, RootState } from 'src/dashboard/types';
-export const SET_DATASOURCE = 'SET_DATASOURCE';
-export function setDatasource(datasource, key) {
- return { type: SET_DATASOURCE, datasource, key };
+// update datasources index for Dashboard
+export enum DatasourcesAction {
+ SET_DATASOURCES = 'SET_DATASOURCES',
+ SET_DATASOURCE = 'SET_DATASOURCE',
}
-export const FETCH_DATASOURCE_STARTED = 'FETCH_DATASOURCE_STARTED';
-export function fetchDatasourceStarted(key) {
- return { type: FETCH_DATASOURCE_STARTED, key };
+export type DatasourcesActionPayload =
+ | {
+ type: DatasourcesAction.SET_DATASOURCES;
+ datasources: Datasource[] | null;
+ }
+ | {
+ type: DatasourcesAction.SET_DATASOURCE;
+ key: Datasource['uid'];
+ datasource: Datasource;
+ };
+
+export function setDatasources(datasources: Datasource[] | null) {
+ return {
+ type: DatasourcesAction.SET_DATASOURCES,
+ datasources,
+ };
}
-export const FETCH_DATASOURCE_FAILED = 'FETCH_DATASOURCE_FAILED';
-export function fetchDatasourceFailed(error, key) {
- return { type: FETCH_DATASOURCE_FAILED, error, key };
Review comment:
Cleaned up `FETCH_DATASOURCE_STARTED` and `FETCH_DATASOURCE_FAILED` because there is no corresponding reducers for them.
##########
File path: superset-frontend/src/dashboard/actions/datasources.ts
##########
@@ -16,26 +16,44 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { Dispatch } from 'redux';
import { SupersetClient } from '@superset-ui/core';
-import { getClientErrorObject } from '../../utils/getClientErrorObject';
+import { Datasource, RootState } from 'src/dashboard/types';
-export const SET_DATASOURCE = 'SET_DATASOURCE';
-export function setDatasource(datasource, key) {
- return { type: SET_DATASOURCE, datasource, key };
+// update datasources index for Dashboard
+export enum DatasourcesAction {
+ SET_DATASOURCES = 'SET_DATASOURCES',
+ SET_DATASOURCE = 'SET_DATASOURCE',
}
-export const FETCH_DATASOURCE_STARTED = 'FETCH_DATASOURCE_STARTED';
-export function fetchDatasourceStarted(key) {
- return { type: FETCH_DATASOURCE_STARTED, key };
+export type DatasourcesActionPayload =
+ | {
+ type: DatasourcesAction.SET_DATASOURCES;
+ datasources: Datasource[] | null;
+ }
+ | {
+ type: DatasourcesAction.SET_DATASOURCE;
+ key: Datasource['uid'];
+ datasource: Datasource;
+ };
+
+export function setDatasources(datasources: Datasource[] | null) {
+ return {
+ type: DatasourcesAction.SET_DATASOURCES,
+ datasources,
+ };
}
-export const FETCH_DATASOURCE_FAILED = 'FETCH_DATASOURCE_FAILED';
-export function fetchDatasourceFailed(error, key) {
- return { type: FETCH_DATASOURCE_FAILED, error, key };
Review comment:
Cleaned up `FETCH_DATASOURCE_STARTED` and `FETCH_DATASOURCE_FAILED` because there are no corresponding reducers for them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880320556
# [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15699](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e3ccef1) into [master](https://codecov.io/gh/apache/superset/commit/b489cffb570c4d078ec982fd11c3e46dd0310f24?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b489cff) will **increase** coverage by `0.00%`.
> The diff coverage is `31.25%`.
[](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15699 +/- ##
=======================================
Coverage 76.82% 76.83%
=======================================
Files 983 983
Lines 51585 51578 -7
Branches 6974 6976 +2
=======================================
- Hits 39632 39630 -2
+ Misses 11729 11726 -3
+ Partials 224 222 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.35% <31.25%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (ø)` | |
| [...set-frontend/src/dashboard/containers/Dashboard.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC50cw==) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/dashboard/containers/DashboardPage.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZFBhZ2UudHN4) | `0.00% <0.00%> (ø)` | |
| [...rset-frontend/src/dashboard/actions/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2RhdGFzb3VyY2VzLnRz) | `25.00% <30.00%> (ø)` | |
| [...perset-frontend/src/explore/components/Control.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sLnRzeA==) | `76.47% <71.42%> (+1.47%)` | :arrow_up: |
| [...set-frontend/src/dashboard/reducers/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXRhc291cmNlcy50cw==) | `75.00% <75.00%> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `1.70% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `58.97% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupErrorMessages.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlcy50cw==) | `0.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b489cff...e3ccef1](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] graceguo-supercat commented on a change in pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on a change in pull request #15699:
URL: https://github.com/apache/superset/pull/15699#discussion_r670163883
##########
File path: superset-frontend/src/types/Dashboard.ts
##########
@@ -19,7 +19,7 @@
import Owner from './Owner';
import Role from './Role';
-type Dashboard = {
+export interface Dashboard {
Review comment:
what is this change for? Dashboard doesn't have methods why prefer interface?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] suddjian commented on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880841638
Have we verified yet that none of the chart plugins use datasets? If so we should also stop passing datasets to the charts.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880320556
# [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15699](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (92fda2d) into [master](https://codecov.io/gh/apache/superset/commit/b489cffb570c4d078ec982fd11c3e46dd0310f24?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b489cff) will **increase** coverage by `0.02%`.
> The diff coverage is `31.57%`.
> :exclamation: Current head 92fda2d differs from pull request most recent head ddee3e9. Consider uploading reports for the commit ddee3e9 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15699 +/- ##
==========================================
+ Coverage 76.82% 76.84% +0.02%
==========================================
Files 983 983
Lines 51585 51566 -19
Branches 6974 6975 +1
==========================================
- Hits 39632 39628 -4
+ Misses 11729 11716 -13
+ Partials 224 222 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.38% <31.57%> (+0.03%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (ø)` | |
| [...set-frontend/src/dashboard/containers/Dashboard.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC50cw==) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/dashboard/containers/DashboardPage.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZFBhZ2UudHN4) | `0.00% <0.00%> (ø)` | |
| [...rset-frontend/src/dashboard/actions/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2RhdGFzb3VyY2VzLnRz) | `60.00% <50.00%> (ø)` | |
| [...perset-frontend/src/explore/components/Control.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sLnRzeA==) | `76.47% <71.42%> (+1.47%)` | :arrow_up: |
| [...set-frontend/src/dashboard/reducers/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXRhc291cmNlcy50cw==) | `83.33% <83.33%> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `1.70% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `58.97% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupErrorMessages.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlcy50cw==) | `0.00% <0.00%> (ø)` | |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b489cff...ddee3e9](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #15699:
URL: https://github.com/apache/superset/pull/15699#discussion_r670675425
##########
File path: superset-frontend/src/common/hooks/apiResources/dashboards.ts
##########
@@ -33,10 +34,10 @@ export const useDashboard = (idOrSlug: string | number) =>
// gets the chart definitions for a dashboard
export const useDashboardCharts = (idOrSlug: string | number) =>
- useApiV1Resource(`/api/v1/dashboard/${idOrSlug}/charts`);
+ useApiV1Resource<Chart[]>(`/api/v1/dashboard/${idOrSlug}/charts`);
// gets the datasets for a dashboard
// important: this endpoint only returns the fields in the dataset
// that are necessary for rendering the given dashboard
export const useDashboardDatasets = (idOrSlug: string | number) =>
- useApiV1Resource(`/api/v1/dashboard/${idOrSlug}/datasets`);
+ useApiV1Resource<Datasource[]>(`/api/v1/dashboard/${idOrSlug}/datasets`);
Review comment:
There are many places where the Datasource/Dataset type is defined---including two places in `superset-ui`. I think we need a separate PR to consolidate those. Although they do have small differences depending on the context.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880320556
# [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15699](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ec7416) into [master](https://codecov.io/gh/apache/superset/commit/b489cffb570c4d078ec982fd11c3e46dd0310f24?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b489cff) will **increase** coverage by `0.00%`.
> The diff coverage is `33.82%`.
> :exclamation: Current head 4ec7416 differs from pull request most recent head f6a090b. Consider uploading reports for the commit f6a090b to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15699 +/- ##
=======================================
Coverage 76.82% 76.83%
=======================================
Files 983 983
Lines 51585 51581 -4
Branches 6974 6979 +5
=======================================
Hits 39632 39632
+ Misses 11729 11727 -2
+ Partials 224 222 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.35% <33.82%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/chart/Chart.jsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0LmpzeA==) | `46.29% <ø> (ø)` | |
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (ø)` | |
| [...set-frontend/src/dashboard/containers/Dashboard.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC50cw==) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/dashboard/containers/DashboardPage.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZFBhZ2UudHN4) | `0.00% <0.00%> (ø)` | |
| [...rset-frontend/src/dashboard/actions/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2RhdGFzb3VyY2VzLnRz) | `25.00% <30.00%> (ø)` | |
| [superset-frontend/src/chart/ChartRenderer.jsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NoYXJ0L0NoYXJ0UmVuZGVyZXIuanN4) | `37.97% <50.00%> (+0.31%)` | :arrow_up: |
| [...perset-frontend/src/explore/components/Control.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sLnRzeA==) | `76.47% <71.42%> (+1.47%)` | :arrow_up: |
| [...set-frontend/src/dashboard/reducers/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXRhc291cmNlcy50cw==) | `75.00% <75.00%> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `1.70% <100.00%> (ø)` | |
| [.../src/dashboard/components/gridComponents/Chart.jsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0NoYXJ0LmpzeA==) | `80.37% <100.00%> (+0.18%)` | :arrow_up: |
| ... and [3 more](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b489cff...f6a090b](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] graceguo-supercat commented on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
graceguo-supercat commented on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880442129
this is a reasonable trade-off we can try. Is it possible that filter_box started query before dataset metadata is not ready, will that cause any issue?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #15699:
URL: https://github.com/apache/superset/pull/15699#discussion_r670270489
##########
File path: superset-frontend/src/types/Dashboard.ts
##########
@@ -19,7 +19,7 @@
import Owner from './Owner';
import Role from './Role';
-type Dashboard = {
+export interface Dashboard {
Review comment:
Interface is always preferred other than in React props.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud edited a comment on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880924867
> Have we verified yet that none of the chart plugins use datasets? If so we should also stop passing datasets to the charts.
Thanks for pointing this out. I did more vetting and realized that some charts do use info on `datasource`, mostly non-critical info such as column name `verboseMap` here: https://github.com/apache-superset/superset-ui/blob/61accc6ce9c546331b7407cde21d6ec163fe191d/plugins/legacy-plugin-chart-pivot-table/src/transformProps.js
Fixed a re-render issue now that charts will be updated accordingly once datasets are loaded:
https://user-images.githubusercontent.com/335541/125838954-c30b276b-24e2-4612-97c8-60d97f931065.mp4
Notice the name of the first column. It uses `verbose_map` and updated after `/datasets` is fully loaded. Note that I artificially slowed down the `/datasets` API. For most users, this minor update may not be even noticeable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud merged pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud merged pull request #15699:
URL: https://github.com/apache/superset/pull/15699
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880320556
# [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15699](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e3ccef1) into [master](https://codecov.io/gh/apache/superset/commit/b489cffb570c4d078ec982fd11c3e46dd0310f24?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b489cff) will **increase** coverage by `0.00%`.
> The diff coverage is `31.25%`.
> :exclamation: Current head e3ccef1 differs from pull request most recent head f6a090b. Consider uploading reports for the commit f6a090b to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15699 +/- ##
=======================================
Coverage 76.82% 76.83%
=======================================
Files 983 983
Lines 51585 51578 -7
Branches 6974 6976 +2
=======================================
- Hits 39632 39630 -2
+ Misses 11729 11726 -3
+ Partials 224 222 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.35% <31.25%> (+0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (ø)` | |
| [...set-frontend/src/dashboard/containers/Dashboard.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC50cw==) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/dashboard/containers/DashboardPage.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZFBhZ2UudHN4) | `0.00% <0.00%> (ø)` | |
| [...rset-frontend/src/dashboard/actions/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2RhdGFzb3VyY2VzLnRz) | `25.00% <30.00%> (ø)` | |
| [...perset-frontend/src/explore/components/Control.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sLnRzeA==) | `76.47% <71.42%> (+1.47%)` | :arrow_up: |
| [...set-frontend/src/dashboard/reducers/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXRhc291cmNlcy50cw==) | `75.00% <75.00%> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `1.70% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `58.97% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupErrorMessages.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlcy50cw==) | `0.00% <0.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b489cff...f6a090b](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] etr2460 commented on a change in pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #15699:
URL: https://github.com/apache/superset/pull/15699#discussion_r670438909
##########
File path: superset-frontend/src/common/hooks/apiResources/dashboards.ts
##########
@@ -33,10 +34,10 @@ export const useDashboard = (idOrSlug: string | number) =>
// gets the chart definitions for a dashboard
export const useDashboardCharts = (idOrSlug: string | number) =>
- useApiV1Resource(`/api/v1/dashboard/${idOrSlug}/charts`);
+ useApiV1Resource<Chart[]>(`/api/v1/dashboard/${idOrSlug}/charts`);
// gets the datasets for a dashboard
// important: this endpoint only returns the fields in the dataset
// that are necessary for rendering the given dashboard
export const useDashboardDatasets = (idOrSlug: string | number) =>
- useApiV1Resource(`/api/v1/dashboard/${idOrSlug}/datasets`);
+ useApiV1Resource<Datasource[]>(`/api/v1/dashboard/${idOrSlug}/datasets`);
Review comment:
nit: perhaps we should rename the Datasource type to Dataset? Or at least alias it for now?
##########
File path: superset-frontend/src/explore/components/Control.tsx
##########
@@ -43,53 +43,38 @@ export type ControlProps = {
renderTrigger?: boolean;
};
-export default class Control extends React.PureComponent<
- ControlProps,
- { hovered: boolean }
-> {
- onMouseEnter: () => void;
+export default function Control(props: ControlProps) {
+ const {
+ actions: { setControlValue },
+ name,
+ type,
+ hidden,
+ } = props;
- onMouseLeave: () => void;
+ const [hovered, setHovered] = useState(false);
+ const onChange = useCallback(
+ (value: any, errors: any[]) => setControlValue(name, value, errors),
+ [name, setControlValue],
+ );
- constructor(props: ControlProps) {
- super(props);
- this.state = { hovered: false };
- this.onChange = this.onChange.bind(this);
- this.onMouseEnter = this.setHover.bind(this, true);
- this.onMouseLeave = this.setHover.bind(this, false);
- }
-
- onChange(value: any, errors: any[]) {
- this.props.actions.setControlValue(this.props.name, value, errors);
- }
+ if (!type) return null;
- setHover(hovered: boolean) {
- this.setState({ hovered });
+ const ControlComponent = typeof type === 'string' ? controlMap[type] : type;
+ if (!ControlComponent) {
+ return <>Unknown controlType: {type}</>;
Review comment:
why do we need a fragment here? Couldn't this be:
```suggestion
return `Unknown controlType: ${type}`;
```
##########
File path: superset-frontend/src/types/Dashboard.ts
##########
@@ -19,7 +19,7 @@
import Owner from './Owner';
import Role from './Role';
-type Dashboard = {
+export interface Dashboard {
Review comment:
fwiw i think there's still a bunch of discrepancies across Superset for this. I don't really care one way or another (consistency is better in my mind) but noting that even after this change there will still be a bunch of inconsistencies
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880320556
# [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15699](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ddee3e9) into [master](https://codecov.io/gh/apache/superset/commit/b489cffb570c4d078ec982fd11c3e46dd0310f24?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b489cff) will **increase** coverage by `0.02%`.
> The diff coverage is `32.14%`.
> :exclamation: Current head ddee3e9 differs from pull request most recent head e3ccef1. Consider uploading reports for the commit e3ccef1 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15699 +/- ##
==========================================
+ Coverage 76.82% 76.85% +0.02%
==========================================
Files 983 983
Lines 51585 51565 -20
Branches 6974 6975 +1
==========================================
- Hits 39632 39628 -4
+ Misses 11729 11715 -14
+ Partials 224 222 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.38% <32.14%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (ø)` | |
| [...set-frontend/src/dashboard/containers/Dashboard.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC50cw==) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/dashboard/containers/DashboardPage.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZFBhZ2UudHN4) | `0.00% <0.00%> (ø)` | |
| [...rset-frontend/src/dashboard/actions/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2RhdGFzb3VyY2VzLnRz) | `60.00% <50.00%> (ø)` | |
| [...perset-frontend/src/explore/components/Control.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sLnRzeA==) | `76.47% <71.42%> (+1.47%)` | :arrow_up: |
| [...set-frontend/src/dashboard/reducers/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXRhc291cmNlcy50cw==) | `83.33% <83.33%> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `1.70% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `58.97% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupErrorMessages.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlcy50cw==) | `0.00% <0.00%> (ø)` | |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b489cff...e3ccef1](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880924867
> Have we verified yet that none of the chart plugins use datasets? If so we should also stop passing datasets to the charts.
Thanks for pointing this out. I did more vetting and realized that some charts do use info on `datasource`, mostly non-critical info such as column name `verboseMap` here: https://github.com/apache-superset/superset-ui/blob/61accc6ce9c546331b7407cde21d6ec163fe191d/plugins/legacy-plugin-chart-pivot-table/src/transformProps.js
Fixed a re-render issue now that charts will be updated accordingly once datasets are loaded:
https://user-images.githubusercontent.com/335541/125838954-c30b276b-24e2-4612-97c8-60d97f931065.mp4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880320556
# [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15699](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ddee3e9) into [master](https://codecov.io/gh/apache/superset/commit/b489cffb570c4d078ec982fd11c3e46dd0310f24?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b489cff) will **increase** coverage by `0.02%`.
> The diff coverage is `32.14%`.
> :exclamation: Current head ddee3e9 differs from pull request most recent head 21d3e92. Consider uploading reports for the commit 21d3e92 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15699 +/- ##
==========================================
+ Coverage 76.82% 76.85% +0.02%
==========================================
Files 983 983
Lines 51585 51565 -20
Branches 6974 6975 +1
==========================================
- Hits 39632 39628 -4
+ Misses 11729 11715 -14
+ Partials 224 222 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.38% <32.14%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (ø)` | |
| [...set-frontend/src/dashboard/containers/Dashboard.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC50cw==) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/dashboard/containers/DashboardPage.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZFBhZ2UudHN4) | `0.00% <0.00%> (ø)` | |
| [...rset-frontend/src/dashboard/actions/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2RhdGFzb3VyY2VzLnRz) | `60.00% <50.00%> (ø)` | |
| [...perset-frontend/src/explore/components/Control.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sLnRzeA==) | `76.47% <71.42%> (+1.47%)` | :arrow_up: |
| [...set-frontend/src/dashboard/reducers/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXRhc291cmNlcy50cw==) | `83.33% <83.33%> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `1.70% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `58.97% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupErrorMessages.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlcy50cw==) | `0.00% <0.00%> (ø)` | |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b489cff...21d3e92](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] suddjian edited a comment on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
suddjian edited a comment on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880841638
Have we verified yet that none of the chart plugins use datasets? If so we should also stop passing datasets to the charts.
If any of the plugins do use datasets, this might break them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on a change in pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud commented on a change in pull request #15699:
URL: https://github.com/apache/superset/pull/15699#discussion_r670675895
##########
File path: superset-frontend/src/explore/components/Control.tsx
##########
@@ -43,53 +43,38 @@ export type ControlProps = {
renderTrigger?: boolean;
};
-export default class Control extends React.PureComponent<
- ControlProps,
- { hovered: boolean }
-> {
- onMouseEnter: () => void;
+export default function Control(props: ControlProps) {
+ const {
+ actions: { setControlValue },
+ name,
+ type,
+ hidden,
+ } = props;
- onMouseLeave: () => void;
+ const [hovered, setHovered] = useState(false);
+ const onChange = useCallback(
+ (value: any, errors: any[]) => setControlValue(name, value, errors),
+ [name, setControlValue],
+ );
- constructor(props: ControlProps) {
- super(props);
- this.state = { hovered: false };
- this.onChange = this.onChange.bind(this);
- this.onMouseEnter = this.setHover.bind(this, true);
- this.onMouseLeave = this.setHover.bind(this, false);
- }
-
- onChange(value: any, errors: any[]) {
- this.props.actions.setControlValue(this.props.name, value, errors);
- }
+ if (!type) return null;
- setHover(hovered: boolean) {
- this.setState({ hovered });
+ const ControlComponent = typeof type === 'string' ? controlMap[type] : type;
+ if (!ControlComponent) {
+ return <>Unknown controlType: {type}</>;
Review comment:
React typing would complain if returning a plain string. Something about expecting a ComponentType to always return ReactElement.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880320556
# [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#15699](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ddee3e9) into [master](https://codecov.io/gh/apache/superset/commit/b489cffb570c4d078ec982fd11c3e46dd0310f24?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b489cff) will **increase** coverage by `0.02%`.
> The diff coverage is `32.14%`.
[](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #15699 +/- ##
==========================================
+ Coverage 76.82% 76.85% +0.02%
==========================================
Files 983 983
Lines 51585 51565 -20
Branches 6974 6975 +1
==========================================
- Hits 39632 39628 -4
+ Misses 11729 11715 -14
+ Partials 224 222 -2
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.38% <32.14%> (+0.04%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (ø)` | |
| [...set-frontend/src/dashboard/containers/Dashboard.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZC50cw==) | `0.00% <0.00%> (ø)` | |
| [...rontend/src/dashboard/containers/DashboardPage.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0Rhc2hib2FyZFBhZ2UudHN4) | `0.00% <0.00%> (ø)` | |
| [...rset-frontend/src/dashboard/actions/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2RhdGFzb3VyY2VzLnRz) | `60.00% <50.00%> (ø)` | |
| [...perset-frontend/src/explore/components/Control.tsx](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9Db250cm9sLnRzeA==) | `76.47% <71.42%> (+1.47%)` | :arrow_up: |
| [...set-frontend/src/dashboard/reducers/datasources.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9kYXRhc291cmNlcy50cw==) | `83.33% <83.33%> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `1.70% <100.00%> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `58.97% <0.00%> (ø)` | |
| [superset-frontend/src/setup/setupErrorMessages.ts](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlcy50cw==) | `0.00% <0.00%> (ø)` | |
| ... and [1 more](https://codecov.io/gh/apache/superset/pull/15699/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [b489cff...ddee3e9](https://codecov.io/gh/apache/superset/pull/15699?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] ktmud commented on pull request #15699: perf(dashboard): make loading datasets non-blocking
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #15699:
URL: https://github.com/apache/superset/pull/15699#issuecomment-880525115
> this is a reasonable trade-off we can try. Is it possible that filter_box started query before dataset metadata is not ready, will that cause any issue?
Time Column and Granularity control in filter boxes may not have select choices. Filter indicators may show inexplicable filter values. But those are expected to be temporary and not very noticeable.
In case datasets loading fails, in previous case, the whole page won't render, while after this change you still have a largely functional Dashboard page.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org