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 2022/05/13 13:01:31 UTC

[GitHub] [superset] cccs-Dustin opened a new pull request, #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

cccs-Dustin opened a new pull request, #20056:
URL: https://github.com/apache/superset/pull/20056

   <!---
   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 -->
   In Superset's config file, there is a config option called `DEFAULT_VIZ_TYPE` which you can use to define which viz you would like to use in the chart explorer by default. This is a very useful config option, especially when you have a custom viz you would like to use instead of the Superset `Table` viz. However, when you are in SQL Lab and you want to create and explore a dataset, there is currently no way to to change the default viz which will be used. 
   
   This PR allows for the `DEFAULT_VIZ_TYPE` config option to also be used by SQL Lab when you want to explore a dataset. This allows for consistency between the chart explorer, and exploring through SQL Lab.
   
   The changes include modifying the front-end index file so that instead of hard coding the `table` viz, it uses a blank string for `viz_type`. This allows the back-end to change the viz_type to be whatever the `DEFAULT_VIZ_TYPE` config option is.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   N/A
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   1. Modify the config file so that the `DEFAULT_VIZ_TYPE` is a different viz from the standard `Table`.
   2. Within Superset, select "SQL Lab" from the top menu.
   3. Select "SQL Editor" from the drop down menu which was opened during step 2.
   4. On the left side of the page, select a database and schema that you would like to run the test query on.
   5. For testing purposes, add some basic SQL to run against a table (e.g., `SELECT * FROM table_1;`).
   6. Select the `Run` button which will run the query.
   7. Once the results are displayed under the "Results" tab, select the `Explore` button to explore the result set in the data exploration view.
   8. Once the exploration view opens, you should see the viz type you defined in step 1 selected in the "Visualization Type" section.
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [X] 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
   - [X] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   I also created a new discussion for this feature, it can be found here: 


-- 
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] rusackas commented on pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #20056:
URL: https://github.com/apache/superset/pull/20056#issuecomment-1162441899

   > @cccs-Dustin Thanks for the contribution.
   > 
   > Many charts need to provide how to **aggregate**, in other words, you have to provide appropriate metric(s) or appropriate dimension. This is why datasets created from SQLLab are displayed in non-aggregated tables by default.
   > 
   > a specific example: a dataset without a time dimension cannot generate a timeseries chart.
   
   I think this is OK for their use case... they want to replace the table with a different non-aggregated chart. This should allow for that use case without affecting other users/installations of Superset. 
   
   If people want to set some other aggregated chart as their default, it'll be more difficult indeed, but that's not the intent of the PR here.


-- 
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] cccs-Dustin commented on a diff in pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
cccs-Dustin commented on code in PR #20056:
URL: https://github.com/apache/superset/pull/20056#discussion_r892242874


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -343,7 +343,7 @@ export default class ResultSet extends React.PureComponent<
           metrics: [],
           groupby: [],
           time_range: 'No filter',
-          viz_type: 'table',
+          viz_type: '',
           all_columns: selectedColumns.map(c => c.name),

Review Comment:
   Modified the code in the most resent commit such that the default form data is populated in the backend (follows along with the proposed option 2)



-- 
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] zhaoyongjie commented on pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on PR #20056:
URL: https://github.com/apache/superset/pull/20056#issuecomment-1157335334

   @cccs-Dustin Thanks for the contribution. 
   
   Many charts need to provide how to **aggregate**, in other words, you have to provide appropriate metric(s). This is why datasets created from SQLLab are displayed in non-aggregated tables by default. 
   
   a specific example: a dataset without a time dimension cannot generate a timeseries chart. 


-- 
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] rusackas commented on pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
rusackas commented on PR #20056:
URL: https://github.com/apache/superset/pull/20056#issuecomment-1162442027

   /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


[GitHub] [superset] codecov[bot] commented on pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20056:
URL: https://github.com/apache/superset/pull/20056#issuecomment-1126040377

   # [Codecov](https://codecov.io/gh/apache/superset/pull/20056?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 [#20056](https://codecov.io/gh/apache/superset/pull/20056?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1b9c7d0) into [master](https://codecov.io/gh/apache/superset/commit/f144de4ee2bf213bb7e17f903bd3975d504c4136?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f144de4) will **decrease** coverage by `12.07%`.
   > The diff coverage is `54.14%`.
   
   > :exclamation: Current head 1b9c7d0 differs from pull request most recent head 500512d. Consider uploading reports for the commit 500512d to get more accurate results
   
   ```diff
   @@             Coverage Diff             @@
   ##           master   #20056       +/-   ##
   ===========================================
   - Coverage   66.29%   54.22%   -12.08%     
   ===========================================
     Files        1712     1712               
     Lines       63962    64099      +137     
     Branches     6731     6744       +13     
   ===========================================
   - Hits        42404    34757     -7647     
   - Misses      19847    27629     +7782     
   - Partials     1711     1713        +2     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | hive | `?` | |
   | mysql | `?` | |
   | postgres | `?` | |
   | presto | `53.52% <44.49%> (+0.01%)` | :arrow_up: |
   | python | `57.34% <45.33%> (-25.14%)` | :arrow_down: |
   | sqlite | `?` | |
   | unit | `49.02% <39.83%> (+0.39%)` | :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/20056?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ugins/legacy-plugin-chart-calendar/src/Calendar.js](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LWNhbGVuZGFyL3NyYy9DYWxlbmRhci5qcw==) | `0.00% <ø> (ø)` | |
   | [...legacy-plugin-chart-calendar/src/ReactCalendar.jsx](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LWNhbGVuZGFyL3NyYy9SZWFjdENhbGVuZGFyLmpzeA==) | `0.00% <0.00%> (ø)` | |
   | [...egacy-plugin-chart-world-map/src/ReactWorldMap.jsx](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvUmVhY3RXb3JsZE1hcC5qc3g=) | `0.00% <0.00%> (ø)` | |
   | [...gins/legacy-plugin-chart-world-map/src/WorldMap.js](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcGx1Z2luLWNoYXJ0LXdvcmxkLW1hcC9zcmMvV29ybGRNYXAuanM=) | `0.00% <ø> (ø)` | |
   | [...acy-preset-chart-deckgl/src/components/Tooltip.tsx](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9sZWdhY3ktcHJlc2V0LWNoYXJ0LWRlY2tnbC9zcmMvY29tcG9uZW50cy9Ub29sdGlwLnRzeA==) | `0.00% <0.00%> (ø)` | |
   | [.../plugin-chart-echarts/src/Funnel/transformProps.ts](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvRnVubmVsL3RyYW5zZm9ybVByb3BzLnRz) | `71.73% <ø> (ø)` | |
   | [...hart-echarts/src/MixedTimeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL3RyYW5zZm9ybVByb3BzLnRz) | `0.00% <ø> (ø)` | |
   | [...ins/plugin-chart-echarts/src/Pie/transformProps.ts](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvUGllL3RyYW5zZm9ybVByb3BzLnRz) | `55.07% <ø> (ø)` | |
   | [...s/plugin-chart-echarts/src/Radar/transformProps.ts](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvUmFkYXIvdHJhbnNmb3JtUHJvcHMudHM=) | `0.00% <ø> (ø)` | |
   | [...gin-chart-echarts/src/Timeseries/transformProps.ts](https://codecov.io/gh/apache/superset/pull/20056/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-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvVGltZXNlcmllcy90cmFuc2Zvcm1Qcm9wcy50cw==) | `57.89% <ø> (ø)` | |
   | ... and [363 more](https://codecov.io/gh/apache/superset/pull/20056/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/20056?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/20056?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 [f144de4...500512d](https://codecov.io/gh/apache/superset/pull/20056?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] github-actions[bot] commented on pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20056:
URL: https://github.com/apache/superset/pull/20056#issuecomment-1163419479

   Ephemeral environment shutdown and build artifacts deleted.


-- 
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] github-actions[bot] commented on pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20056:
URL: https://github.com/apache/superset/pull/20056#issuecomment-1162443124

   @rusackas Ephemeral environment spinning up at http://35.89.223.60: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


[GitHub] [superset] rusackas commented on a diff in pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
rusackas commented on code in PR #20056:
URL: https://github.com/apache/superset/pull/20056#discussion_r889286338


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -343,7 +343,7 @@ export default class ResultSet extends React.PureComponent<
           metrics: [],
           groupby: [],
           time_range: 'No filter',
-          viz_type: 'table',
+          viz_type: '',

Review Comment:
   Looks like you went with door #2. Resolving this comment :)



-- 
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] villebro commented on a diff in pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
villebro commented on code in PR #20056:
URL: https://github.com/apache/superset/pull/20056#discussion_r875896912


##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -343,7 +343,7 @@ export default class ResultSet extends React.PureComponent<
           metrics: [],
           groupby: [],
           time_range: 'No filter',
-          viz_type: 'table',
+          viz_type: '',

Review Comment:
   In bootstrap data we have `common.conf` which contains `DEFAULT_VIZ_TYPE` (you can check the page source and look for `data-bootstrap="..."`). Instead of passing this as an empty string, I would propose one of the following:
   1. passing the `DEFAULT_VIZ_TYPE` to this component from wherever the bootstrap data is available, or
   2. Removing it all together, and then just replacing it with `DEFAULT_VIZ_TYPE` is if's undefined in the backend.



##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -343,7 +343,7 @@ export default class ResultSet extends React.PureComponent<
           metrics: [],
           groupby: [],
           time_range: 'No filter',
-          viz_type: 'table',
+          viz_type: '',
           all_columns: selectedColumns.map(c => c.name),

Review Comment:
   The `all_columns` prop is mostly specific to the table chart, so we may need to consider if this logic also needs to be made configurable. Since it will be very difficult to define a callback in Python that gets executed on the frontend, it may be better do also populate the default form data in the backend (option 2)



-- 
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] rusackas merged pull request #20056: feat(SQL Lab): Make SQL Lab explore use the default viz from the config file

Posted by GitBox <gi...@apache.org>.
rusackas merged PR #20056:
URL: https://github.com/apache/superset/pull/20056


-- 
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