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 2021/11/03 17:56:09 UTC
[GitHub] [superset] michael-s-molina opened a new pull request #17337: feat: Adds a key-value store endpoint for Superset
michael-s-molina opened a new pull request #17337:
URL: https://github.com/apache/superset/pull/17337
### SUMMARY
WIP: This PR will add a key-value store endpoint to Superset with secure ID generation and management of store entries.
TODO
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TODO
### TESTING INSTRUCTIONS
TODO
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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] michael-s-molina commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-965399369
> left some comments, we should add more tests also
Thanks for the review @dpgaspar! I'll address all your 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.
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r747539542
##########
File path: superset/key_value/commands/create.py
##########
@@ -0,0 +1,61 @@
+# 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.
+import logging
+from hashlib import sha3_256
+from typing import Any, Dict, List
+from uuid import uuid4
+
+from flask import current_app
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+from marshmallow import ValidationError
+
+from superset.commands.base import BaseCommand
+from superset.dao.exceptions import DAOException
+from superset.key_value.commands.exceptions import KeyValueCreateFailedError
+from superset.key_value.dao import KeyValueDAO
+from superset.models.key_value import KeyValue
+
+logger = logging.getLogger(__name__)
+
+
+class CreateKeyValueCommand(BaseCommand):
+ def __init__(self, user: User, data: Dict[str, Any]):
+ self._actor = user
+ self._properties = data.copy()
+
+ def run(self) -> Model:
+ try:
+ secretKey = current_app.config["SECRET_KEY"]
+ key = sha3_256(secretKey.encode() + str(uuid4()).encode()).hexdigest()
Review comment:
I ended up using https://docs.python.org/3/library/secrets.html#secrets.token_urlsafe for the key generation π
--
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 a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r742622341
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,154 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueGetFailedError,
+)
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import (
+ KeyValuePostSchema,
+ KeyValueGetSchema,
+)
+from superset.extensions import event_logger
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ post_schema = KeyValuePostSchema()
+ get_schema = KeyValueGetSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
Review comment:
allow methods in the endpoint
```
include_route_methods = {RouteMethod.POST, RouteMethod.GET}
```
--
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 a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r742622341
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,154 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueGetFailedError,
+)
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import (
+ KeyValuePostSchema,
+ KeyValueGetSchema,
+)
+from superset.extensions import event_logger
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ post_schema = KeyValuePostSchema()
+ get_schema = KeyValueGetSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
Review comment:
include_route_methods = {RouteMethod.POST, RouteMethod.GET}
--
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] michael-s-molina commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-966568187
@dpgaspar @willbarrett @betodealmeida I updated the PR description with the results of our last interactions about security and suggested use cases. I also added examples of the endpoint operations and their configurations.
@john-bodley I would really like your review too.
@dpgaspar even though the names in the API are editor and viewer, Iβm still going to use the owner nomenclature in the database to match our existing schema π
You'll notice that the code is not reflecting the current description. I'm looking for more reviews on the security model before implementing it. If you can, just give a thumbs-up in this comment so I know you're ok with it or add a comment otherwise.
I'm really excited about my first Python PR so thank you all for the reviews. It's good to be back to full-stack development ππΌ
--
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] github-actions[bot] commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-968337033
β οΈ @michael-s-molina Your base branch `master` has just also updated `superset/migrations`.
β **Please consider rebasing your branch to avoid db migration conflicts.**
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r745768525
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ include_route_methods = {
+ RouteMethod.POST,
+ RouteMethod.PUT,
+ RouteMethod.GET,
+ RouteMethod.DELETE,
+ }
+ allow_browser_login = True
+ openapi_spec_tag = "Key Value Store"
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+ log_to_statsd=False,
+ )
+ def post(self) -> Response:
+ """Stores a new value.
+ ---
+ post:
+ description: >-
+ Stores a new value.
+ requestBody:
+ description: Key value schema
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ value:
+ type: string
+ description: Any type of JSON supported value.
+ required: true
+ duration_ms:
+ type: number
+ description: The duration of the value on the key store. If no duration is specified the value won't expire.
+ required: false
+ default: null
+ reset_duration_on_retrieval:
+ type: boolean
+ description: If the duration should be reset when the value is retrieved. This is useful if you wish to expire unused values but keep the ones that are actively retrieved.
+ required: false
+ default: true
+ responses:
+ 201:
+ description: The value was stored successfully. It returns the key to retrieve the value.
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ key:
+ type: string
+ description: The key to retrieve the value.
+ 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")
Review comment:
Yes! Thanks for pointing that out!
--
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] michael-s-molina commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-964426050
Hi @willbarrett. Thanks so much for helping with the review!
> I'm concerned about some of the proposed usages - particularly access tokens, refresh tokens, and public key storage. These items should all be held in an encrypted system, not a plain-text field. That's a big security no-no.
You're right about this. There are more secure structures for this type of information. I removed them from possible use cases in the PR description.
> I would also recommend adding a user_id to the key value store and only allowing retrieval of items by the same user. Otherwise this system potentially allows any user to read any stored key, which for most of the uses you recommend would constitute a security hole.
> The original key value store was originally deprecated due to similar security concerns. Keep in mind that obscurity of a long key is not the same thing as security - even if the keys are hard to guess, we should have security controls on the individual keys.
The key-value table has a `created_by` field for that purpose. One of the main uses of this store is to share content between users like an URL state, a draft, etc. Currently, the access is being controlled by the possession of a secure key. It's the same type of access control as Google Docs when you select the "Anyone with the link" type of sharing. We could increment this and also offer the second type of control where we can select which users are allowed to access the content, similar to Google Docs restricted sharing. 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.
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] willbarrett commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
willbarrett commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-964431343
Thanks for the response @michael-s-molina - I think as Superset moves into more organizations we should default to closed in all cases, so I would support the closed-sharing model. The open-sharing model is pretty dangerous - it becomes easy to create a link that's accessible to everyone in a company, which I believe wouldn't be a desired behavior by most larger organizations as a default behavior. In Superset, URL parameters, application state, and cache can contain highly sensitive information so I think we should shy away from the open-sharing model in all cases.
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r748293125
##########
File path: superset/key_value/commands/get.py
##########
@@ -0,0 +1,50 @@
+# 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.
+import logging
+from typing import Any, Dict, List
+from datetime import datetime
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+from marshmallow import ValidationError
+
+from superset.key_value.commands.exceptions import KeyValueGetFailedError
+from superset.key_value.dao import KeyValueDAO
+from superset.commands.base import BaseCommand
+from superset.dao.exceptions import DAOException
+
+logger = logging.getLogger(__name__)
+
+
+class GetKeyValueCommand(BaseCommand):
+ def __init__(self, user: User, key: str):
+ self._actor = user
+ self._key = key
+
+ def run(self) -> Model:
+ try:
+ model = KeyValueDAO.find_by_id(self._key)
+ if not model:
+ return None
+ setattr(model, 'retrieved_on', datetime.utcnow())
Review comment:
I just didn't know about it π. Now I'm using it.
--
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] codecov[bot] edited a comment on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-969016316
# [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17337](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f899453) into [master](https://codecov.io/gh/apache/superset/commit/e2a429b0c8042ba867f834f5dc5561d1e402289b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2a429b) will **decrease** coverage by `0.15%`.
> The diff coverage is `75.52%`.
> :exclamation: Current head f899453 differs from pull request most recent head c32bfeb. Consider uploading reports for the commit c32bfeb to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17337 +/- ##
==========================================
- Coverage 76.94% 76.79% -0.16%
==========================================
Files 1042 1052 +10
Lines 56248 56533 +285
Branches 7784 7784
==========================================
+ Hits 43282 43416 +134
- Misses 12707 12858 +151
Partials 259 259
```
| Flag | Coverage Ξ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.90% <75.52%> (-0.06%)` | :arrow_down: |
| presto | `81.78% <75.52%> (-0.06%)` | :arrow_down: |
| python | `82.12% <75.52%> (-0.35%)` | :arrow_down: |
| sqlite | `81.58% <75.52%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.50% <ΓΈ> (ΓΈ)` | |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.49% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/tasks/scheduler.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3Mvc2NoZWR1bGVyLnB5) | `62.96% <33.33%> (-5.93%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `44.11% <44.11%> (ΓΈ)` | |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `57.69% <57.69%> (ΓΈ)` | |
| [superset/key\_value/commands/cleanup.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NsZWFudXAucHk=) | `62.50% <62.50%> (ΓΈ)` | |
| [superset/key\_value/api.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2FwaS5weQ==) | `81.42% <81.42%> (ΓΈ)` | |
| [superset/key\_value/commands/get.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2dldC5weQ==) | `83.33% <83.33%> (ΓΈ)` | |
| [superset/key\_value/dao.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2Rhby5weQ==) | `85.29% <85.29%> (ΓΈ)` | |
| [superset/key\_value/commands/create.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `87.87% <87.87%> (ΓΈ)` | |
| ... and [11 more](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Ξ = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e2a429b...c32bfeb](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] codecov[bot] edited a comment on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-969016316
# [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17337](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b4a465f) into [master](https://codecov.io/gh/apache/superset/commit/e2a429b0c8042ba867f834f5dc5561d1e402289b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2a429b) will **decrease** coverage by `0.21%`.
> The diff coverage is `75.78%`.
[](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17337 +/- ##
==========================================
- Coverage 76.94% 76.72% -0.22%
==========================================
Files 1042 1052 +10
Lines 56248 56533 +285
Branches 7784 7784
==========================================
+ Hits 43282 43377 +95
- Misses 12707 12897 +190
Partials 259 259
```
| Flag | Coverage Ξ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.89% <75.78%> (-0.06%)` | :arrow_down: |
| postgres | `81.90% <75.78%> (-0.06%)` | :arrow_down: |
| presto | `?` | |
| python | `81.99% <75.78%> (-0.48%)` | :arrow_down: |
| sqlite | `81.58% <75.78%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.50% <ΓΈ> (ΓΈ)` | |
| [superset/tasks/scheduler.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3Mvc2NoZWR1bGVyLnB5) | `62.96% <33.33%> (-5.93%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `44.11% <44.11%> (ΓΈ)` | |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `57.69% <57.69%> (ΓΈ)` | |
| [superset/key\_value/commands/cleanup.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NsZWFudXAucHk=) | `62.50% <62.50%> (ΓΈ)` | |
| [superset/key\_value/api.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2FwaS5weQ==) | `81.42% <81.42%> (ΓΈ)` | |
| [superset/key\_value/commands/get.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2dldC5weQ==) | `83.33% <83.33%> (ΓΈ)` | |
| [superset/key\_value/dao.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2Rhby5weQ==) | `85.29% <85.29%> (ΓΈ)` | |
| [superset/key\_value/commands/create.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `87.87% <87.87%> (ΓΈ)` | |
| [superset/initialization/\_\_init\_\_.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvaW5pdGlhbGl6YXRpb24vX19pbml0X18ucHk=) | `88.07% <100.00%> (+0.08%)` | :arrow_up: |
| ... and [11 more](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Ξ = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e2a429b...b4a465f](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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 a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r742622341
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,154 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueGetFailedError,
+)
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import (
+ KeyValuePostSchema,
+ KeyValueGetSchema,
+)
+from superset.extensions import event_logger
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ post_schema = KeyValuePostSchema()
+ get_schema = KeyValueGetSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
Review comment:
allow the method in the endpoind
```
include_route_methods = {RouteMethod.POST, RouteMethod.GET}
```
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r745769882
##########
File path: superset/key_value/commands/cleanup.py
##########
@@ -0,0 +1,47 @@
+# 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.
+import logging
+from datetime import datetime, timedelta
+
+from superset.commands.base import BaseCommand
+from superset.models.key_value import KeyValue
+from superset.key_value.utils import is_expired
+from superset.utils.celery import session_scope
+
+logger = logging.getLogger(__name__)
+
+
+class CleanupCommand(BaseCommand):
+ """
+ Expiration cleanup command.
+ """
+
+ def __init__(self, worker_context: bool = True):
+ self._worker_context = worker_context
+
+ def run(self) -> None:
+ logger.info("Key value store cleanup starting")
+ with session_scope(nullpool=True) as session:
+ for keyValue in session.query(KeyValue).all():
+ if is_expired(keyValue):
Review comment:
Yes. In fact, I'm currently doing this exact change. I'm just trying to learn how to do that with SQL Alchemy since this is my first Python PR :sweat_smile:
--
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] willbarrett commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
willbarrett commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-964457047
Thanks @michael-s-molina - if we do implement the "anyone with a key" model, we should throw some restrictions or confirmation around it so it's very clear to the user that they're about to share very widely. Something to think about on the UI-side of the house.
--
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 a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r742640654
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,154 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueGetFailedError,
+)
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import (
+ KeyValuePostSchema,
+ KeyValueGetSchema,
+)
+from superset.extensions import event_logger
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ post_schema = KeyValuePostSchema()
+ get_schema = KeyValueGetSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+
Review comment:
use this property and add uncomment `@protect()`
```
allow_browser_login = True
```
--
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] dpgaspar commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-965200100
> Thanks @michael-s-molina - if we do implement the "anyone with a key" model, we should throw some restrictions or confirmation around it so it's very clear to the user that they're about to share very widely. Something to think about on the UI-side of the house.
Would definitely fall to restrict access to the owner of the key. but the K/V store goal is not clear yet or it's just too broad. Session management and caching are sensitive, caching values could potentially defeat dataset ownership and RBAC permissions.
We can make the ownership restriction optional and on by default behind a config key. Or discuss this further on a secure sharing 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] betodealmeida commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r745776765
##########
File path: superset/key_value/commands/cleanup.py
##########
@@ -0,0 +1,47 @@
+# 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.
+import logging
+from datetime import datetime, timedelta
+
+from superset.commands.base import BaseCommand
+from superset.models.key_value import KeyValue
+from superset.key_value.utils import is_expired
+from superset.utils.celery import session_scope
+
+logger = logging.getLogger(__name__)
+
+
+class CleanupCommand(BaseCommand):
+ """
+ Expiration cleanup command.
+ """
+
+ def __init__(self, worker_context: bool = True):
+ self._worker_context = worker_context
+
+ def run(self) -> None:
+ logger.info("Key value store cleanup starting")
+ with session_scope(nullpool=True) as session:
+ for keyValue in session.query(KeyValue).all():
+ if is_expired(keyValue):
Review comment:
Oh, good job then! I could never tell this was your first Python PR!
And yeah, doing SQL with SQLAlchemy is not fun... :-/
--
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] michael-s-molina edited a comment on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-971588829
We had a meeting about the proposed solution in this PR and decided to take a slightly different approach to resolve the use cases. Exposing a generic key-value interface had the potential to induce users to store any type of information in the store, diminishing the benefits of the current endpoints where each type of resource is represented by a specific path. We could also introduce some conflicts with the current security model in terms of access grants. One example would be to store information in the key-value store that is related to a dashboard. The dashboard already has configured access grants and re-defining those in each key-value entry had the potential for security problems. We decided to offer the same type of functionality under each currently defined endpoint to preserve the more typed nature of our endpoints and to leverage the existing security model. I'm closing this PR to preserve all the discussions and conclusions for historical reasons and I'll open a new
one with the new approach.
A big thanks to all the reviewers that are helping to shape this new feature.
--
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] codecov[bot] edited a comment on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-969016316
# [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17337](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f899453) into [master](https://codecov.io/gh/apache/superset/commit/e2a429b0c8042ba867f834f5dc5561d1e402289b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2a429b) will **decrease** coverage by `0.15%`.
> The diff coverage is `75.52%`.
> :exclamation: Current head f899453 differs from pull request most recent head b4a465f. Consider uploading reports for the commit b4a465f to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17337 +/- ##
==========================================
- Coverage 76.94% 76.79% -0.16%
==========================================
Files 1042 1052 +10
Lines 56248 56533 +285
Branches 7784 7784
==========================================
+ Hits 43282 43416 +134
- Misses 12707 12858 +151
Partials 259 259
```
| Flag | Coverage Ξ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.90% <75.52%> (-0.06%)` | :arrow_down: |
| presto | `81.78% <75.52%> (-0.06%)` | :arrow_down: |
| python | `82.12% <75.52%> (-0.35%)` | :arrow_down: |
| sqlite | `81.58% <75.52%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.50% <ΓΈ> (ΓΈ)` | |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.49% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/tasks/scheduler.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3Mvc2NoZWR1bGVyLnB5) | `62.96% <33.33%> (-5.93%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `44.11% <44.11%> (ΓΈ)` | |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `57.69% <57.69%> (ΓΈ)` | |
| [superset/key\_value/commands/cleanup.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NsZWFudXAucHk=) | `62.50% <62.50%> (ΓΈ)` | |
| [superset/key\_value/api.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2FwaS5weQ==) | `81.42% <81.42%> (ΓΈ)` | |
| [superset/key\_value/commands/get.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2dldC5weQ==) | `83.33% <83.33%> (ΓΈ)` | |
| [superset/key\_value/dao.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2Rhby5weQ==) | `85.29% <85.29%> (ΓΈ)` | |
| [superset/key\_value/commands/create.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `87.87% <87.87%> (ΓΈ)` | |
| ... and [11 more](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Ξ = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e2a429b...b4a465f](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r749403011
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ include_route_methods = {
+ RouteMethod.POST,
+ RouteMethod.PUT,
+ RouteMethod.GET,
+ RouteMethod.DELETE,
+ }
+ allow_browser_login = True
+ openapi_spec_tag = "Key Value Store"
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+ log_to_statsd=False,
+ )
+ def post(self) -> Response:
+ """Stores a new value.
+ ---
+ post:
+ description: >-
+ Stores a new value.
+ requestBody:
+ description: Key value schema
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ value:
+ type: string
+ description: Any type of JSON supported value.
+ required: true
+ duration_ms:
+ type: number
+ description: The duration of the value on the key store. If no duration is specified the value won't expire.
+ required: false
+ default: null
+ reset_duration_on_retrieval:
+ type: boolean
+ description: If the duration should be reset when the value is retrieved. This is useful if you wish to expire unused values but keep the ones that are actively retrieved.
+ required: false
+ default: true
+ responses:
+ 201:
+ description: The value was stored successfully. It returns the key to retrieve the value.
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ key:
+ type: string
+ description: The key to retrieve the value.
+ 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")
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.
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] michael-s-molina closed pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina closed pull request #17337:
URL: https://github.com/apache/superset/pull/17337
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r747450842
##########
File path: superset/key_value/commands/create.py
##########
@@ -0,0 +1,61 @@
+# 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.
+import logging
+from hashlib import sha3_256
+from typing import Any, Dict, List
+from uuid import uuid4
+
+from flask import current_app
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+from marshmallow import ValidationError
+
+from superset.commands.base import BaseCommand
+from superset.dao.exceptions import DAOException
+from superset.key_value.commands.exceptions import KeyValueCreateFailedError
+from superset.key_value.dao import KeyValueDAO
+from superset.models.key_value import KeyValue
+
+logger = logging.getLogger(__name__)
+
+
+class CreateKeyValueCommand(BaseCommand):
+ def __init__(self, user: User, data: Dict[str, Any]):
+ self._actor = user
+ self._properties = data.copy()
+
+ def run(self) -> Model:
+ try:
+ secretKey = current_app.config["SECRET_KEY"]
+ key = sha3_256(secretKey.encode() + str(uuid4()).encode()).hexdigest()
Review comment:
I think this is a good point. Will replace it with a random seed.
--
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] michael-s-molina commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-964460305
> Thanks @michael-s-molina - if we do implement the "anyone with a key" model, we should throw some restrictions or confirmation around it so it's very clear to the user that they're about to share very widely. Something to think about on the UI-side of the house.
Definitely! We should be really clear about the security implications. I was just checking the Google Drive interface and we can follow the same pattern.
--
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] dpgaspar commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r746576108
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
Review comment:
nit: let's use the `add_model_schema` and `edit_model_schema` attrs, they already exist and their goal is exactly this
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ include_route_methods = {
+ RouteMethod.POST,
+ RouteMethod.PUT,
+ RouteMethod.GET,
+ RouteMethod.DELETE,
+ }
+ allow_browser_login = True
+ openapi_spec_tag = "Key Value Store"
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+ log_to_statsd=False,
+ )
+ def post(self) -> Response:
+ """Stores a new value.
+ ---
+ post:
+ description: >-
+ Stores a new value.
+ requestBody:
+ description: Key value schema
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ value:
+ type: string
+ description: Any type of JSON supported value.
+ required: true
+ duration_ms:
+ type: number
+ description: The duration of the value on the key store. If no duration is specified the value won't expire.
+ required: false
+ default: null
+ reset_duration_on_retrieval:
+ type: boolean
+ description: If the duration should be reset when the value is retrieved. This is useful if you wish to expire unused values but keep the ones that are actively retrieved.
+ required: false
+ default: true
+ responses:
+ 201:
+ description: The value was stored successfully. It returns the key to retrieve the value.
+ content:
+ application/json:
Review comment:
Create a marshmallow schema for the response and reference it here, you can register new marshmallow schemas like this: https://github.com/apache/superset/blob/master/superset/dashboards/api.py#L202
and then reference them like: https://github.com/apache/superset/blob/master/superset/dashboards/api.py#L323
##########
File path: superset/models/key_value.py
##########
@@ -0,0 +1,34 @@
+# 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 datetime import datetime
+
+from flask_appbuilder import Model
+from sqlalchemy import Boolean, Column, DateTime, String, ForeignKey, Integer, Text
+
+
+class KeyValue(Model):
+
+ """Key value store entity"""
+
+ __tablename__ = "key_value"
+ key = Column(String(256), primary_key=True)
+ value = Column(Text, nullable=False)
Review comment:
if `value` is potentially sensitive, we should encrypt this field
##########
File path: superset/key_value/commands/cleanup.py
##########
@@ -0,0 +1,47 @@
+# 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.
+import logging
+from datetime import datetime, timedelta
+
+from superset.commands.base import BaseCommand
+from superset.models.key_value import KeyValue
+from superset.key_value.utils import is_expired
+from superset.utils.celery import session_scope
+
+logger = logging.getLogger(__name__)
+
+
+class CleanupCommand(BaseCommand):
+ """
+ Expiration cleanup command.
+ """
+
+ def __init__(self, worker_context: bool = True):
+ self._worker_context = worker_context
+
+ def run(self) -> None:
+ logger.info("Key value store cleanup starting")
+ with session_scope(nullpool=True) as session:
+ for keyValue in session.query(KeyValue).all():
+ if is_expired(keyValue):
Review comment:
There must be some way to restrict this full table scan `session.query(KeyValue).all()`. What if, when we create/update a key we timestamp an expiration date time then filter by it?
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ include_route_methods = {
+ RouteMethod.POST,
+ RouteMethod.PUT,
+ RouteMethod.GET,
+ RouteMethod.DELETE,
+ }
+ allow_browser_login = True
+ openapi_spec_tag = "Key Value Store"
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+ log_to_statsd=False,
+ )
+ def post(self) -> Response:
+ """Stores a new value.
+ ---
+ post:
+ description: >-
+ Stores a new value.
+ requestBody:
+ description: Key value schema
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
Review comment:
it's shorter to just use a reference to the marshmallow schema like this: https://github.com/apache/superset/blob/master/superset/dashboards/api.py#L427
##########
File path: superset/key_value/commands/create.py
##########
@@ -0,0 +1,61 @@
+# 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.
+import logging
+from hashlib import sha3_256
+from typing import Any, Dict, List
+from uuid import uuid4
+
+from flask import current_app
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+from marshmallow import ValidationError
+
+from superset.commands.base import BaseCommand
+from superset.dao.exceptions import DAOException
+from superset.key_value.commands.exceptions import KeyValueCreateFailedError
+from superset.key_value.dao import KeyValueDAO
+from superset.models.key_value import KeyValue
+
+logger = logging.getLogger(__name__)
+
+
+class CreateKeyValueCommand(BaseCommand):
+ def __init__(self, user: User, data: Dict[str, Any]):
+ self._actor = user
+ self._properties = data.copy()
+
+ def run(self) -> Model:
+ try:
+ secretKey = current_app.config["SECRET_KEY"]
+ key = sha3_256(secretKey.encode() + str(uuid4()).encode()).hexdigest()
Review comment:
probably does not make any diference to the solution itself, but I would prefer to keep the `SECRET_KEY` out of this, think secret key rotation, something that some organisations are already implementing (no impact to the solution but). I think UUID4 over sha3_256 is good enough, UUID4 possible combinations 2^128. We can increase this by using sha3_512 for example
##########
File path: superset/key_value/utils.py
##########
@@ -0,0 +1,27 @@
+# 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 datetime import datetime, timedelta
+from superset.models.key_value import KeyValue
+from sqlalchemy import Boolean
+
+
+def is_expired(keyValue: KeyValue) -> Boolean:
Review comment:
Highly optional, even debatable, but we could implement this on the model itself
##########
File path: superset/key_value/commands/get.py
##########
@@ -0,0 +1,50 @@
+# 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.
+import logging
+from typing import Any, Dict, List
+from datetime import datetime
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+from marshmallow import ValidationError
+
+from superset.key_value.commands.exceptions import KeyValueGetFailedError
+from superset.key_value.dao import KeyValueDAO
+from superset.commands.base import BaseCommand
+from superset.dao.exceptions import DAOException
+
+logger = logging.getLogger(__name__)
+
+
+class GetKeyValueCommand(BaseCommand):
+ def __init__(self, user: User, key: str):
+ self._actor = user
+ self._key = key
+
+ def run(self) -> Model:
+ try:
+ model = KeyValueDAO.find_by_id(self._key)
+ if not model:
+ return None
+ setattr(model, 'retrieved_on', datetime.utcnow())
Review comment:
curious, are you avoiding auditmixin?
--
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] betodealmeida commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r745735010
##########
File path: superset/config.py
##########
@@ -732,6 +732,10 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
"task": "reports.prune_log",
"schedule": crontab(minute=0, hour=0),
},
+ "key_value.cleanup": {
+ "task": "key_value.cleanup",
+ "schedule": timedelta(seconds=10),
Review comment:
How long does it take to run the jobs? 10 seconds looks a bit aggressive to me, maybe we could have a default of ~1 minute to be conservative on resources?
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ include_route_methods = {
+ RouteMethod.POST,
+ RouteMethod.PUT,
+ RouteMethod.GET,
+ RouteMethod.DELETE,
+ }
+ allow_browser_login = True
+ openapi_spec_tag = "Key Value Store"
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+ log_to_statsd=False,
+ )
+ def post(self) -> Response:
+ """Stores a new value.
+ ---
+ post:
+ description: >-
+ Stores a new value.
+ requestBody:
+ description: Key value schema
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ value:
+ type: string
+ description: Any type of JSON supported value.
+ required: true
+ duration_ms:
+ type: number
+ description: The duration of the value on the key store. If no duration is specified the value won't expire.
+ required: false
+ default: null
+ reset_duration_on_retrieval:
+ type: boolean
+ description: If the duration should be reset when the value is retrieved. This is useful if you wish to expire unused values but keep the ones that are actively retrieved.
+ required: false
+ default: true
+ responses:
+ 201:
+ description: The value was stored successfully. It returns the key to retrieve the value.
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ key:
+ type: string
+ description: The key to retrieve the value.
+ 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")
Review comment:
Can we make the error responses comply with SIP-40? https://github.com/apache/superset/issues/9194
We should return a JSON payload with more detail instead of a string whenever an error occurs.
##########
File path: tests/integration_tests/key_value/api_tests.py
##########
@@ -0,0 +1,79 @@
+# 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.
+# isort:skip_file
+import json
+
+from superset import db
+from superset.models.key_value import KeyValue
+from tests.integration_tests.base_tests import SupersetTestCase
+
+duration_ms = 10000
+reset_duration_on_retrieval = True
+value = "test"
+
+
+class KeyValueTests(SupersetTestCase):
+ def post(self):
+ payload = {
+ "duration_ms": duration_ms,
+ "reset_duration_on_retrieval": reset_duration_on_retrieval,
+ "value": value,
+ }
+ resp = self.client.post("api/v1/key_value_store/", json=payload)
+ data = json.loads(resp.data.decode("utf-8"))
+ return data.get("key")
+
+ def setUp(self):
+ self.login(username="admin")
+ rows = db.session.query(KeyValue).all()
+ for row in rows:
+ db.session.delete(row)
+ db.session.commit()
+
+ def test_post(self):
+ key = self.post()
+ result = db.session.query(KeyValue).first()
+ self.assertEqual(key, str(result.key))
Review comment:
Nit: `pytest` works better with naked asserts, eg:
```suggestion
assert key == str(result.key)
```
https://docs.pytest.org/en/latest/how-to/assert.html#assert
##########
File path: superset/key_value/commands/cleanup.py
##########
@@ -0,0 +1,47 @@
+# 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.
+import logging
+from datetime import datetime, timedelta
+
+from superset.commands.base import BaseCommand
+from superset.models.key_value import KeyValue
+from superset.key_value.utils import is_expired
+from superset.utils.celery import session_scope
+
+logger = logging.getLogger(__name__)
+
+
+class CleanupCommand(BaseCommand):
+ """
+ Expiration cleanup command.
+ """
+
+ def __init__(self, worker_context: bool = True):
+ self._worker_context = worker_context
+
+ def run(self) -> None:
+ logger.info("Key value store cleanup starting")
+ with session_scope(nullpool=True) as session:
+ for keyValue in session.query(KeyValue).all():
+ if is_expired(keyValue):
Review comment:
It would be much better for performance to implement the logic of `is_expired` in SQL instead of Python, specially because we already have a query on line 39.
##########
File path: tests/integration_tests/key_value/api_tests.py
##########
@@ -0,0 +1,79 @@
+# 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.
+# isort:skip_file
+import json
+
+from superset import db
+from superset.models.key_value import KeyValue
+from tests.integration_tests.base_tests import SupersetTestCase
+
+duration_ms = 10000
+reset_duration_on_retrieval = True
+value = "test"
+
+
+class KeyValueTests(SupersetTestCase):
+ def post(self):
+ payload = {
+ "duration_ms": duration_ms,
+ "reset_duration_on_retrieval": reset_duration_on_retrieval,
+ "value": value,
+ }
+ resp = self.client.post("api/v1/key_value_store/", json=payload)
+ data = json.loads(resp.data.decode("utf-8"))
+ return data.get("key")
+
+ def setUp(self):
+ self.login(username="admin")
+ rows = db.session.query(KeyValue).all()
+ for row in rows:
+ db.session.delete(row)
+ db.session.commit()
+
+ def test_post(self):
+ key = self.post()
+ result = db.session.query(KeyValue).first()
+ self.assertEqual(key, str(result.key))
+ self.assertEqual(duration_ms, result.duration_ms)
+ self.assertEqual(
+ reset_duration_on_retrieval, result.reset_duration_on_retrieval
+ )
+ self.assertEqual(value, result.value)
+
+ def test_get_not_found(self):
+ key = self.post()
+ resp = self.client.get(key)
+ self.assertEqual(404, resp.status_code)
+
+ def test_get(self):
+ key = self.post()
+ resp = self.client.get(f"api/v1/key_value_store/{key}/")
+ self.assertEqual(resp.status_code, 200)
+ data = json.loads(resp.data.decode("utf-8"))
+ self.assertEqual(duration_ms, data.get("duration_ms"))
+ self.assertEqual(
+ reset_duration_on_retrieval, data.get("reset_duration_on_retrieval")
+ )
+ self.assertEqual(value, data.get("value"))
+
+ def test_get_retrieved_on(self):
Review comment:
Can you use `freezegun` and add a unit test checking that `retrived_on` is set to an expected value?
https://github.com/apache/superset/blob/4dc859f89e5668ed3f94cd1ef0532a301a3ab85a/tests/integration_tests/reports/scheduler_tests.py#L45
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ include_route_methods = {
+ RouteMethod.POST,
+ RouteMethod.PUT,
+ RouteMethod.GET,
+ RouteMethod.DELETE,
+ }
+ allow_browser_login = True
+ openapi_spec_tag = "Key Value Store"
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+ log_to_statsd=False,
+ )
+ def post(self) -> Response:
+ """Stores a new value.
+ ---
+ post:
+ description: >-
+ Stores a new value.
+ requestBody:
+ description: Key value schema
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ value:
+ type: string
+ description: Any type of JSON supported value.
+ required: true
+ duration_ms:
+ type: number
+ description: The duration of the value on the key store. If no duration is specified the value won't expire.
+ required: false
+ default: null
+ reset_duration_on_retrieval:
+ type: boolean
+ description: If the duration should be reset when the value is retrieved. This is useful if you wish to expire unused values but keep the ones that are actively retrieved.
+ required: false
+ default: true
+ responses:
+ 201:
+ description: The value was stored successfully. It returns the key to retrieve the value.
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ key:
+ type: string
+ description: The key to retrieve the value.
+ 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")
+ try:
+ item = self.schema.load(request.json)
+ key = CreateKeyValueCommand(g.user, item).run()
+ return self.response(201, key=key)
+ except KeyValueCreateFailedError as ex:
+ logger.error(
+ "Error creating value %s: %s",
+ self.__class__.__name__,
+ str(ex),
+ exc_info=True,
+ )
+ return self.response_422(message=str(ex))
Review comment:
Same here an in other methods below.
--
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] michael-s-molina commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-964449300
> Thanks for the response @michael-s-molina - I think as Superset moves into more organizations we should default to closed in all cases, so I would support the closed-sharing model. The open-sharing model is pretty dangerous - it becomes easy to create a link that's accessible to everyone in a company, which I believe wouldn't be a desired behavior by most larger organizations as a default behavior. In Superset, URL parameters, application state, and cache can contain highly sensitive information so I think we should shy away from the open-sharing model in all cases.
@willbarrett These are good points. I agree that changing the default to the restricted model is more appropriate. I also think we should support the "Anyone with the key" model because we have some resources like public dashboards where we can benefit from it and we don't need the whole security configuration part from the user. I'll ping @dpgaspar to discuss this and increment the PR with these requirements. Thank you so much again!
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r747765147
##########
File path: superset/key_value/commands/cleanup.py
##########
@@ -0,0 +1,47 @@
+# 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.
+import logging
+from datetime import datetime, timedelta
+
+from superset.commands.base import BaseCommand
+from superset.models.key_value import KeyValue
+from superset.key_value.utils import is_expired
+from superset.utils.celery import session_scope
+
+logger = logging.getLogger(__name__)
+
+
+class CleanupCommand(BaseCommand):
+ """
+ Expiration cleanup command.
+ """
+
+ def __init__(self, worker_context: bool = True):
+ self._worker_context = worker_context
+
+ def run(self) -> None:
+ logger.info("Key value store cleanup starting")
+ with session_scope(nullpool=True) as session:
+ for keyValue in session.query(KeyValue).all():
+ if is_expired(keyValue):
Review comment:
I'm on it. A DELETE with a WHERE clause and a simple condition will be sufficient.
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r748292431
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
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.
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] codecov[bot] commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-969016316
# [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17337](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f899453) into [master](https://codecov.io/gh/apache/superset/commit/e2a429b0c8042ba867f834f5dc5561d1e402289b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2a429b) will **decrease** coverage by `0.24%`.
> The diff coverage is `75.52%`.
> :exclamation: Current head f899453 differs from pull request most recent head c32bfeb. Consider uploading reports for the commit c32bfeb to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17337 +/- ##
==========================================
- Coverage 76.94% 76.70% -0.25%
==========================================
Files 1042 1052 +10
Lines 56248 56533 +285
Branches 7784 7784
==========================================
+ Hits 43282 43365 +83
- Misses 12707 12909 +202
Partials 259 259
```
| Flag | Coverage Ξ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.90% <75.52%> (-0.06%)` | :arrow_down: |
| presto | `?` | |
| python | `81.95% <75.52%> (-0.52%)` | :arrow_down: |
| sqlite | `81.58% <75.52%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.50% <ΓΈ> (ΓΈ)` | |
| [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `69.49% <0.00%> (-16.99%)` | :arrow_down: |
| [superset/tasks/scheduler.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3Mvc2NoZWR1bGVyLnB5) | `62.96% <33.33%> (-5.93%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `44.11% <44.11%> (ΓΈ)` | |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `57.69% <57.69%> (ΓΈ)` | |
| [superset/key\_value/commands/cleanup.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NsZWFudXAucHk=) | `62.50% <62.50%> (ΓΈ)` | |
| [superset/key\_value/api.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2FwaS5weQ==) | `81.42% <81.42%> (ΓΈ)` | |
| [superset/key\_value/commands/get.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2dldC5weQ==) | `83.33% <83.33%> (ΓΈ)` | |
| [superset/key\_value/dao.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2Rhby5weQ==) | `85.29% <85.29%> (ΓΈ)` | |
| [superset/key\_value/commands/create.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `87.87% <87.87%> (ΓΈ)` | |
| ... and [14 more](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Ξ = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e2a429b...c32bfeb](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] michael-s-molina commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-971588829
We had a meeting about the proposed solution in this PR and decided to take a slightly different approach to resolve the use cases. Exposing a generic key-value interface had the potential to induce users to store any type of information in the store, diminishing the benefits of the current endpoints where each type of resource is represented by a specific path. We could also introduce some conflicts with the current security model in terms of access grants. One example would be to store information in the key-value store that is related to a dashboard. The dashboard already has configured access grants and re-defining those in each key-value entry had the potential for security problems. We decided to offer the same type of functionality under each currently defined endpoint to preserve the more typed nature of our endpoints and to leverage the existing security model. I'm closing this PR to preserve all the discussions and conclusions for historical reasons and I'll open a new
one with the new approach.
A big thanks to all the reviewers that are helping shape this new feature.
--
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] codecov[bot] edited a comment on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-969016316
# [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17337](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b4a465f) into [master](https://codecov.io/gh/apache/superset/commit/e2a429b0c8042ba867f834f5dc5561d1e402289b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2a429b) will **decrease** coverage by `0.08%`.
> The diff coverage is `75.78%`.
[](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17337 +/- ##
==========================================
- Coverage 76.94% 76.86% -0.09%
==========================================
Files 1042 1052 +10
Lines 56248 56533 +285
Branches 7784 7784
==========================================
+ Hits 43282 43454 +172
- Misses 12707 12820 +113
Partials 259 259
```
| Flag | Coverage Ξ | |
|---|---|---|
| hive | `81.48% <75.78%> (-0.06%)` | :arrow_down: |
| mysql | `81.89% <75.78%> (-0.06%)` | :arrow_down: |
| postgres | `81.90% <75.78%> (-0.06%)` | :arrow_down: |
| presto | `?` | |
| python | `82.25% <75.78%> (-0.22%)` | :arrow_down: |
| sqlite | `81.58% <75.78%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.50% <ΓΈ> (ΓΈ)` | |
| [superset/tasks/scheduler.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3Mvc2NoZWR1bGVyLnB5) | `62.96% <33.33%> (-5.93%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `44.11% <44.11%> (ΓΈ)` | |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `57.69% <57.69%> (ΓΈ)` | |
| [superset/key\_value/commands/cleanup.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NsZWFudXAucHk=) | `62.50% <62.50%> (ΓΈ)` | |
| [superset/key\_value/api.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2FwaS5weQ==) | `81.42% <81.42%> (ΓΈ)` | |
| [superset/key\_value/commands/get.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2dldC5weQ==) | `83.33% <83.33%> (ΓΈ)` | |
| [superset/key\_value/dao.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2Rhby5weQ==) | `85.29% <85.29%> (ΓΈ)` | |
| [superset/key\_value/commands/create.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `87.87% <87.87%> (ΓΈ)` | |
| [superset/initialization/\_\_init\_\_.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvaW5pdGlhbGl6YXRpb24vX19pbml0X18ucHk=) | `88.07% <100.00%> (+0.08%)` | :arrow_up: |
| ... and [6 more](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Ξ = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e2a429b...b4a465f](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r748292536
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ include_route_methods = {
+ RouteMethod.POST,
+ RouteMethod.PUT,
+ RouteMethod.GET,
+ RouteMethod.DELETE,
+ }
+ allow_browser_login = True
+ openapi_spec_tag = "Key Value Store"
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+ log_to_statsd=False,
+ )
+ def post(self) -> Response:
+ """Stores a new value.
+ ---
+ post:
+ description: >-
+ Stores a new value.
+ requestBody:
+ description: Key value schema
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
Review comment:
Done
##########
File path: superset/key_value/api.py
##########
@@ -0,0 +1,324 @@
+# 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.
+import logging
+
+from flask import g, request, Response
+from flask_appbuilder.api import expose, permission_name, protect, safe
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+
+from superset.constants import MODEL_API_RW_METHOD_PERMISSION_MAP, RouteMethod
+from superset.extensions import event_logger
+from superset.key_value.commands.create import CreateKeyValueCommand
+from superset.key_value.commands.delete import DeleteKeyValueCommand
+from superset.key_value.commands.exceptions import (
+ KeyValueCreateFailedError,
+ KeyValueDeleteFailedError,
+ KeyValueGetFailedError,
+ KeyValueUpdateFailedError,
+)
+from superset.key_value.commands.get import GetKeyValueCommand
+from superset.key_value.commands.update import UpdateKeyValueCommand
+from superset.key_value.dao import KeyValueDAO
+from superset.key_value.schemas import KeyValueSchema
+from superset.models.key_value import KeyValue
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+
+logger = logging.getLogger(__name__)
+
+
+class KeyValueRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(KeyValue)
+ schema = KeyValueSchema()
+ class_permission_name = "KeyValue"
+ resource_name = "key_value_store"
+ method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP
+ include_route_methods = {
+ RouteMethod.POST,
+ RouteMethod.PUT,
+ RouteMethod.GET,
+ RouteMethod.DELETE,
+ }
+ allow_browser_login = True
+ openapi_spec_tag = "Key Value Store"
+
+ @expose("/", methods=["POST"])
+ @protect()
+ @safe
+ @statsd_metrics
+ @event_logger.log_this_with_context(
+ action=lambda self, *args, **kwargs: f"{self.__class__.__name__}.post",
+ log_to_statsd=False,
+ )
+ def post(self) -> Response:
+ """Stores a new value.
+ ---
+ post:
+ description: >-
+ Stores a new value.
+ requestBody:
+ description: Key value schema
+ required: true
+ content:
+ application/json:
+ schema:
+ type: object
+ properties:
+ value:
+ type: string
+ description: Any type of JSON supported value.
+ required: true
+ duration_ms:
+ type: number
+ description: The duration of the value on the key store. If no duration is specified the value won't expire.
+ required: false
+ default: null
+ reset_duration_on_retrieval:
+ type: boolean
+ description: If the duration should be reset when the value is retrieved. This is useful if you wish to expire unused values but keep the ones that are actively retrieved.
+ required: false
+ default: true
+ responses:
+ 201:
+ description: The value was stored successfully. It returns the key to retrieve the value.
+ content:
+ application/json:
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.
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] geido commented on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
geido commented on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-971492086
Awesome work @michael-s-molina!
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r745768184
##########
File path: superset/config.py
##########
@@ -732,6 +732,10 @@ class CeleryConfig: # pylint: disable=too-few-public-methods
"task": "reports.prune_log",
"schedule": crontab(minute=0, hour=0),
},
+ "key_value.cleanup": {
+ "task": "key_value.cleanup",
+ "schedule": timedelta(seconds=10),
Review comment:
I forgot to change that π€ͺ. I was just using 10 seconds for testing but the default should be 24 hours.
--
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] codecov[bot] edited a comment on pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #17337:
URL: https://github.com/apache/superset/pull/17337#issuecomment-969016316
# [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17337](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b4a465f) into [master](https://codecov.io/gh/apache/superset/commit/e2a429b0c8042ba867f834f5dc5561d1e402289b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e2a429b) will **decrease** coverage by `0.22%`.
> The diff coverage is `75.78%`.
[](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #17337 +/- ##
==========================================
- Coverage 76.94% 76.72% -0.23%
==========================================
Files 1042 1052 +10
Lines 56248 56533 +285
Branches 7784 7784
==========================================
+ Hits 43282 43374 +92
- Misses 12707 12900 +193
Partials 259 259
```
| Flag | Coverage Ξ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.89% <75.78%> (-0.06%)` | :arrow_down: |
| postgres | `81.89% <75.78%> (-0.08%)` | :arrow_down: |
| presto | `?` | |
| python | `81.98% <75.78%> (-0.49%)` | :arrow_down: |
| sqlite | `81.58% <75.78%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Ξ | |
|---|---|---|
| [superset/config.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvY29uZmlnLnB5) | `91.50% <ΓΈ> (ΓΈ)` | |
| [superset/tasks/scheduler.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFza3Mvc2NoZWR1bGVyLnB5) | `62.96% <33.33%> (-5.93%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `44.11% <44.11%> (ΓΈ)` | |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `57.69% <57.69%> (ΓΈ)` | |
| [superset/key\_value/commands/cleanup.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NsZWFudXAucHk=) | `62.50% <62.50%> (ΓΈ)` | |
| [superset/key\_value/api.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2FwaS5weQ==) | `81.42% <81.42%> (ΓΈ)` | |
| [superset/key\_value/commands/get.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2dldC5weQ==) | `83.33% <83.33%> (ΓΈ)` | |
| [superset/key\_value/dao.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2Rhby5weQ==) | `85.29% <85.29%> (ΓΈ)` | |
| [superset/key\_value/commands/create.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `87.87% <87.87%> (ΓΈ)` | |
| [superset/initialization/\_\_init\_\_.py](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvaW5pdGlhbGl6YXRpb24vX19pbml0X18ucHk=) | `88.07% <100.00%> (+0.08%)` | :arrow_up: |
| ... and [14 more](https://codecov.io/gh/apache/superset/pull/17337/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Ξ = absolute <relative> (impact)`, `ΓΈ = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [e2a429b...b4a465f](https://codecov.io/gh/apache/superset/pull/17337?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] michael-s-molina commented on a change in pull request #17337: feat: Adds a key-value store endpoint for Superset
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on a change in pull request #17337:
URL: https://github.com/apache/superset/pull/17337#discussion_r749402799
##########
File path: tests/integration_tests/key_value/api_tests.py
##########
@@ -0,0 +1,79 @@
+# 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.
+# isort:skip_file
+import json
+
+from superset import db
+from superset.models.key_value import KeyValue
+from tests.integration_tests.base_tests import SupersetTestCase
+
+duration_ms = 10000
+reset_duration_on_retrieval = True
+value = "test"
+
+
+class KeyValueTests(SupersetTestCase):
+ def post(self):
+ payload = {
+ "duration_ms": duration_ms,
+ "reset_duration_on_retrieval": reset_duration_on_retrieval,
+ "value": value,
+ }
+ resp = self.client.post("api/v1/key_value_store/", json=payload)
+ data = json.loads(resp.data.decode("utf-8"))
+ return data.get("key")
+
+ def setUp(self):
+ self.login(username="admin")
+ rows = db.session.query(KeyValue).all()
+ for row in rows:
+ db.session.delete(row)
+ db.session.commit()
+
+ def test_post(self):
+ key = self.post()
+ result = db.session.query(KeyValue).first()
+ self.assertEqual(key, str(result.key))
+ self.assertEqual(duration_ms, result.duration_ms)
+ self.assertEqual(
+ reset_duration_on_retrieval, result.reset_duration_on_retrieval
+ )
+ self.assertEqual(value, result.value)
+
+ def test_get_not_found(self):
+ key = self.post()
+ resp = self.client.get(key)
+ self.assertEqual(404, resp.status_code)
+
+ def test_get(self):
+ key = self.post()
+ resp = self.client.get(f"api/v1/key_value_store/{key}/")
+ self.assertEqual(resp.status_code, 200)
+ data = json.loads(resp.data.decode("utf-8"))
+ self.assertEqual(duration_ms, data.get("duration_ms"))
+ self.assertEqual(
+ reset_duration_on_retrieval, data.get("reset_duration_on_retrieval")
+ )
+ self.assertEqual(value, data.get("value"))
+
+ def test_get_retrieved_on(self):
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.
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