You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "geido (via GitHub)" <gi...@apache.org> on 2024/07/18 12:25:21 UTC
[PR] chore(Dashboard): Remove unnecessary loading [superset]
geido opened a new pull request, #29632:
URL: https://github.com/apache/superset/pull/29632
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] 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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2284016234
@geido Tested again but it looks like the requests are still dispatched twice
<img width="1720" alt="image" src="https://github.com/user-attachments/assets/3d904fbd-d8e3-4a55-ba6e-707d7b8d8f1b">
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2276000907
@geido Ephemeral environment spinning up at http://34.215.200.79:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #29632:
URL: https://github.com/apache/superset/pull/29632#discussion_r1742216692
##########
tests/integration_tests/superset_test_config.py:
##########
@@ -154,3 +154,4 @@ class CeleryConfig:
}
PRESERVE_CONTEXT_ON_EXCEPTION = False
+COMPRESS_REGISTER = False
Review Comment:
To remove
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2271552203
@geido It looks like the data requests are done multiple times - twice without native filters, and even more times with native filters
BEFORE without and with native filters - all data requests fired only once
<img width="1728" alt="Screenshot 2024-08-06 at 17 16 26" src="https://github.com/user-attachments/assets/f41a481c-7a40-4d84-9190-b993614c534b">
<img width="1728" alt="Screenshot 2024-08-06 at 17 16 45" src="https://github.com/user-attachments/assets/cac750f4-3667-43e3-bd7f-2ae01dd27831">
AFTER without and with native filters
<img width="1728" alt="Screenshot 2024-08-06 at 17 15 49" src="https://github.com/user-attachments/assets/19dd31fb-9dc1-477b-9765-baf56b7acba4">
<img width="1728" alt="Screenshot 2024-08-06 at 17 14 27" src="https://github.com/user-attachments/assets/f7f9f36c-815f-4774-bcd5-3f26473de682">
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2326679551
Tentatively opening this up for review
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2275971708
/testenv up
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2269037477
/testenv up
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2325008897
/testenv up
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2414341041
This PR will be superseded by smaller PRs. I will update this thread accordingly:
https://github.com/apache/superset/pull/30611
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido closed pull request #29632: chore(Dashboard): Remove unnecessary loading
URL: https://github.com/apache/superset/pull/29632
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2269175709
Dashboard doesn't load when I try to open it from tab permalink
https://github.com/user-attachments/assets/fca176fd-c5f3-4f5b-8db6-275b2b1562ac
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2271430590
Great catch @kgabryje. After a bit of investigation I found that permalinks use `uuid` for some reasons, but the frontend was completely unaware of their existence and just relied on ID or slugs. After adding some checks this issue popped up. Now the backend is exposing the uuid to the frontend to do the necessary checks when permalinks are involved.
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2275971301
Thanks again @kgabryje. By unblocking the skeleton a few competing places were trying to run the requests. I did my best to spot all of them and control the flow a little bit better. I am continuing with functional tests. All the help is appreciated!
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2269058579
@kgabryje Ephemeral environment spinning up at http://34.214.114.205:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2296961215
Thanks for catching that again @kgabryje. It looks like that was happening on Dashboards with filters but none applied and it wasn't doing it for the other cases. I made more changes but these feel riskier as I am now controlling the triggering of the query which was set to `true` by default before. This approach needs more testing. Moving this back to draft for now.
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "geido (via GitHub)" <gi...@apache.org>.
geido commented on code in PR #29632:
URL: https://github.com/apache/superset/pull/29632#discussion_r1742216058
##########
superset-frontend/src/dashboard/containers/DashboardPage.tsx:
##########
@@ -192,7 +239,8 @@ export const DashboardPage: FC<PageProps> = ({ idOrSlug }: PageProps) => {
}, [addDangerToast, datasets, datasetsApiError, dispatch]);
if (error) throw error; // caught in error boundary
- if (!readyToRender || !hasDashboardInfoInitiated) return <Loading />;
+
+ const isLoading = !dashboardInfo;
Review Comment:
Must decouple the data that is required to show the charts list from the data required to show other parts of the Dashboard to enable progressive rendering.
--
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
Re: [PR] chore(Dashboard): Remove unnecessary loading [superset]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #29632:
URL: https://github.com/apache/superset/pull/29632#issuecomment-2325025357
@geido Ephemeral environment spinning up at http://52.27.96.20:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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