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/01/23 14:14:15 UTC
[GitHub] [incubator-superset] dpgaspar opened a new pull request #9002:
[database] new, API table schema info
dpgaspar opened a new pull request #9002: [database] new, API table schema info
URL: https://github.com/apache/incubator-superset/pull/9002
### CATEGORY
- [ ] Bug Fix
- [X] Enhancement (new features, refinement)
- [X] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
Migrate `/table/<database_id>/<table_name>/<schema>/` to `/api/v1/database` resource.
- Guarantees database filtering based on the user permissions
- Full OpenAPI spec
- Better error handling
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Requires DB Migration.
- [ ] Confirm DB Migration upgrade and downgrade tested.
- [X] Introduces new feature or API
- [X] 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] villebro commented on a change in pull request
#9002: [database] new, API table metadata
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9002: [database] new, API table metadata
URL: https://github.com/apache/incubator-superset/pull/9002#discussion_r370818316
##########
File path: superset/views/database/api.py
##########
@@ -50,3 +137,144 @@ class DatabaseRestApi(DatabaseMixin, BaseSupersetModelRestApi):
# Removes the local limit for the page size
max_page_size = -1
validators_columns = {"sqlalchemy_uri": sqlalchemy_uri_validator}
+
+ @expose(
+ "/<int:pk>/table/<string:table_name>/<string:schema_name>/", methods=["GET"]
+ )
+ @protect()
+ @safe
+ @event_logger.log_this
+ def table_metadata(
+ self, pk: int, table_name: str, schema_name: str
+ ): # pylint: disable=invalid-name
+ """ Table schema info
+ ---
+ get:
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The database id
+ - in: path
+ schema:
+ type: string
+ name: table_name
+ description: Table name
+ - in: path
+ schema:
+ type: string
+ name: schema
+ description: Table schema
+ responses:
+ 200:
+ description: Table schema info
+ content:
+ text/plain:
+ schema:
+ type: object
+ properties:
+ columns:
+ type: array
+ description: Table columns info
+ items:
+ type: object
+ properties:
+ keys:
+ type: array
+ items:
+ type: string
+ longType:
+ type: string
+ name:
+ type: string
+ type:
+ type: string
+ foreignKeys:
+ type: array
+ description: Table list of foreign keys
+ items:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ options:
+ type: object
+ referred_columns:
+ type: array
+ items:
+ type: string
+ referred_schema:
+ type: string
+ referred_table:
+ type: string
+ type:
+ type: string
+ indexes:
+ type: array
+ description: Table list of indexes
+ items:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ options:
+ type: object
+ referred_columns:
+ type: array
+ items:
+ type: string
+ referred_schema:
+ type: string
+ referred_table:
+ type: string
+ type:
+ type: string
+ primaryKey:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ type:
+ type: string
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ table_name_parsed: Optional[str] = parse_js_uri_path_item(table_name)
+ schema_parsed: Optional[str] = parse_js_uri_path_item(
+ schema_name, eval_undefined=True
+ )
Review comment:
As the return type is already typed in `parse_js_uri_path_item`, do these need to be explicitly typed 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
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 #9002:
[database] new, API table metadata
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #9002: [database] new, API table metadata
URL: https://github.com/apache/incubator-superset/pull/9002#issuecomment-578173671
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9002?src=pr&el=h1) Report
> Merging [#9002](https://codecov.io/gh/apache/incubator-superset/pull/9002?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/43789e54da383048c6258690e3f612c5c8ef816d?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9002?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9002 +/- ##
==========================================
- Coverage 59.16% 59.15% -0.02%
==========================================
Files 367 367
Lines 11680 11682 +2
Branches 2863 2864 +1
==========================================
Hits 6910 6910
- Misses 4591 4593 +2
Partials 179 179
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/9002?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...rset/assets/src/dashboard/actions/sliceEntities.js](https://codecov.io/gh/apache/incubator-superset/pull/9002/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvYWN0aW9ucy9zbGljZUVudGl0aWVzLmpz) | `9.67% <ø> (ø)` | :arrow_up: |
| [.../assets/src/SqlLab/components/AceEditorWrapper.jsx](https://codecov.io/gh/apache/incubator-superset/pull/9002/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9BY2VFZGl0b3JXcmFwcGVyLmpzeA==) | `54.32% <0%> (-1.38%)` | :arrow_down: |
| [superset/assets/src/chart/chartAction.js](https://codecov.io/gh/apache/incubator-superset/pull/9002/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9jaGFydC9jaGFydEFjdGlvbi5qcw==) | `47.94% <0%> (ø)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9002?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/9002?src=pr&el=footer). Last update [43789e5...326d321](https://codecov.io/gh/apache/incubator-superset/pull/9002?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] dpgaspar commented on a change in pull request
#9002: [database] new, API table metadata
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9002: [database] new, API table metadata
URL: https://github.com/apache/incubator-superset/pull/9002#discussion_r370836960
##########
File path: superset/views/database/api.py
##########
@@ -50,3 +137,144 @@ class DatabaseRestApi(DatabaseMixin, BaseSupersetModelRestApi):
# Removes the local limit for the page size
max_page_size = -1
validators_columns = {"sqlalchemy_uri": sqlalchemy_uri_validator}
+
+ @expose(
+ "/<int:pk>/table/<string:table_name>/<string:schema_name>/", methods=["GET"]
+ )
+ @protect()
+ @safe
+ @event_logger.log_this
+ def table_metadata(
+ self, pk: int, table_name: str, schema_name: str
+ ): # pylint: disable=invalid-name
+ """ Table schema info
+ ---
+ get:
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The database id
+ - in: path
+ schema:
+ type: string
+ name: table_name
+ description: Table name
+ - in: path
+ schema:
+ type: string
+ name: schema
+ description: Table schema
+ responses:
+ 200:
+ description: Table schema info
+ content:
+ text/plain:
+ schema:
+ type: object
+ properties:
+ columns:
+ type: array
+ description: Table columns info
+ items:
+ type: object
+ properties:
+ keys:
+ type: array
+ items:
+ type: string
+ longType:
+ type: string
+ name:
+ type: string
+ type:
+ type: string
+ foreignKeys:
+ type: array
+ description: Table list of foreign keys
+ items:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ options:
+ type: object
+ referred_columns:
+ type: array
+ items:
+ type: string
+ referred_schema:
+ type: string
+ referred_table:
+ type: string
+ type:
+ type: string
+ indexes:
+ type: array
+ description: Table list of indexes
+ items:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ options:
+ type: object
+ referred_columns:
+ type: array
+ items:
+ type: string
+ referred_schema:
+ type: string
+ referred_table:
+ type: string
+ type:
+ type: string
+ primaryKey:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ type:
+ type: string
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ table_name_parsed: Optional[str] = parse_js_uri_path_item(table_name)
+ schema_parsed: Optional[str] = parse_js_uri_path_item(
+ schema_name, eval_undefined=True
+ )
Review comment:
yes, good question. Not really I guess, what do you think?
----------------------------------------------------------------
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 #9002:
[database] new, API table metadata
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #9002: [database] new, API table metadata
URL: https://github.com/apache/incubator-superset/pull/9002#issuecomment-578173671
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9002?src=pr&el=h1) Report
> Merging [#9002](https://codecov.io/gh/apache/incubator-superset/pull/9002?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/16b539cf054d8b2ff8f4518d2eb5a932ee34b33a?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/9002?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #9002 +/- ##
=======================================
Coverage 59.15% 59.15%
=======================================
Files 367 367
Lines 11686 11686
Branches 2866 2866
=======================================
Hits 6913 6913
Misses 4594 4594
Partials 179 179
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/9002?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/9002?src=pr&el=footer). Last update [16b539c...651b28c](https://codecov.io/gh/apache/incubator-superset/pull/9002?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] dpgaspar commented on a change in pull request
#9002: [database] new, API table schema info
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9002: [database] new, API table schema info
URL: https://github.com/apache/incubator-superset/pull/9002#discussion_r370256005
##########
File path: superset/views/database/api.py
##########
@@ -14,20 +14,88 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
+from typing import Dict, List, Optional
+
+from flask_appbuilder.api import expose, protect, safe
from flask_appbuilder.models.sqla.interface import SQLAInterface
+from flask_babel import lazy_gettext as _
+from sqlalchemy.exc import SQLAlchemyError
-import superset.models.core as models
+from superset import event_logger
+from superset.models.core import Database
+from superset.utils.core import error_msg_from_exception, parse_js_uri_path_item
from superset.views.base_api import BaseSupersetModelRestApi
+from superset.views.database.filters import DatabaseFilter
+from superset.views.database.mixins import DatabaseMixin
+from superset.views.database.validators import sqlalchemy_uri_validator
+
+
+def get_table_schema_info(
Review comment:
Not very happy with placing this 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
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
#9002: [database] new, API table metadata
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #9002: [database] new, API table metadata
URL: https://github.com/apache/incubator-superset/pull/9002#discussion_r370837666
##########
File path: superset/views/database/api.py
##########
@@ -50,3 +137,144 @@ class DatabaseRestApi(DatabaseMixin, BaseSupersetModelRestApi):
# Removes the local limit for the page size
max_page_size = -1
validators_columns = {"sqlalchemy_uri": sqlalchemy_uri_validator}
+
+ @expose(
+ "/<int:pk>/table/<string:table_name>/<string:schema_name>/", methods=["GET"]
+ )
+ @protect()
+ @safe
+ @event_logger.log_this
+ def table_metadata(
+ self, pk: int, table_name: str, schema_name: str
+ ): # pylint: disable=invalid-name
+ """ Table schema info
+ ---
+ get:
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The database id
+ - in: path
+ schema:
+ type: string
+ name: table_name
+ description: Table name
+ - in: path
+ schema:
+ type: string
+ name: schema
+ description: Table schema
+ responses:
+ 200:
+ description: Table schema info
+ content:
+ text/plain:
+ schema:
+ type: object
+ properties:
+ columns:
+ type: array
+ description: Table columns info
+ items:
+ type: object
+ properties:
+ keys:
+ type: array
+ items:
+ type: string
+ longType:
+ type: string
+ name:
+ type: string
+ type:
+ type: string
+ foreignKeys:
+ type: array
+ description: Table list of foreign keys
+ items:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ options:
+ type: object
+ referred_columns:
+ type: array
+ items:
+ type: string
+ referred_schema:
+ type: string
+ referred_table:
+ type: string
+ type:
+ type: string
+ indexes:
+ type: array
+ description: Table list of indexes
+ items:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ options:
+ type: object
+ referred_columns:
+ type: array
+ items:
+ type: string
+ referred_schema:
+ type: string
+ referred_table:
+ type: string
+ type:
+ type: string
+ primaryKey:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ type:
+ type: string
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ table_name_parsed: Optional[str] = parse_js_uri_path_item(table_name)
+ schema_parsed: Optional[str] = parse_js_uri_path_item(
+ schema_name, eval_undefined=True
+ )
Review comment:
I think it's already implied i.e.not needed, so would personally remove it unless it makes the code more understandable.
----------------------------------------------------------------
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] dpgaspar commented on a change in pull request
#9002: [database] new, API table metadata
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #9002: [database] new, API table metadata
URL: https://github.com/apache/incubator-superset/pull/9002#discussion_r370960976
##########
File path: superset/views/database/api.py
##########
@@ -50,3 +137,144 @@ class DatabaseRestApi(DatabaseMixin, BaseSupersetModelRestApi):
# Removes the local limit for the page size
max_page_size = -1
validators_columns = {"sqlalchemy_uri": sqlalchemy_uri_validator}
+
+ @expose(
+ "/<int:pk>/table/<string:table_name>/<string:schema_name>/", methods=["GET"]
+ )
+ @protect()
+ @safe
+ @event_logger.log_this
+ def table_metadata(
+ self, pk: int, table_name: str, schema_name: str
+ ): # pylint: disable=invalid-name
+ """ Table schema info
+ ---
+ get:
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ description: The database id
+ - in: path
+ schema:
+ type: string
+ name: table_name
+ description: Table name
+ - in: path
+ schema:
+ type: string
+ name: schema
+ description: Table schema
+ responses:
+ 200:
+ description: Table schema info
+ content:
+ text/plain:
+ schema:
+ type: object
+ properties:
+ columns:
+ type: array
+ description: Table columns info
+ items:
+ type: object
+ properties:
+ keys:
+ type: array
+ items:
+ type: string
+ longType:
+ type: string
+ name:
+ type: string
+ type:
+ type: string
+ foreignKeys:
+ type: array
+ description: Table list of foreign keys
+ items:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ options:
+ type: object
+ referred_columns:
+ type: array
+ items:
+ type: string
+ referred_schema:
+ type: string
+ referred_table:
+ type: string
+ type:
+ type: string
+ indexes:
+ type: array
+ description: Table list of indexes
+ items:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ options:
+ type: object
+ referred_columns:
+ type: array
+ items:
+ type: string
+ referred_schema:
+ type: string
+ referred_table:
+ type: string
+ type:
+ type: string
+ primaryKey:
+ type: object
+ properties:
+ column_names:
+ type: array
+ items:
+ type: string
+ name:
+ type: string
+ type:
+ type: string
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ table_name_parsed: Optional[str] = parse_js_uri_path_item(table_name)
+ schema_parsed: Optional[str] = parse_js_uri_path_item(
+ schema_name, eval_undefined=True
+ )
Review comment:
removed
----------------------------------------------------------------
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] dpgaspar merged pull request #9002: [database]
new, API table metadata
Posted by GitBox <gi...@apache.org>.
dpgaspar merged pull request #9002: [database] new, API table metadata
URL: https://github.com/apache/incubator-superset/pull/9002
----------------------------------------------------------------
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