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/03/01 22:01:08 UTC

[GitHub] [superset] cancan101 opened a new issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

cancan101 opened a new issue #18993:
URL: https://github.com/apache/superset/issues/18993


   When this error handler is hit:
   https://github.com/dpgaspar/Flask-AppBuilder/blob/2d505e819d496308a34c5f623d70331a37599cc4/flask_appbuilder/api/__init__.py#L145-L146 in the `superset.views.datasource.views.Datasource` view, a a further Exception is raised as there is no 400 handler for that view:
   ```
   Traceback (most recent call last):
     File "/Users/alex/.pyenv/versions/3.7.3/envs/app/lib/python3.7/site-packages/superset/views/base.py", line 203, in wraps
       return f(self, *args, **kwargs)
     File "/Users/alex/.pyenv/versions/3.7.3/envs/app/lib/python3.7/site-packages/flask_appbuilder/api/__init__.py", line 145, in wraps
       return self.response_400(
   AttributeError: 'Datasource' object has no attribute 'response_400'
   ```
   
   #### How to reproduce the bug
   
   1. Use legacy datasource editor
   2. Define a table with an `=` in its name
   3. Click sync columns from source
   
   ### Expected results
   
   Ideally it should handle the table name (it looks like the string is not properly `rison` encoded. If it isn't going to allow these strings, then at the very least the 400 error should be properly generated
   
   what you expected to happen.
   
   ### Actual results
   
   A toast showing: "'Datasource' object has no attribute 'response_400'"
   
   what actually happens.
   
   #### Screenshots
   The dataset:
   ![image](https://user-images.githubusercontent.com/51059/156256040-4b71d68e-20af-4cf4-9b5f-709084f0b405.png)
   
   The error toast:
   ![image](https://user-images.githubusercontent.com/51059/156256007-1ebc7578-332f-4d9b-9405-1411cce4f22a.png)
   
   
   ### Environment
   
   (please complete the following information):
   
   - browser type and version: Chrome 98.0.4758.102 
   - superset version: `Superset 1.4.1`
   - python version: `Python 3.7.3`
   - node.js version: `v15.4.0`
   - any feature flags active: n/a
   
   ### Checklist
   
   Make sure to follow these steps before submitting your issue - thank you!
   
   - [x] I have checked the superset logs for python stacktraces and included it here as text if there are any.
   - [x] I have reproduced the issue with at least the latest released version of superset.
   - [x] I have checked the issue tracker for the same issue and I haven't found one similar.
   
   ### Additional context
   
   Add any other context about the problem 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] cancan101 commented on issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
cancan101 commented on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061093829


   So it looks like https://github.com/apache/superset/pull/18056 did fix the rison encoding meaning that the 400 error is no longer generated by that code path. That being said, if something else does generate a 400 error at some point the view does not correctly handle it and will then raise an error that `response_400` is not defined. This can be tests using a CLI tool and `GET`ing against `/datasource/external_metadata_by_name/` with `q=<invalid rison 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.

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 issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1060364708


   Hi @cancan101, I tested this issue on the latest master. The database table containing `=` was added in Superset either legacy FAB UI or newest UI. Could you retry in the latest master branch?
   
   ### Legacy FAB UI
   ![image](https://user-images.githubusercontent.com/2016594/157000891-f7363faa-e2d7-4f48-b5ae-a94fa0e008dd.png)
   
   
   #### Newest UI
   ![image](https://user-images.githubusercontent.com/2016594/157000811-d13bef37-2f47-4924-a9e6-ec07432fbfd7.png)
   


-- 
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 issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061346975


   Hi @cancan101, thanks for the debug. Today, I tested the endpoint in my local test environment. It looks works.
   
   ![image](https://user-images.githubusercontent.com/2016594/157155431-4eced28d-f9e1-4438-9eb1-2c55da1e3619.png)
   


-- 
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 issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061390929


   Hi @cancan101,  Thanks for point this out.
   
   The endpoint `external_metadata_by_name` base on a [BaseSupersetView](https://github.com/apache/superset/blob/e89f0abf95a5b49291bfa11f1d0277ade43eb2c0/superset/views/datasource/views.py#L57). `BaseSupersetView` hasn't `response_400` method, so that when we invoke `self.response_400(message="Not a valid rison argument")` in `rison` [decorator](https://github.com/dpgaspar/Flask-AppBuilder/blob/2d505e819d496308a34c5f623d70331a37599cc4/flask_appbuilder/api/__init__.py#L128-L159), it raise a 500 exception.
   
   I think we have 2 ways to fix this.
   1. Use `BaseSupersetModelRestApi` instead of `BaseSupersetView` in datasource.views
   2. The `rison` decorator raise a `ValueError` exception instead a HTTP response.
   
   @dpgaspar What do you think about this issue?
   


-- 
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 edited a comment on issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
zhaoyongjie edited a comment on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061390929


   Hi @cancan101,  Thanks for pointing this out.
   
   The endpoint `external_metadata_by_name` base on a [BaseSupersetView](https://github.com/apache/superset/blob/e89f0abf95a5b49291bfa11f1d0277ade43eb2c0/superset/views/datasource/views.py#L57). `BaseSupersetView` hasn't `response_400` method, so that when we invoke `self.response_400(message="Not a valid rison argument")` in `rison` [decorator](https://github.com/dpgaspar/Flask-AppBuilder/blob/2d505e819d496308a34c5f623d70331a37599cc4/flask_appbuilder/api/__init__.py#L128-L159), it raise a 500 exception.
   
   I think we have 2 ways to fix this.
   1. Use `BaseSupersetModelRestApi` instead of `BaseSupersetView` in datasource.views
   2. The `rison` decorator raise a `ValueError` exception instead a HTTP response. IMO, the `rison` decorator should decouple with HTTP request handler, we should process HTTP exception in the HTTP handler.
   
   @dpgaspar What do you think about this issue?
   


-- 
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 issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061352880


   @cancan101 You should quote the table_name with single quotation. 
   
   ```
   curl "http://127.0.0.1:8088/datasource/external_metadata_by_name/?q=(database_name:SWAPI,datasource_type:table,schema_name:main,table_name:'allSpecies?foo=1')" ....
   ```


-- 
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 edited a comment on issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
zhaoyongjie edited a comment on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061390929


   Hi @cancan101,  Thanks for pointing this out.
   
   The endpoint `external_metadata_by_name` base on a [BaseSupersetView](https://github.com/apache/superset/blob/e89f0abf95a5b49291bfa11f1d0277ade43eb2c0/superset/views/datasource/views.py#L57). `BaseSupersetView` hasn't `response_400` method, so that when we invoke `self.response_400(message="Not a valid rison argument")` in `rison` [decorator](https://github.com/dpgaspar/Flask-AppBuilder/blob/2d505e819d496308a34c5f623d70331a37599cc4/flask_appbuilder/api/__init__.py#L128-L159), it raise a 500 exception.
   
   I think we have 2 ways to fix this.
   1. Use `BaseSupersetModelRestApi` instead of `BaseSupersetView` in datasource.views
   2. The `rison` decorator raise a `ValueError` exception instead a HTTP response.
   
   @dpgaspar What do you think about this issue?
   


-- 
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 edited a comment on issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
zhaoyongjie edited a comment on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1060364708


   Hi @cancan101, I tested this issue on the latest master. The database table containing `=` can be added in Superset either legacy FAB UI or newest UI. Could you retry in the latest master branch?
   
   ### Legacy FAB UI
   ![image](https://user-images.githubusercontent.com/2016594/157000891-f7363faa-e2d7-4f48-b5ae-a94fa0e008dd.png)
   
   
   #### Newest UI
   ![image](https://user-images.githubusercontent.com/2016594/157000811-d13bef37-2f47-4924-a9e6-ec07432fbfd7.png)
   


-- 
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] cancan101 edited a comment on issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
cancan101 edited a comment on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061349378


   I just ran this:
   ```bash
   curl 'http://127.0.0.1:8088/datasource/external_metadata_by_name/?q=(database_name:SWAPI,datasource_type:table,schema_name:main,table_name:allSpecies?foo=1)' ....
   ```
   (omitting the full curl line)
   and got back:
   ```
   {"error": "'Datasource' object has no attribute 'response_400'"
   ```
   and this is a `500` error ^
   
   this is running against master.


-- 
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] eschutho commented on issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
eschutho commented on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1063368604


   @zhaoyongjie We're doing some work on the dataset models and apis. I think we can take this on and migrate the datasource api to v1, which would use the restApi base model. 


-- 
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] cancan101 commented on issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
cancan101 commented on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061355778


   Right , I intentionally misquoted it to be consistent with the prior broken rison escaping logic. That logic is now fixed in Master however this endpoint should return a 400 error not a 500 error in the case of a malformed query param. 


-- 
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] cancan101 commented on issue #18993: superset.views.datasource.views.Datasource does not implement a response_400

Posted by GitBox <gi...@apache.org>.
cancan101 commented on issue #18993:
URL: https://github.com/apache/superset/issues/18993#issuecomment-1061349378


   I just ran this:
   ```bash
   curl 'http://127.0.0.1:8088/datasource/external_metadata_by_name/?q=(database_name:SWAPI,datasource_type:table,schema_name:main,table_name:allSpecies?foo=1)' ....
   ```
   (omitting the full curl line)
   and got back:
   ```
   {"error": "'Datasource' object has no attribute 'response_400'"
   ```
   this is running against master.


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