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/07/25 17:23:50 UTC
[GitHub] [superset] lyndsiWilliams opened a new pull request, #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
lyndsiWilliams opened a new pull request, #20852:
URL: https://github.com/apache/superset/pull/20852
<!---
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 -->
This changes the current save query button into a split save button if the database has "Allow this database to be explored" enabled. When the user clicks the caret section of the button, they can now save the query as a dataset.
`Bonus`: In order to create this button, I conveniently needed to fix the styling on the "Run" query split button. The split line now goes all the way through the button and the caret is centered.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
#### BEFORE:
#### AFTER:
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
- From SqlLab, open a query for a database that has "Allow this database to be explored" enabled.
- Observe the new split save button
- Click the "Save" section of the button to see the save query modal
- Click the caret `v` section of the button, then click "Save dataset" to see the save dataset modal
- Both of these modals should be fully functional
- From SqlLab, open a query for a database that does NOT have "Allow this database to be explored" enabled.
- Observe that the save button is a normal button, not split
- Clicking this button should show the save query modal
### 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:
- [x] 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] hughhhh commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929130416
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -275,11 +250,19 @@ export default class ResultSet extends React.PureComponent<
this.props.database?.allows_virtual_table_explore && (
<ExploreResultsButton
database={this.props.database}
- onClick={() => this.setState({ showSaveDatasetModal: true })}
Review Comment:
add back this line
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -275,11 +250,19 @@ export default class ResultSet extends React.PureComponent<
this.props.database?.allows_virtual_table_explore && (
<ExploreResultsButton
database={this.props.database}
- onClick={() => this.setState({ showSaveDatasetModal: true })}
+ onClick={() => {
+ // There is currently redux / state issue where sometimes a query will have serverId
+ // and other times it will not. We need this attribute consistently for this to work
+ // const qid = this.props?.query?.results?.query_id;
+ // if (qid) {
+ // // This will open explore using the query as datasource
+ // window.location.href = `/explore/?dataset_type=query&dataset_id=${qid}`;
+ // } else {
+ // this.setState({ showSaveDatasetModal: true });
+ // }
+ this.setState({ showSaveDatasetModal: true });
+ }}
/>
- // In order to use the new workflow for a query powered chart, replace the
Review Comment:
add back these lines
--
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] lyndsiWilliams commented on pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1200029632
/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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929237795
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -275,11 +250,19 @@ export default class ResultSet extends React.PureComponent<
this.props.database?.allows_virtual_table_explore && (
<ExploreResultsButton
database={this.props.database}
- onClick={() => this.setState({ showSaveDatasetModal: true })}
Review Comment:
Added back in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -275,11 +250,19 @@ export default class ResultSet extends React.PureComponent<
this.props.database?.allows_virtual_table_explore && (
<ExploreResultsButton
database={this.props.database}
- onClick={() => this.setState({ showSaveDatasetModal: true })}
+ onClick={() => {
+ // There is currently redux / state issue where sometimes a query will have serverId
+ // and other times it will not. We need this attribute consistently for this to work
+ // const qid = this.props?.query?.results?.query_id;
+ // if (qid) {
+ // // This will open explore using the query as datasource
+ // window.location.href = `/explore/?dataset_type=query&dataset_id=${qid}`;
+ // } else {
+ // this.setState({ showSaveDatasetModal: true });
+ // }
+ this.setState({ showSaveDatasetModal: true });
+ }}
/>
- // In order to use the new workflow for a query powered chart, replace the
Review Comment:
Added back in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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] AAfghahi commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929150695
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -29,19 +29,14 @@ import {
SaveDatasetModal,
} from 'src/SqlLab/components/SaveDatasetModal';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
-import { EXPLORE_CHART_DEFAULT } from 'src/SqlLab/types';
Review Comment:
I think this was lost in a rebase potentially? Please revert this.
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -136,6 +139,12 @@ class Chart extends React.PureComponent {
}
}
+ toggleSaveDatasetModal = () => {
Review Comment:
same
##########
superset-frontend/src/SqlLab/components/SaveQuery/index.tsx:
##########
@@ -118,9 +129,7 @@ export default function SaveQuery({
setDescription(e.target.value);
};
- const toggleSave = () => {
- setShowSave(!showSave);
- };
+ const toggleSave = () => setShowSave(!showSave);
Review Comment:
nit: I think that this would be better taking in a boolean value that we set. Sometimes when connections are slower, the action doesn't immediately render and users click multiple times, causing it to reverse back to original value.
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -122,8 +123,10 @@ const MonospaceDiv = styled.div`
class Chart extends React.PureComponent {
constructor(props) {
super(props);
+ this.state = { showSaveDatasetModal: false };
Review Comment:
same here
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -275,11 +250,19 @@ export default class ResultSet extends React.PureComponent<
this.props.database?.allows_virtual_table_explore && (
<ExploreResultsButton
database={this.props.database}
- onClick={() => this.setState({ showSaveDatasetModal: true })}
+ onClick={() => {
+ // There is currently redux / state issue where sometimes a query will have serverId
+ // and other times it will not. We need this attribute consistently for this to work
+ // const qid = this.props?.query?.results?.query_id;
+ // if (qid) {
+ // // This will open explore using the query as datasource
+ // window.location.href = `/explore/?dataset_type=query&dataset_id=${qid}`;
+ // } else {
+ // this.setState({ showSaveDatasetModal: true });
+ // }
+ this.setState({ showSaveDatasetModal: true });
+ }}
/>
- // In order to use the new workflow for a query powered chart, replace the
Review Comment:
more of a broader question, when do we make this functionality the default behavior?
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -253,6 +262,7 @@ class Chart extends React.PureComponent {
width,
} = this.props;
+ const { showSaveDatasetModal } = this.state;
Review Comment:
same as above
##########
superset-frontend/src/components/Chart/ChartErrorMessage.tsx:
##########
@@ -42,6 +42,5 @@ export const ChartErrorMessage: React.FC<Props> = ({
...error,
extra: { ...error.extra, owners },
};
-
Review Comment:
nit: add this space back?
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -218,26 +213,6 @@ export default class ResultSet extends React.PureComponent<
}
}
- createExploreResultsOnClick = async () => {
Review Comment:
Also this
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -25,6 +25,7 @@ import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
import Loading from 'src/components/Loading';
import { EmptyStateBig } from 'src/components/EmptyState';
import ErrorBoundary from 'src/components/ErrorBoundary';
+import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
Review Comment:
I believe that this is part of an older rebase, and is no longer in this portion of the code base.
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -296,27 +306,39 @@ class Chart extends React.PureComponent {
}
return (
- <ErrorBoundary
- onError={this.handleRenderContainerFailure}
- showMessage={false}
- >
- <Styles
- data-ui-anchor="chart"
- className="chart-container"
- data-test="chart-container"
- height={height}
- width={width}
+ <>
+ <ErrorBoundary
+ onError={this.handleRenderContainerFailure}
+ showMessage={false}
>
- <div className="slice_container" data-test="slice-container">
- <ChartRenderer
- {...this.props}
- source={this.props.dashboardId ? 'dashboard' : 'explore'}
- data-test={this.props.vizType}
- />
- </div>
- {isLoading && !isDeactivatedViz && <Loading />}
- </Styles>
- </ErrorBoundary>
+ <Styles
+ data-ui-anchor="chart"
+ className="chart-container"
+ data-test="chart-container"
+ height={height}
+ width={width}
+ >
+ <div className="slice_container" data-test="slice-container">
+ <ChartRenderer
+ {...this.props}
+ source={this.props.dashboardId ? 'dashboard' : 'explore'}
+ data-test={this.props.vizType}
+ />
+ </div>
+ {isLoading && !isDeactivatedViz && <Loading />}
+ </Styles>
+ </ErrorBoundary>
+ {showSaveDatasetModal && (
+ <SaveDatasetModal
+ key={Math.random()}
Review Comment:
I think the SaveDatasetModal can be deleted from this, we ended up invoking it in another portion of the code.
--
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 #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1199994597
@lyndsiWilliams Ephemeral environment spinning up at http://54.202.3.230: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] lyndsiWilliams commented on pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1200009132
/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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929241835
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -122,8 +123,10 @@ const MonospaceDiv = styled.div`
class Chart extends React.PureComponent {
constructor(props) {
super(props);
+ this.state = { showSaveDatasetModal: false };
Review Comment:
Removed in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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] hughhhh commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929132309
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -296,27 +306,39 @@ class Chart extends React.PureComponent {
}
return (
- <ErrorBoundary
- onError={this.handleRenderContainerFailure}
- showMessage={false}
- >
- <Styles
- data-ui-anchor="chart"
- className="chart-container"
- data-test="chart-container"
- height={height}
- width={width}
+ <>
+ <ErrorBoundary
+ onError={this.handleRenderContainerFailure}
+ showMessage={false}
>
- <div className="slice_container" data-test="slice-container">
- <ChartRenderer
- {...this.props}
- source={this.props.dashboardId ? 'dashboard' : 'explore'}
- data-test={this.props.vizType}
- />
- </div>
- {isLoading && !isDeactivatedViz && <Loading />}
- </Styles>
- </ErrorBoundary>
+ <Styles
+ data-ui-anchor="chart"
+ className="chart-container"
+ data-test="chart-container"
+ height={height}
+ width={width}
+ >
+ <div className="slice_container" data-test="slice-container">
+ <ChartRenderer
+ {...this.props}
+ source={this.props.dashboardId ? 'dashboard' : 'explore'}
+ data-test={this.props.vizType}
+ />
+ </div>
+ {isLoading && !isDeactivatedViz && <Loading />}
+ </Styles>
+ </ErrorBoundary>
+ {showSaveDatasetModal && (
+ <SaveDatasetModal
+ key={Math.random()}
Review Comment:
why are we doing a `Math.random()` here for the key? couldn't we just set it as specific value?
--
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] hughhhh commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929131502
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx:
##########
@@ -55,9 +63,732 @@ const mockedProps = {
datasource: testQuery,
};
-describe('SaveDatasetModal RTL', () => {
- it('renders a "Save as new" field', () => {
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+const testDatasetList = {
Review Comment:
can we put this in a separate fixture file?
--
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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929238027
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -275,11 +250,19 @@ export default class ResultSet extends React.PureComponent<
this.props.database?.allows_virtual_table_explore && (
<ExploreResultsButton
database={this.props.database}
- onClick={() => this.setState({ showSaveDatasetModal: true })}
+ onClick={() => {
+ // There is currently redux / state issue where sometimes a query will have serverId
+ // and other times it will not. We need this attribute consistently for this to work
+ // const qid = this.props?.query?.results?.query_id;
+ // if (qid) {
+ // // This will open explore using the query as datasource
+ // window.location.href = `/explore/?dataset_type=query&dataset_id=${qid}`;
+ // } else {
+ // this.setState({ showSaveDatasetModal: true });
+ // }
+ this.setState({ showSaveDatasetModal: true });
+ }}
Review Comment:
Removed in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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 #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1198431085
@lyndsiWilliams Ephemeral environment spinning up at http://52.25.149.118: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] github-actions[bot] commented on pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1200010771
@lyndsiWilliams Ephemeral environment spinning up at http://54.185.46.90: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] codecov[bot] commented on pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1194695244
# [Codecov](https://codecov.io/gh/apache/superset/pull/20852?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 [#20852](https://codecov.io/gh/apache/superset/pull/20852?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (61bdc42) into [master](https://codecov.io/gh/apache/superset/commit/e3c6380258a46467ac8e09e64db60aad46bc4655?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e3c6380) will **decrease** coverage by `11.68%`.
> The diff coverage is `56.52%`.
> :exclamation: Current head 61bdc42 differs from pull request most recent head e6bdab5. Consider uploading reports for the commit e6bdab5 to get more accurate results
```diff
@@ Coverage Diff @@
## master #20852 +/- ##
===========================================
- Coverage 66.30% 54.62% -11.69%
===========================================
Files 1757 1757
Lines 66860 66860
Branches 7077 7077
===========================================
- Hits 44332 36522 -7810
- Misses 20719 28529 +7810
Partials 1809 1809
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `53.12% <ø> (ø)` | |
| python | `57.46% <ø> (-24.13%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `50.26% <ø> (ø)` | |
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/20852?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...frontend/src/SqlLab/components/ResultSet/index.tsx](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC9pbmRleC50c3g=) | `51.74% <ø> (ø)` | |
| [...d/src/SqlLab/components/SaveDatasetModal/index.tsx](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NhdmVEYXRhc2V0TW9kYWwvaW5kZXgudHN4) | `35.29% <0.00%> (ø)` | |
| [...frontend/src/SqlLab/components/SqlEditor/index.jsx](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NxbEVkaXRvci9pbmRleC5qc3g=) | `51.36% <0.00%> (ø)` | |
| [...d/src/SqlLab/components/TabbedSqlEditors/index.jsx](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMvaW5kZXguanN4) | `56.11% <ø> (ø)` | |
| [superset-frontend/src/SqlLab/fixtures.ts](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9maXh0dXJlcy50cw==) | `100.00% <ø> (ø)` | |
| [...et-frontend/src/SqlLab/reducers/getInitialState.js](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | `46.42% <ø> (ø)` | |
| [superset-frontend/src/SqlLab/reducers/sqlLab.js](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9zcWxMYWIuanM=) | `33.33% <ø> (ø)` | |
| [superset-frontend/src/SqlLab/types.ts](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi90eXBlcy50cw==) | `57.14% <ø> (ø)` | |
| [...t-frontend/src/components/DropdownButton/index.tsx](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRHJvcGRvd25CdXR0b24vaW5kZXgudHN4) | `10.00% <ø> (ø)` | |
| [...frontend/src/SqlLab/components/SaveQuery/index.tsx](https://codecov.io/gh/apache/superset/pull/20852/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NhdmVRdWVyeS9pbmRleC50c3g=) | `57.89% <40.00%> (ø)` | |
| ... and [303 more](https://codecov.io/gh/apache/superset/pull/20852/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) | |
Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?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] lyndsiWilliams merged pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams merged PR #20852:
URL: https://github.com/apache/superset/pull/20852
--
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] lyndsiWilliams commented on pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1198391900
/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] lyndsiWilliams commented on pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1199991091
/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] github-actions[bot] commented on pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1200031032
@lyndsiWilliams Ephemeral environment spinning up at http://35.165.194.84: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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929242000
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -136,6 +139,12 @@ class Chart extends React.PureComponent {
}
}
+ toggleSaveDatasetModal = () => {
Review Comment:
Removed in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -253,6 +262,7 @@ class Chart extends React.PureComponent {
width,
} = this.props;
+ const { showSaveDatasetModal } = this.state;
Review Comment:
Removed in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929242228
##########
superset-frontend/src/components/Chart/ChartErrorMessage.tsx:
##########
@@ -42,6 +42,5 @@ export const ChartErrorMessage: React.FC<Props> = ({
...error,
extra: { ...error.extra, owners },
};
-
Review Comment:
Weird rebase stuff haha! Added back in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929238290
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx:
##########
@@ -55,9 +63,732 @@ const mockedProps = {
datasource: testQuery,
};
-describe('SaveDatasetModal RTL', () => {
- it('renders a "Save as new" field', () => {
- render(<SaveDatasetModal {...mockedProps} />, { useRedux: true });
+const testDatasetList = {
Review Comment:
Yeah, once I'm done fixing this test file I'll be cleaning it 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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929239550
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -25,6 +25,7 @@ import { PLACEHOLDER_DATASOURCE } from 'src/dashboard/constants';
import Loading from 'src/components/Loading';
import { EmptyStateBig } from 'src/components/EmptyState';
import ErrorBoundary from 'src/components/ErrorBoundary';
+import { SaveDatasetModal } from 'src/SqlLab/components/SaveDatasetModal';
Review Comment:
Removed in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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 #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #20852:
URL: https://github.com/apache/superset/pull/20852#issuecomment-1201630290
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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929239346
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -29,19 +29,14 @@ import {
SaveDatasetModal,
} from 'src/SqlLab/components/SaveDatasetModal';
import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
-import { EXPLORE_CHART_DEFAULT } from 'src/SqlLab/types';
Review Comment:
Yeah there seems to be some weird rebase stuff on this PR, thanks for catching all these! Re-added in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -218,26 +213,6 @@ export default class ResultSet extends React.PureComponent<
}
}
- createExploreResultsOnClick = async () => {
Review Comment:
Re-added in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929237795
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -275,11 +250,19 @@ export default class ResultSet extends React.PureComponent<
this.props.database?.allows_virtual_table_explore && (
<ExploreResultsButton
database={this.props.database}
- onClick={() => this.setState({ showSaveDatasetModal: true })}
Review Comment:
Added back in [`this commit`](https://github.com/apache/superset/pull/20852/commits/ed4391fb65f65d714c10080db5e44b4870c61d22).
--
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] hughhhh commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
hughhhh commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929130717
##########
superset-frontend/src/SqlLab/components/ResultSet/index.tsx:
##########
@@ -275,11 +250,19 @@ export default class ResultSet extends React.PureComponent<
this.props.database?.allows_virtual_table_explore && (
<ExploreResultsButton
database={this.props.database}
- onClick={() => this.setState({ showSaveDatasetModal: true })}
+ onClick={() => {
+ // There is currently redux / state issue where sometimes a query will have serverId
+ // and other times it will not. We need this attribute consistently for this to work
+ // const qid = this.props?.query?.results?.query_id;
+ // if (qid) {
+ // // This will open explore using the query as datasource
+ // window.location.href = `/explore/?dataset_type=query&dataset_id=${qid}`;
+ // } else {
+ // this.setState({ showSaveDatasetModal: true });
+ // }
+ this.setState({ showSaveDatasetModal: true });
+ }}
Review Comment:
remove these lines
--
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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929238679
##########
superset-frontend/src/components/Chart/Chart.jsx:
##########
@@ -296,27 +306,39 @@ class Chart extends React.PureComponent {
}
return (
- <ErrorBoundary
- onError={this.handleRenderContainerFailure}
- showMessage={false}
- >
- <Styles
- data-ui-anchor="chart"
- className="chart-container"
- data-test="chart-container"
- height={height}
- width={width}
+ <>
+ <ErrorBoundary
+ onError={this.handleRenderContainerFailure}
+ showMessage={false}
>
- <div className="slice_container" data-test="slice-container">
- <ChartRenderer
- {...this.props}
- source={this.props.dashboardId ? 'dashboard' : 'explore'}
- data-test={this.props.vizType}
- />
- </div>
- {isLoading && !isDeactivatedViz && <Loading />}
- </Styles>
- </ErrorBoundary>
+ <Styles
+ data-ui-anchor="chart"
+ className="chart-container"
+ data-test="chart-container"
+ height={height}
+ width={width}
+ >
+ <div className="slice_container" data-test="slice-container">
+ <ChartRenderer
+ {...this.props}
+ source={this.props.dashboardId ? 'dashboard' : 'explore'}
+ data-test={this.props.vizType}
+ />
+ </div>
+ {isLoading && !isDeactivatedViz && <Loading />}
+ </Styles>
+ </ErrorBoundary>
+ {showSaveDatasetModal && (
+ <SaveDatasetModal
+ key={Math.random()}
Review Comment:
This was removed in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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] lyndsiWilliams commented on a diff in pull request #20852: feat(SqlLab): Change Save Dataset Button to Split Save Query Button IV
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on code in PR #20852:
URL: https://github.com/apache/superset/pull/20852#discussion_r929238853
##########
superset-frontend/src/SqlLab/components/SaveQuery/index.tsx:
##########
@@ -118,9 +129,7 @@ export default function SaveQuery({
setDescription(e.target.value);
};
- const toggleSave = () => {
- setShowSave(!showSave);
- };
+ const toggleSave = () => setShowSave(!showSave);
Review Comment:
Good call, I cleaned this up in [`this commit`](https://github.com/apache/superset/pull/20852/commits/61bdc42398bc3e8f458d1ba27536a4322b8e5097).
--
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