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 2020/07/02 02:47:11 UTC
[GitHub] [incubator-superset] etr2460 opened a new pull request #10226: chore: type ResultSet.tsx
etr2460 opened a new pull request #10226:
URL: https://github.com/apache/incubator-superset/pull/10226
### SUMMARY
As titled. Also includes a couple small refactors/bug fixes that i came across as I was working
### TEST PLAN
CI
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
to: @ktmud @graceguo-supercat @nytai @bkyryliuk
----------------------------------------------------------------
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.
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] [incubator-superset] etr2460 merged pull request #10226: chore: type ResultSet.tsx
Posted by GitBox <gi...@apache.org>.
etr2460 merged pull request #10226:
URL: https://github.com/apache/incubator-superset/pull/10226
----------------------------------------------------------------
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.
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] [incubator-superset] etr2460 commented on pull request #10226: chore: type ResultSet.tsx
Posted by GitBox <gi...@apache.org>.
etr2460 commented on pull request #10226:
URL: https://github.com/apache/incubator-superset/pull/10226#issuecomment-652804004
Thanks for the quick stamps! I'm going to wait for @bkyryliuk to take a look as well since the CTAS logic was the main functional change here and i want to make sure it looks good to him
----------------------------------------------------------------
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.
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] [incubator-superset] etr2460 commented on a change in pull request #10226: chore: type ResultSet.tsx
Posted by GitBox <gi...@apache.org>.
etr2460 commented on a change in pull request #10226:
URL: https://github.com/apache/incubator-superset/pull/10226#discussion_r448718360
##########
File path: superset-frontend/spec/javascripts/sqllab/fixtures.js
##########
@@ -387,7 +387,7 @@ export const stoppedQuery = {
startDttm: 1497400851936,
state: 'stopped',
tab: 'Untitled Query 2',
- tempTableName: '',
+ tempTable: '',
Review comment:
I aligned on this name everywhere because that's how the backend sends it
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.tsx
##########
@@ -235,20 +236,21 @@ export default class ResultSet extends React.PureComponent {
<Alert bsStyle="info">
{t(object)} [
<strong>
- {tmpSchema}.{tmpTable}
+ {tempSchema ? `${tempSchema}.` : ''}
Review comment:
When testing on sqllite, there wasn't always a schema, so i cleaned up the cases where schema can be null
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.tsx
##########
@@ -218,14 +226,7 @@ export default class ResultSet extends React.PureComponent {
</Alert>
);
} else if (query.state === 'success' && query.ctas) {
- // Async queries
- let tmpSchema = query.tempSchema;
- let tmpTable = query.tempTableName;
- // Sync queries, query.results.query contains the source of truth for them.
- if (query.results && query.results.query) {
- tmpTable = query.results.query.tempTable;
- tmpSchema = query.results.query.tempSchema;
- }
Review comment:
this logic is moved to the reducer, it seems cleaner (although it's a big janky to have the query in 2 different places 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.
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