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/03 15:38:45 UTC
[GitHub] [incubator-superset] dpgaspar opened a new pull request #8917:
[charts] New REST API
dpgaspar opened a new pull request #8917: [charts] New REST API
URL: https://github.com/apache/incubator-superset/pull/8917
### CATEGORY
Choose one
- [ ] Bug Fix
- [X] Enhancement (new features, refinement)
- [ ] Refactor
- [ ] Add tests
- [ ] Build / Development Environment
- [ ] Documentation
### SUMMARY
New charts REST API
### 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
### 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 #8917:
[WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2726f21cbc52bd45a80b8584f1f85523e8766b24?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [2726f21...beecb25](https://codecov.io/gh/apache/incubator-superset/pull/8917?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] codecov-io edited a comment on issue #8917:
[charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c9f333f600ee967a64cd990ad832849dc0ad2e1d?src=pr&el=desc) will **increase** coverage by `0.17%`.
> The diff coverage is `67.44%`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
==========================================
+ Coverage 58.97% 59.15% +0.17%
==========================================
Files 359 367 +8
Lines 11333 11681 +348
Branches 2787 2863 +76
==========================================
+ Hits 6684 6910 +226
- Misses 4471 4592 +121
- Partials 178 179 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [.../assets/src/SqlLab/components/AceEditorWrapper.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29tcG9uZW50cy9BY2VFZGl0b3JXcmFwcGVyLmpzeA==) | `55.69% <ø> (-4.08%)` | :arrow_down: |
| [superset/assets/src/welcome/App.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy93ZWxjb21lL0FwcC5qc3g=) | `0% <ø> (ø)` | :arrow_up: |
| [superset/assets/src/dashboard/reducers/index.js](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvcmVkdWNlcnMvaW5kZXguanM=) | `100% <ø> (ø)` | :arrow_up: |
| [...ssets/src/dashboard/containers/DashboardHeader.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvY29udGFpbmVycy9EYXNoYm9hcmRIZWFkZXIuanN4) | `100% <ø> (ø)` | :arrow_up: |
| [...set/assets/src/dashboard/actions/dashboardState.js](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvYWN0aW9ucy9kYXNoYm9hcmRTdGF0ZS5qcw==) | `40.96% <0%> (ø)` | :arrow_up: |
| [...t/assets/src/dashboard/reducers/dashboardLayout.js](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvcmVkdWNlcnMvZGFzaGJvYXJkTGF5b3V0Lmpz) | `93% <0%> (-1.9%)` | :arrow_down: |
| [.../src/explore/components/AdhocMetricEditPopover.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9leHBsb3JlL2NvbXBvbmVudHMvQWRob2NNZXRyaWNFZGl0UG9wb3Zlci5qc3g=) | `62.35% <100%> (ø)` | :arrow_up: |
| [...explore/components/AdhocMetricEditPopoverTitle.jsx](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9leHBsb3JlL2NvbXBvbmVudHMvQWRob2NNZXRyaWNFZGl0UG9wb3ZlclRpdGxlLmpzeA==) | `70% <100%> (ø)` | :arrow_up: |
| [superset/assets/src/SqlLab/constants.js](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9TcWxMYWIvY29uc3RhbnRzLmpz) | `100% <100%> (ø)` | :arrow_up: |
| [...src/dashboard/util/getFilterConfigsFromFormdata.js](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree#diff-c3VwZXJzZXQvYXNzZXRzL3NyYy9kYXNoYm9hcmQvdXRpbC9nZXRGaWx0ZXJDb25maWdzRnJvbUZvcm1kYXRhLmpz) | `88.46% <100%> (+1.5%)` | :arrow_up: |
| ... and [22 more](https://codecov.io/gh/apache/incubator-superset/pull/8917/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [c9f333f...8bb704f](https://codecov.io/gh/apache/incubator-superset/pull/8917?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
#8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r364323463
##########
File path: superset/views/dashboard/api.py
##########
@@ -80,32 +75,22 @@ def validate_slug_uniqueness(value):
def validate_owners(value):
Review comment:
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] dpgaspar commented on a change in pull request
#8917: [WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363555438
##########
File path: superset/views/chart/api.py
##########
@@ -0,0 +1,153 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from flask import current_app
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from marshmallow import fields, post_load, validates_schema, ValidationError
+from marshmallow.validate import Length
+from sqlalchemy.orm.exc import NoResultFound
+
+from superset import appbuilder
+from superset.connectors.connector_registry import ConnectorRegistry
+from superset.exceptions import SupersetException
+from superset.models.slice import Slice
+from superset.utils import core as utils
+from superset.views.base import BaseOwnedModelRestApi, BaseOwnedSchema
+from superset.views.chart.mixin import SliceMixin
+
+
+def validate_json(value):
+ try:
+ utils.validate_json(value)
+ except SupersetException:
+ raise ValidationError("JSON not valid")
+
+
+def validate_owners(value):
Review comment:
Makes sense
----------------------------------------------------------------
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 #8917:
[charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2726f21cbc52bd45a80b8584f1f85523e8766b24?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [2726f21...c8841e7](https://codecov.io/gh/apache/incubator-superset/pull/8917?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] codecov-io edited a comment on issue #8917:
[WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2726f21cbc52bd45a80b8584f1f85523e8766b24?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [2726f21...fb6bb5e](https://codecov.io/gh/apache/incubator-superset/pull/8917?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] mistercrunch merged pull request #8917:
[charts] New, REST API
Posted by GitBox <gi...@apache.org>.
mistercrunch merged pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917
----------------------------------------------------------------
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
#8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363021175
##########
File path: superset/views/chart/api.py
##########
@@ -0,0 +1,223 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from flask import current_app, request
+from flask_appbuilder.api import expose, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from marshmallow import fields, post_load, ValidationError
+from marshmallow.validate import Length
+from sqlalchemy.exc import SQLAlchemyError
+
+from superset import appbuilder
+from superset.exceptions import SupersetException
+from superset.models.slice import Slice
+from superset.utils import core as utils
+from superset.views.base import BaseSupersetModelRestApi, BaseSupersetSchema
+
+from .mixin import SliceMixin
+
+
+def validate_json(value):
+ try:
+ utils.validate_json(value)
+ except SupersetException:
+ raise ValidationError("JSON not valid")
+
+
+def validate_owners(value):
+ owner = (
+ current_app.appbuilder.get_session.query(
+ current_app.appbuilder.sm.user_model.id
+ )
+ .filter_by(id=value)
+ .one_or_none()
Review comment:
I think a cleaner approach here is to use a try-catch block with `.one()`, as we don't expect there to be a `None` result.
----------------------------------------------------------------
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
#8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363021119
##########
File path: superset/views/api.py
##########
@@ -27,6 +27,7 @@
from superset.utils import core as utils
from .base import api, BaseSupersetView, handle_api_exception
+from .chart import api as chart_api # pylint: disable=unused-import
from .dashboard import api as dashboard_api # pylint: disable=unused-import
from .database import api as database_api # pylint: disable=unused-import
Review comment:
There was talk about absolute vs relative imports with @john-bodley some time back, and I believe there was consensus that we should always go for absolute paths when possible.
----------------------------------------------------------------
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
#8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r364323326
##########
File path: superset/views/chart/api.py
##########
@@ -0,0 +1,153 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from flask import current_app
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from marshmallow import fields, post_load, validates_schema, ValidationError
+from marshmallow.validate import Length
+from sqlalchemy.orm.exc import NoResultFound
+
+from superset import appbuilder
+from superset.connectors.connector_registry import ConnectorRegistry
+from superset.exceptions import SupersetException
+from superset.models.slice import Slice
+from superset.utils import core as utils
+from superset.views.base import BaseOwnedModelRestApi, BaseOwnedSchema
+from superset.views.chart.mixin import SliceMixin
+
+
+def validate_json(value):
+ try:
+ utils.validate_json(value)
+ except SupersetException:
+ raise ValidationError("JSON not valid")
+
+
+def validate_owners(value):
Review comment:
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] codecov-io edited a comment on issue #8917:
[charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/c8841e79dba98b97e4701c3f47be7fc1e34df75e?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [c8841e7...c9f333f](https://codecov.io/gh/apache/incubator-superset/pull/8917?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] codecov-io edited a comment on issue #8917:
[WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/2726f21cbc52bd45a80b8584f1f85523e8766b24?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [2726f21...98d841a](https://codecov.io/gh/apache/incubator-superset/pull/8917?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] codecov-io commented on issue #8917: [charts]
New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1dbf17bebb4bce0ff960612d1a67ea312e8d5583?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [1dbf17b...a955d45](https://codecov.io/gh/apache/incubator-superset/pull/8917?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] codecov-io edited a comment on issue #8917:
[charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1dbf17bebb4bce0ff960612d1a67ea312e8d5583?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [1dbf17b...1044bb8](https://codecov.io/gh/apache/incubator-superset/pull/8917?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
#8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363251663
##########
File path: superset/views/core.py
##########
@@ -105,8 +105,8 @@
json_success,
SupersetModelView,
)
+from .chart import views as chart_views
from .dashboard import views as dash_views
-from .dashboard.filters import DashboardFilter
from .database import views as in_views
from .utils import (
Review comment:
Will make a separate PR to address all relative imports on views
----------------------------------------------------------------
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] willbarrett commented on a change in pull
request #8917: [WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363537871
##########
File path: superset/views/chart/api.py
##########
@@ -0,0 +1,153 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from flask import current_app
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from marshmallow import fields, post_load, validates_schema, ValidationError
+from marshmallow.validate import Length
+from sqlalchemy.orm.exc import NoResultFound
+
+from superset import appbuilder
+from superset.connectors.connector_registry import ConnectorRegistry
+from superset.exceptions import SupersetException
+from superset.models.slice import Slice
+from superset.utils import core as utils
+from superset.views.base import BaseOwnedModelRestApi, BaseOwnedSchema
+from superset.views.chart.mixin import SliceMixin
+
+
+def validate_json(value):
+ try:
+ utils.validate_json(value)
+ except SupersetException:
+ raise ValidationError("JSON not valid")
+
+
+def validate_owners(value):
Review comment:
This method name is plural, but it operates on a singular value. Perhaps it could be `validate_owner` or `assert_user_exists`?
----------------------------------------------------------------
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] willbarrett commented on a change in pull
request #8917: [WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363537215
##########
File path: superset/views/base.py
##########
@@ -520,6 +566,155 @@ def related(self, column_name: str, **kwargs):
return self.response(200, count=count, result=result)
+class BaseOwnedModelRestApi(BaseSupersetModelRestApi):
+ @expose("/<pk>", methods=["PUT"])
+ @protect()
+ @check_ownership_and_item_exists
+ @safe
+ def put(self, item): # pylint: disable=arguments-differ
+ """Changes a owned Model
+ ---
+ put:
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ requestBody:
+ description: Model schema
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/{{self.__class__.__name__}}.put'
+ responses:
+ 200:
+ description: Item changed
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ result:
+ $ref: '#/components/schemas/{{self.__class__.__name__}}.put'
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 403:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not request.is_json:
+ self.response_400(message="Request is not JSON")
+ item = self.edit_model_schema.load(request.json, instance=item)
+ if item.errors:
+ return self.response_422(message=item.errors)
+ try:
+ self.datamodel.edit(item.data, raise_exception=True)
+ return self.response(
+ 200, result=self.edit_model_schema.dump(item.data, many=False).data
+ )
+ except SQLAlchemyError as e:
+ return self.response_422(message=str(e))
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ def post(self):
+ """Creates a new owned Model
+ ---
+ post:
+ requestBody:
+ description: Model schema
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/{{self.__class__.__name__}}.post'
+ responses:
+ 201:
+ description: Model added
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ id:
+ type: string
+ result:
+ $ref: '#/components/schemas/{{self.__class__.__name__}}.post'
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not request.is_json:
+ return self.response_400(message="Request is not JSON")
+ item = self.add_model_schema.load(request.json)
+ # This validates custom Schema with custom validations
+ if item.errors:
+ return self.response_422(message=item.errors)
+ try:
+ self.datamodel.add(item.data, raise_exception=True)
+ return self.response(
+ 201,
+ result=self.add_model_schema.dump(item.data, many=False).data,
+ id=item.data.id,
+ )
+ except SQLAlchemyError as e:
+ return self.response_422(message=str(e))
+
+ @expose("/<pk>", methods=["DELETE"])
+ @protect()
+ @check_ownership_and_item_exists
+ @safe
+ def delete(self, item): # pylint: disable=arguments-differ
Review comment:
I see that delete is not verifying that the request is JSON format, which differs from the other methods defined in this base view. Is this intentional?
----------------------------------------------------------------
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
#8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363251663
##########
File path: superset/views/core.py
##########
@@ -105,8 +105,8 @@
json_success,
SupersetModelView,
)
+from .chart import views as chart_views
from .dashboard import views as dash_views
-from .dashboard.filters import DashboardFilter
from .database import views as in_views
from .utils import (
Review comment:
Will make a separate PR to address all relative imports on views (non charts related)
----------------------------------------------------------------
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] willbarrett commented on a change in pull
request #8917: [WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363536223
##########
File path: superset/views/base.py
##########
@@ -396,6 +407,41 @@ def load(
return super().load(data, many=many, partial=partial, **kwargs)
+class BaseOwnedSchema(BaseSupersetSchema):
Review comment:
I think there's a natural place to split up this module here. It may be fruitful to move the base schemas to their own module in order to keep this file from growing much further. Currently this file contains base views, base schemas, and some stand-alone functions. It would be great to separate the concerns.
----------------------------------------------------------------
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
#8917: [WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363555367
##########
File path: superset/views/base.py
##########
@@ -520,6 +566,155 @@ def related(self, column_name: str, **kwargs):
return self.response(200, count=count, result=result)
+class BaseOwnedModelRestApi(BaseSupersetModelRestApi):
+ @expose("/<pk>", methods=["PUT"])
+ @protect()
+ @check_ownership_and_item_exists
+ @safe
+ def put(self, item): # pylint: disable=arguments-differ
+ """Changes a owned Model
+ ---
+ put:
+ parameters:
+ - in: path
+ schema:
+ type: integer
+ name: pk
+ requestBody:
+ description: Model schema
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/{{self.__class__.__name__}}.put'
+ responses:
+ 200:
+ description: Item changed
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ result:
+ $ref: '#/components/schemas/{{self.__class__.__name__}}.put'
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 403:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not request.is_json:
+ self.response_400(message="Request is not JSON")
+ item = self.edit_model_schema.load(request.json, instance=item)
+ if item.errors:
+ return self.response_422(message=item.errors)
+ try:
+ self.datamodel.edit(item.data, raise_exception=True)
+ return self.response(
+ 200, result=self.edit_model_schema.dump(item.data, many=False).data
+ )
+ except SQLAlchemyError as e:
+ return self.response_422(message=str(e))
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ def post(self):
+ """Creates a new owned Model
+ ---
+ post:
+ requestBody:
+ description: Model schema
+ required: true
+ content:
+ application/json:
+ schema:
+ $ref: '#/components/schemas/{{self.__class__.__name__}}.post'
+ responses:
+ 201:
+ description: Model added
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ id:
+ type: string
+ result:
+ $ref: '#/components/schemas/{{self.__class__.__name__}}.post'
+ 400:
+ $ref: '#/components/responses/400'
+ 401:
+ $ref: '#/components/responses/401'
+ 422:
+ $ref: '#/components/responses/422'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not request.is_json:
+ return self.response_400(message="Request is not JSON")
+ item = self.add_model_schema.load(request.json)
+ # This validates custom Schema with custom validations
+ if item.errors:
+ return self.response_422(message=item.errors)
+ try:
+ self.datamodel.add(item.data, raise_exception=True)
+ return self.response(
+ 201,
+ result=self.add_model_schema.dump(item.data, many=False).data,
+ id=item.data.id,
+ )
+ except SQLAlchemyError as e:
+ return self.response_422(message=str(e))
+
+ @expose("/<pk>", methods=["DELETE"])
+ @protect()
+ @check_ownership_and_item_exists
+ @safe
+ def delete(self, item): # pylint: disable=arguments-differ
Review comment:
It is, delete will not process request data, just the id on the URI `/api/v1/<resource>/<id>`
----------------------------------------------------------------
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] craig-rueda commented on a change in pull
request #8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r367605018
##########
File path: superset/views/chart/api.py
##########
@@ -0,0 +1,183 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Dict, List
+
+from flask import current_app
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from marshmallow import fields, post_load, validates_schema, ValidationError
+from marshmallow.validate import Length
+from sqlalchemy.orm.exc import NoResultFound
+
+from superset import appbuilder
+from superset.connectors.connector_registry import ConnectorRegistry
+from superset.exceptions import SupersetException
+from superset.models.dashboard import Dashboard
+from superset.models.slice import Slice
+from superset.utils import core as utils
+from superset.views.base_api import BaseOwnedModelRestApi
+from superset.views.base_schemas import BaseOwnedSchema, validate_owner
+from superset.views.chart.mixin import SliceMixin
+
+
+def validate_json(value):
+ try:
+ utils.validate_json(value)
+ except SupersetException:
+ raise ValidationError("JSON not valid")
+
+
+def validate_dashboard(value):
+ try:
+ (current_app.appbuilder.get_session.query(Dashboard).filter_by(id=value).one())
+ except NoResultFound:
+ raise ValidationError(f"Dashboard {value} does not exist")
+
+
+def validate_update_datasource(data: Dict):
+ if not ("datasource_type" in data and "datasource_id" in data):
+ return
+ datasource_type = data["datasource_type"]
+ datasource_id = data["datasource_id"]
+ try:
+ datasource = ConnectorRegistry.get_datasource(
+ datasource_type, datasource_id, current_app.appbuilder.get_session
+ )
+ except (NoResultFound, KeyError):
+ raise ValidationError(
+ f"Datasource [{datasource_type}].{datasource_id} does not exist"
+ )
+ data["datasource_name"] = datasource.name
+
+
+def populate_dashboards(instance: Slice, dashboards: List[int]):
+ """
+ Mutates a Slice with the dashboards SQLA Models
+ """
+ dashboards_tmp = []
+ for dashboard_id in dashboards:
+ dashboards_tmp.append(
+ current_app.appbuilder.get_session.query(Dashboard)
+ .filter_by(id=dashboard_id)
+ .one()
+ )
+ instance.dashboards = dashboards_tmp
+
+
+class ChartPostSchema(BaseOwnedSchema):
+ __class_model__ = Slice
+
+ slice_name = fields.String(required=True, validate=Length(1, 250))
+ description = fields.String(allow_none=True)
+ viz_type = fields.String(allow_none=True, validate=Length(0, 250))
+ owners = fields.List(fields.Integer(validate=validate_owner))
+ params = fields.String(allow_none=True, validate=validate_json)
+ cache_timeout = fields.Integer()
+ datasource_id = fields.Integer(required=True)
+ datasource_type = fields.String(required=True)
+ datasource_name = fields.String(allow_none=True)
+ dashboards = fields.List(fields.Integer(validate=validate_dashboard))
+
+ @validates_schema
+ def validate_schema(self, data: Dict): # pylint: disable=no-self-use
+ validate_update_datasource(data)
+
+ @post_load
+ def make_object(self, data: Dict, discard: List[str] = None) -> Slice:
+ instance = super().make_object(data, discard=["dashboards"])
+ populate_dashboards(instance, data.get("dashboards", []))
+ return instance
+
+
+class ChartPutSchema(BaseOwnedSchema):
+ instance: Slice
+
+ slice_name = fields.String(allow_none=True, validate=Length(0, 250))
+ description = fields.String(allow_none=True)
+ viz_type = fields.String(allow_none=True, validate=Length(0, 250))
+ owners = fields.List(fields.Integer(validate=validate_owner))
+ params = fields.String(allow_none=True)
+ cache_timeout = fields.Integer()
+ datasource_id = fields.Integer(allow_none=True)
+ datasource_type = fields.String(allow_none=True)
+ dashboards = fields.List(fields.Integer(validate=validate_dashboard))
+
+ @validates_schema
+ def validate_schema(self, data: Dict): # pylint: disable=no-self-use
+ validate_update_datasource(data)
+
+ @post_load
+ def make_object(self, data: Dict, discard: List[str] = None) -> Slice:
+ self.instance = super().make_object(data, ["dashboards"])
+ if "dashboards" in data:
+ populate_dashboards(self.instance, data["dashboards"])
+ return self.instance
+
+
+class ChartRestApi(SliceMixin, BaseOwnedModelRestApi):
+ datamodel = SQLAInterface(Slice)
+
+ resource_name = "chart"
+ allow_browser_login = True
+
+ class_permission_name = "SliceModelView"
+ method_permission_name = {
+ "get_list": "list",
+ "get": "show",
+ "post": "add",
+ "put": "edit",
+ "delete": "delete",
+ "info": "list",
+ "related": "list",
+ }
+ show_columns = [
+ "slice_name",
+ "description",
+ "owners.id",
+ "owners.username",
+ "dashboards.id",
+ "dashboards.dashboard_title",
+ "viz_type",
+ "params",
+ "cache_timeout",
+ ]
+ list_columns = [
+ "slice_name",
+ "description",
+ "changed_by.username",
+ "changed_by_name",
+ "changed_on",
+ "viz_type",
+ "params",
+ "cache_timeout",
+ ]
+ # Will just affect _info endpoint
+ edit_columns = ["slice_name"]
+ add_columns = edit_columns
+
+ # exclude_route_methods = ("info",)
+
+ add_model_schema = ChartPostSchema()
+ edit_model_schema = ChartPutSchema()
+
+ order_rel_fields = {
+ "slices": ("slice_name", "asc"),
+ "owners": ("first_name", "asc"),
+ }
+ filter_rel_fields_field = {"owners": "first_name", "dashboards": "dashboard_title"}
+
+
+appbuilder.add_api(ChartRestApi)
Review comment:
Move this into `app.py`
----------------------------------------------------------------
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 #8917:
[charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1dbf17bebb4bce0ff960612d1a67ea312e8d5583?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [1dbf17b...c20a158](https://codecov.io/gh/apache/incubator-superset/pull/8917?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] codecov-io edited a comment on issue #8917:
[charts] New, REST API
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#issuecomment-570617790
# [Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=h1) Report
> Merging [#8917](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-superset/commit/1dbf17bebb4bce0ff960612d1a67ea312e8d5583?src=pr&el=desc) will **not change** coverage.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-superset/pull/8917?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #8917 +/- ##
=======================================
Coverage 58.97% 58.97%
=======================================
Files 359 359
Lines 11333 11333
Branches 2787 2787
=======================================
Hits 6684 6684
Misses 4471 4471
Partials 178 178
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-superset/pull/8917?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/8917?src=pr&el=footer). Last update [1dbf17b...08969bf](https://codecov.io/gh/apache/incubator-superset/pull/8917?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] willbarrett commented on a change in pull
request #8917: [WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
willbarrett commented on a change in pull request #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363539954
##########
File path: superset/views/dashboard/api.py
##########
@@ -80,32 +75,22 @@ def validate_slug_uniqueness(value):
def validate_owners(value):
Review comment:
This method is duplicated in superset/views/chart/api.py - can we extract it to a shared util?
----------------------------------------------------------------
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
#8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363021260
##########
File path: superset/views/core.py
##########
@@ -105,8 +105,8 @@
json_success,
SupersetModelView,
)
+from .chart import views as chart_views
from .dashboard import views as dash_views
-from .dashboard.filters import DashboardFilter
from .database import views as in_views
from .utils import (
Review comment:
Same here regarding absolute vs relative imports.
----------------------------------------------------------------
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
#8917: [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8917: [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r364323209
##########
File path: superset/views/base.py
##########
@@ -396,6 +407,41 @@ def load(
return super().load(data, many=many, partial=partial, **kwargs)
+class BaseOwnedSchema(BaseSupersetSchema):
Review comment:
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] dpgaspar commented on a change in pull request
#8917: [WiP] [charts] New, REST API
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #8917: [WiP] [charts] New, REST API
URL: https://github.com/apache/incubator-superset/pull/8917#discussion_r363555587
##########
File path: superset/views/dashboard/api.py
##########
@@ -80,32 +75,22 @@ def validate_slug_uniqueness(value):
def validate_owners(value):
Review comment:
Yap, planning to DRY things a bit more, still a work in progress
----------------------------------------------------------------
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