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/03/24 19:05:00 UTC
[GitHub] [incubator-superset] bkyryliuk opened a new pull request #9370:
[WIP] Add visualization flow to the CTA queries
bkyryliuk opened a new pull request #9370: [WIP] Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370
### CATEGORY
Choose one
- [ ] Bug Fix
- [x] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
Adds a visualization flow for the CTAS queries.
A flow is like this:
* CTAS creates a table on the user behalf
* Clicking on the viz button creates a table definition
* If creation was successful - it opens a exploration page
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
### TEST PLAN
* unit tested
* tested locally
* used in dropbox in production for all of the 2020 Q1
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [x] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
### REVIEWERS
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io edited a comment on issue #9370:
Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-605406369
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=h1) Report
> Merging [#9370](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/5ec0192bccd743374943131d6cae12c813dc198a&el=desc) will **decrease** coverage by `0.19%`.
> The diff coverage is `8.33%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9370 +/- ##
==========================================
- Coverage 58.76% 58.57% -0.20%
==========================================
Files 385 386 +1
Lines 12240 12287 +47
Branches 3022 3025 +3
==========================================
+ Hits 7193 7197 +4
- Misses 4863 4906 +43
Partials 184 184
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/9370/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.48% <0.00%> (-1.49%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9370/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC5qc3g=) | `71.68% <0.00%> (-4.02%)` | :arrow_down: |
| [...src/SqlLab/components/ExploreCtasResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9370/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVDdGFzUmVzdWx0c0J1dHRvbi5qc3g=) | `13.33% <13.33%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=footer). Last update [5ec0192...1e8443a](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r398129307
##########
File path: superset/views/core.py
##########
@@ -1968,6 +1970,48 @@ def sync_druid_source(self):
return json_error_response(utils.error_msg_from_exception(e))
return Response(status=201)
+ @has_access
+ @expose("/sqllab_table_viz/", methods=["POST"])
Review comment:
What happens if you try to create a table that already exists with a different schema?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on issue #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-604742620
@suddjian addressed the comments, thanks!
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull
request #9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r398178814
##########
File path: superset/views/core.py
##########
@@ -1968,6 +1970,48 @@ def sync_druid_source(self):
return json_error_response(utils.error_msg_from_exception(e))
return Response(status=201)
+ @has_access
+ @expose("/sqllab_table_viz/", methods=["POST"])
Review comment:
RE: What happens if you try to create a table that already exists with a different schema?
- it return the found value.
Good suggestion with the name, renamed the endpoint.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on issue #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-611859703
rebased
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] codecov-io commented on issue #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-605406369
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=h1) Report
> Merging [#9370](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/fd227888b6cdb853682eee4393ef7cc3c5383b06&el=desc) will **decrease** coverage by `0.19%`.
> The diff coverage is `8.33%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9370 +/- ##
==========================================
- Coverage 59.01% 58.81% -0.20%
==========================================
Files 378 379 +1
Lines 12162 12209 +47
Branches 3004 3007 +3
==========================================
+ Hits 7177 7181 +4
- Misses 4805 4848 +43
Partials 180 180
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/incubator-superset/pull/9370/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `60.48% <0.00%> (-1.49%)` | :arrow_down: |
| [...erset-frontend/src/SqlLab/components/ResultSet.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9370/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC5qc3g=) | `71.68% <0.00%> (-4.02%)` | :arrow_down: |
| [...src/SqlLab/components/ExploreCtasResultsButton.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9370/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL0V4cGxvcmVDdGFzUmVzdWx0c0J1dHRvbi5qc3g=) | `13.33% <13.33%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=footer). Last update [fd22788...e4d7673](https://codecov.io/gh/apache/incubator-superset/pull/9370?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request
#9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r399209171
##########
File path: superset/views/base.py
##########
@@ -145,6 +156,38 @@ def get_datasource_exist_error_msg(full_name):
return __("Datasource %(name)s already exists", name=full_name)
+def validate_sqlatable(table: models.SqlaTable) -> None:
+ """Checks the table existence in the database."""
+ with db.session.no_autoflush:
+ table_query = db.session.query(models.SqlaTable).filter(
+ models.SqlaTable.table_name == table.table_name,
+ models.SqlaTable.schema == table.schema,
+ models.SqlaTable.database_id == table.database_id,
+ )
+ if db.session.query(table_query.exists()).scalar():
+ raise Exception(get_datasource_exist_error_msg(table.full_name))
+
+ # Fail before adding if the table can't be found
+ try:
+ table.get_sqla_table_object()
+ except Exception as e:
+ logger.exception(f"Got an error in pre_add for {table.name}")
+ raise Exception(
+ _(
+ "Table [{}] could not be found, "
+ "please double check your "
+ "database connection, schema, and "
+ "table name, error: {}"
+ ).format(table.name, str(e))
+ )
Review comment:
This should be `raise Exception(_("Table [%{table}s]..., error: %{error}", table=table_name, error=str(e))` to not burn in the variables into the i18n string.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request
#9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r399204682
##########
File path: superset-frontend/src/SqlLab/components/ResultSet.jsx
##########
@@ -216,18 +217,45 @@ export default class ResultSet extends React.PureComponent {
</Alert>
);
} else if (query.state === 'success' && query.ctas) {
+ // Get the schema of the temporary table that was created.
+ let schema = query.schema;
+ let tempTable = query.tempTable;
+ // Sync queries only have tempTable in query.results.query
+ if (
+ query.results &&
+ query.results.query &&
+ query.results.query.tempTable
+ ) {
+ tempTable = query.results.query.tempTable;
+ }
+ if (tempTable !== undefined) {
+ const tableNameParts = tempTable.split('.');
+ if (tableNameParts.length > 1) {
+ schema = tableNameParts[0];
+ }
Review comment:
In reference to earlier discussions, I think it might make sense to emit an error if a user tries to create a temp table that contains a period until we fix this for good.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on issue #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-605397764
Build is mostly green, cypress hit some rate limiting issues:
```
File "/opt/python/3.6.7/lib/python3.6/urllib/request.py", line 504, in _call_chain
result = func(*args)
File "/opt/python/3.6.7/lib/python3.6/urllib/request.py", line 650, in http_error_default
raise HTTPError(req.full_url, code, msg, hdrs, fp)
urllib.error.HTTPError: HTTP Error 429: too many requests
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on issue #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
villebro commented on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-605400146
@bkyryliuk I can restart the failed job, no problem. If you can fix the translation error I'll make sure cypress goes green.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on issue #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-605405628
Thanks @villebro ! done
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on a change in pull
request #9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r399583578
##########
File path: superset/views/base.py
##########
@@ -145,6 +156,38 @@ def get_datasource_exist_error_msg(full_name):
return __("Datasource %(name)s already exists", name=full_name)
+def validate_sqlatable(table: models.SqlaTable) -> None:
+ """Checks the table existence in the database."""
+ with db.session.no_autoflush:
+ table_query = db.session.query(models.SqlaTable).filter(
+ models.SqlaTable.table_name == table.table_name,
+ models.SqlaTable.schema == table.schema,
+ models.SqlaTable.database_id == table.database_id,
+ )
+ if db.session.query(table_query.exists()).scalar():
+ raise Exception(get_datasource_exist_error_msg(table.full_name))
+
+ # Fail before adding if the table can't be found
+ try:
+ table.get_sqla_table_object()
+ except Exception as e:
+ logger.exception(f"Got an error in pre_add for {table.name}")
+ raise Exception(
+ _(
+ "Table [{}] could not be found, "
+ "please double check your "
+ "database connection, schema, and "
+ "table name, error: {}"
+ ).format(table.name, str(e))
+ )
Review comment:
good catch, thanks!
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on issue #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-612226549
@villebro - anything else I can do to get this PR merged?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r398131060
##########
File path: superset/views/core.py
##########
@@ -1968,6 +1970,48 @@ def sync_druid_source(self):
return json_error_response(utils.error_msg_from_exception(e))
return Response(status=201)
+ @has_access
+ @expose("/sqllab_table_viz/", methods=["POST"])
Review comment:
I think this endpoint should probably have a name that reflects what it does, rather than being sqllab-specific.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r398095648
##########
File path: superset-frontend/src/SqlLab/actions/sqlLab.js
##########
@@ -1249,3 +1249,27 @@ export function createDatasource(vizOptions) {
});
};
}
+
+export function createCtasDatasource(vizOptions) {
+ return dispatch => {
+ dispatch(createDatasourceStarted());
+ return SupersetClient.post({
+ endpoint: '/superset/sqllab_table_viz/',
+ postPayload: { data: vizOptions },
+ })
+ .then(({ json }) => {
+ dispatch(createDatasourceSuccess(json));
+
+ return Promise.resolve(json);
+ })
+ .catch(() => {
+ dispatch(
+ createDatasourceFailed(
+ t('An error occurred while creating the data source'),
+ ),
+ );
+
+ return Promise.reject();
Review comment:
Promises should be rejected with an Error object.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] suddjian commented on a change in pull request
#9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r398095093
##########
File path: superset-frontend/src/SqlLab/actions/sqlLab.js
##########
@@ -1249,3 +1249,27 @@ export function createDatasource(vizOptions) {
});
};
}
+
+export function createCtasDatasource(vizOptions) {
+ return dispatch => {
+ dispatch(createDatasourceStarted());
+ return SupersetClient.post({
+ endpoint: '/superset/sqllab_table_viz/',
+ postPayload: { data: vizOptions },
+ })
+ .then(({ json }) => {
+ dispatch(createDatasourceSuccess(json));
+
+ return Promise.resolve(json);
Review comment:
Promise.resolve() is unnecessary when you're in a promise method.
```suggestion
return json;
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro commented on a change in pull request
#9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#discussion_r399623123
##########
File path: superset/views/base.py
##########
@@ -145,6 +156,38 @@ def get_datasource_exist_error_msg(full_name):
return __("Datasource %(name)s already exists", name=full_name)
+def validate_sqlatable(table: models.SqlaTable) -> None:
+ """Checks the table existence in the database."""
+ with db.session.no_autoflush:
+ table_query = db.session.query(models.SqlaTable).filter(
+ models.SqlaTable.table_name == table.table_name,
+ models.SqlaTable.schema == table.schema,
+ models.SqlaTable.database_id == table.database_id,
+ )
+ if db.session.query(table_query.exists()).scalar():
+ raise Exception(get_datasource_exist_error_msg(table.full_name))
+
+ # Fail before adding if the table can't be found
+ try:
+ table.get_sqla_table_object()
+ except Exception as e:
+ logger.exception(f"Got an error in pre_add for {table.name}")
+ raise Exception(
+ _(
+ "Table [%{table}s] could not be found, "
+ "please double check your "
+ "database connection, schema, and "
+ "table name, %{error}"
+ ).format(table=table.name, error=str(e))
Review comment:
```suggestion
_(
"Table [%{table}s] could not be found, "
"please double check your "
"database connection, schema, and "
"table name, %{error}s"
).format(table=table.name, error=str(e))
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] bkyryliuk commented on issue #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on issue #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-605397677
thanks @villebro - resolved the comments. Yeah it is cleaner to use the tmpSchema
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] villebro merged pull request #9370: Add
visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
villebro merged pull request #9370: Add visualization flow to the CTA queries
URL: https://github.com/apache/incubator-superset/pull/9370
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [incubator-superset] ktmud commented on pull request #9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #9370:
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-686889776
@bkyryliuk is there a reason why `ExploreCtasResultsButto.jsx` and `ExploreCtasResultsButton_spec.jsx` are placed under `superset/assets` instead of `superset-frontend` ?
----------------------------------------------------------------
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] bkyryliuk commented on pull request #9370: Add visualization flow to the CTA queries
Posted by GitBox <gi...@apache.org>.
bkyryliuk commented on pull request #9370:
URL: https://github.com/apache/incubator-superset/pull/9370#issuecomment-687238206
> @bkyryliuk is there a reason why `ExploreCtasResultsButto.jsx` and `ExploreCtasResultsButton_spec.jsx` are placed under `superset/assets` instead of `superset-frontend` ?
Nope, probably my miss.
----------------------------------------------------------------
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