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/04/14 12:16:34 UTC
[GitHub] [superset] ofekisr opened a new pull request #14015: feat(filter-set): Add filterset resource
ofekisr opened a new pull request #14015:
URL: https://github.com/apache/superset/pull/14015
### SUMMARY
this is the second milestone in the filter set feature.
it is about managing a scope for filter set either Dashboard/User
1. Dashboard scoped filter sets will be available for all dashboard permitted user and will be maintained with admins dashboard owners
2. User scoped filter sets will be private for each user separately
implementation:
a subset API of dashboard ` "/<dashboard_id>/filtersets"` for managing a dashboard
notes:
future milestone will deal with sharing and collaborating on filter-sets
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TEST PLAN
api_tests.py test suite
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Changes UI
- [X] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [X ] Migration is atomic, supports rollback & is backwards-compatible
- [X] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [X] 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.
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-818969938
⚠️ @ofekisr 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.
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-892906001
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-909435573
@suddjian Ephemeral environment spinning up at http://54.148.252.117:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r707500847
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
The dashboard model uses JSON as well and it is very convenient to use... If I expect to Json object why not use JSON?
why do you use int and not everything as string?
Please explain to us why JSON is bad practice? how are you sure Text is ok? If you trust sqlalchemy you should use their interface.
Should Superset support as administration Database others than MySQL, Postgress, SQLite, and Microsoft SQL Server 2016? if yes how does Superset support it until now since the Dashboard model uses Json as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r701338423
##########
File path: tests/integration_tests/dashboards/superset_factory_util.py
##########
@@ -145,11 +160,14 @@ def create_datasource_table_to_db(
def create_datasource_table(
name: Optional[str] = None,
db_id: Optional[int] = None,
+ database: Optional[Database] = None,
owners: Optional[List[User]] = None,
) -> SqlaTable:
- name = name or random_str()
- owners = owners or []
- db_id = db_id or create_database_to_db(name=name + "_db").id
+ name = name if name is not None else random_str()
+ owners = owners if owners is not None else []
Review comment:
I see a lot of these changes from an 'or 'statement to explicitly checking for None. It doesn't look like there are other falsy values that you would have, I'm curious what the advantage of this change is?
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (414b0c0) into [master](https://codecov.io/gh/apache/superset/commit/1cc726364a5f4ec61bb8af673cc0cd27c044ba87?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1cc7263) will **decrease** coverage by `0.05%`.
> The diff coverage is `90.12%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.93% 76.87% -0.06%
==========================================
Files 1005 1020 +15
Lines 54049 54618 +569
Branches 7339 7339
==========================================
+ Hits 41581 41989 +408
- Misses 12228 12389 +161
Partials 240 240
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.82% <89.91%> (+0.21%)` | :arrow_up: |
| postgres | `81.93% <90.12%> (+0.21%)` | :arrow_up: |
| presto | `?` | |
| python | `82.01% <90.12%> (-0.22%)` | :arrow_down: |
| sqlite | `81.50% <89.91%> (+0.21%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `93.87% <93.87%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [69 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1cc7263...414b0c0](https://codecov.io/gh/apache/superset/pull/14015?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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-885280163
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-909435573
@suddjian Ephemeral environment spinning up at http://54.148.252.117:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-900697612
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-882958194
⚠️ @ofekisr 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] villebro commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r709315163
##########
File path: superset/dashboards/filter_sets/commands/base.py
##########
@@ -0,0 +1,95 @@
+# 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 cast, Optional
+
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+
+from superset.commands.base import BaseCommand
+from superset.common.not_authrized_object import NotAuthorizedException
+from superset.common.request_contexed_based import is_user_admin
+from superset.dashboards.commands.exceptions import DashboardNotFoundError
+from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_sets.commands.exceptions import (
+ FilterSetForbiddenError,
+ FilterSetNotFoundError,
+)
+from superset.dashboards.filter_sets.consts import USER_OWNER_TYPE
+from superset.models.dashboard import Dashboard
+from superset.models.filter_set import FilterSet
+
+logger = logging.getLogger(__name__)
+
+
+class BaseFilterSetCommand(BaseCommand):
+ # pylint: disable=C0103
+ _dashboard: Dashboard
+ _filter_set_id: Optional[int]
+ _filter_set: Optional[FilterSet]
+
+ def __init__(self, user: User, dashboard_id: int):
+ self._actor = user
+ self._is_actor_admin = is_user_admin()
+ self._dashboard_id = dashboard_id
+
+ def run(self) -> Model:
+ pass
+
+ def validate(self) -> None:
+ self._validate_filterset_dashboard_exists()
+
+ def _validate_filterset_dashboard_exists(self) -> None:
+ self._dashboard = DashboardDAO.get_by_id_or_slug(str(self._dashboard_id))
+ if not self._dashboard:
+ raise DashboardNotFoundError()
Review comment:
I would recommend not introducing abstractions before they are needed. If `validate` grows out of proportion it totally makes sense to break out the separate validators (we have linting rules for this), but until then I suggest just keeping it all in `validate()` with a comment explaining what each step is about if it's really difficult to understand (probably not needed here).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2674d9a) into [master](https://codecov.io/gh/apache/superset/commit/1cc726364a5f4ec61bb8af673cc0cd27c044ba87?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1cc7263) will **decrease** coverage by `0.05%`.
> The diff coverage is `90.12%`.
> :exclamation: Current head 2674d9a differs from pull request most recent head 414b0c0. Consider uploading reports for the commit 414b0c0 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.93% 76.87% -0.06%
==========================================
Files 1005 1020 +15
Lines 54049 54618 +569
Branches 7339 7339
==========================================
+ Hits 41581 41989 +408
- Misses 12228 12389 +161
Partials 240 240
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.82% <89.91%> (+0.21%)` | :arrow_up: |
| postgres | `81.89% <90.12%> (+0.17%)` | :arrow_up: |
| presto | `?` | |
| python | `82.01% <90.12%> (-0.22%)` | :arrow_down: |
| sqlite | `81.54% <89.91%> (+0.25%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `93.87% <93.87%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [69 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1cc7263...414b0c0](https://codecov.io/gh/apache/superset/pull/14015?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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-892906001
⚠️ @ofekisr 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] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (55dbfa4) into [master](https://codecov.io/gh/apache/superset/commit/1ffd73d03b5c89d2206cadee42bb0f761233dfe2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1ffd73d) will **decrease** coverage by `0.07%`.
> The diff coverage is `90.41%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.70% 76.63% -0.08%
==========================================
Files 1002 1016 +14
Lines 53680 54324 +644
Branches 6852 6852
==========================================
+ Hits 41177 41629 +452
- Misses 12266 12458 +192
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `?` | |
| postgres | `81.74% <90.41%> (+0.19%)` | :arrow_up: |
| presto | `?` | |
| python | `81.78% <90.41%> (-0.28%)` | :arrow_down: |
| sqlite | `81.35% <90.23%> (+0.15%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `77.96% <77.96%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [45 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1ffd73d...55dbfa4](https://codecov.io/gh/apache/superset/pull/14015?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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r707259615
##########
File path: superset/commands/exceptions.py
##########
@@ -31,6 +31,26 @@ def __repr__(self) -> str:
return repr(self)
+class ObjectNotFoundError(CommandException):
+ status = 404
+ message_format = "{} {}not found."
+
+ def __init__(
+ self,
+ object_type: str,
+ object_id: Optional[str] = None,
+ exception: Optional[Exception] = None,
+ ) -> None:
+ super().__init__(
+ _(
+ self.message_format.format(
+ object_type, '"%s" ' % object_id if object_id else ""
+ )
+ ),
Review comment:
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-891286186
⚠️ @ofekisr 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] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d500272) into [master](https://codecov.io/gh/apache/superset/commit/62d8ab7f9c044e2887de794484c30c2108213e0b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62d8ab7) will **decrease** coverage by `0.02%`.
> The diff coverage is `90.41%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.63% 76.60% -0.03%
==========================================
Files 1002 1015 +13
Lines 53680 54155 +475
Branches 6852 6852
==========================================
+ Hits 41138 41488 +350
- Misses 12305 12430 +125
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.66% <90.23%> (+0.17%)` | :arrow_up: |
| postgres | `81.69% <90.41%> (+0.14%)` | :arrow_up: |
| python | `81.77% <90.41%> (-0.14%)` | :arrow_down: |
| sqlite | `81.29% <90.23%> (+0.10%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `77.96% <77.96%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [30 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [62d8ab7...d500272](https://codecov.io/gh/apache/superset/pull/14015?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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (51e2084) into [master](https://codecov.io/gh/apache/superset/commit/997320ac1a969bbdf97c4a5cb9cef04fff590f3a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (997320a) will **increase** coverage by `0.03%`.
> The diff coverage is `90.14%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
+ Coverage 76.89% 76.93% +0.03%
==========================================
Files 1005 1018 +13
Lines 54169 54660 +491
Branches 7456 7456
==========================================
+ Hits 41653 42050 +397
- Misses 12272 12366 +94
Partials 244 244
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.45% <90.14%> (+0.10%)` | :arrow_up: |
| mysql | `81.88% <89.93%> (+0.16%)` | :arrow_up: |
| postgres | `81.94% <90.14%> (+0.13%)` | :arrow_up: |
| presto | `?` | |
| python | `82.30% <90.14%> (-0.03%)` | :arrow_down: |
| sqlite | `81.56% <89.93%> (+0.13%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `93.87% <93.87%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [26 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [997320a...51e2084](https://codecov.io/gh/apache/superset/pull/14015?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] amitmiran137 commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r699559909
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
Ok so a bit context here .
The purpose of this field is for storing the list of native-filters config as they are stored today under the Dashboard metadata.
About the field name , I'm ok with anything we decide .
Let's see what others like .
@villebro any thoughts?
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-841560256
⚠️ @ofekisr 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.
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-878023586
⚠️ @ofekisr 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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r705439941
##########
File path: superset/dashboards/filter_sets/commands/base.py
##########
@@ -0,0 +1,95 @@
+# 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 cast, Optional
+
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+
+from superset.commands.base import BaseCommand
+from superset.common.not_authrized_object import NotAuthorizedException
+from superset.common.request_contexed_based import is_user_admin
+from superset.dashboards.commands.exceptions import DashboardNotFoundError
+from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_sets.commands.exceptions import (
+ FilterSetForbiddenError,
+ FilterSetNotFoundError,
+)
+from superset.dashboards.filter_sets.consts import USER_OWNER_TYPE
+from superset.models.dashboard import Dashboard
+from superset.models.filter_set import FilterSet
+
+logger = logging.getLogger(__name__)
+
+
+class BaseFilterSetCommand(BaseCommand):
+ # pylint: disable=C0103
+ _dashboard: Dashboard
+ _filter_set_id: Optional[int]
+ _filter_set: Optional[FilterSet]
+
+ def __init__(self, user: User, dashboard_id: int):
+ self._actor = user
+ self._is_actor_admin = is_user_admin()
+ self._dashboard_id = dashboard_id
+
+ def run(self) -> Model:
+ pass
+
+ def validate(self) -> None:
+ self._validate_filterset_dashboard_exists()
+
+ def _validate_filterset_dashboard_exists(self) -> None:
+ self._dashboard = DashboardDAO.get_by_id_or_slug(str(self._dashboard_id))
+ if not self._dashboard:
+ raise DashboardNotFoundError()
Review comment:
validate is coming from the base... and could be consists several of validators
if you see only "validate" you can understand what is validate that is why, but you can understand what is "validate filterset dashboard exists
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-882958194
⚠️ @ofekisr 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] ofekisr closed pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr closed pull request #14015:
URL: https://github.com/apache/superset/pull/14015
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-894394569
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-925604553
Ephemeral environment shutdown and build artifacts deleted.
--
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] amitmiran137 commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r613112547
##########
File path: superset/app.py
##########
@@ -523,6 +523,9 @@ def init_views(self) -> None:
icon="fa-cog",
)
appbuilder.add_separator("Data")
+ from superset.dashboards.filter_sets.api import FilterSetRestApi
+
+ appbuilder.add_api(FilterSetRestApi)
Review comment:
```suggestion
if feature_flag_manager.is_feature_enabled("DASHBOARD_NATIVE_FILTERS_SET"):
from superset.dashboards.filter_sets.api import FilterSetRestApi
appbuilder.add_api(FilterSetRestApi)
```
Lets add it under the feature flag
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] amitmiran137 merged pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
amitmiran137 merged pull request #14015:
URL: https://github.com/apache/superset/pull/14015
--
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] suddjian commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r699533208
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
We should avoid including the type as part of the column/variable name. Also, if possible can we find a more descriptive name than `metadata`? I'm not sure what all is contained in `metadata`. Perhaps `configuration` or `definition`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
eschutho commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-911962678
Is there anything that we can test for on this PR in the virtual env? I see that most of the changes are in the backend, so not sure if anything is visibly changed in the app.
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-896131077
⚠️ @ofekisr 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] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2127177) into [master](https://codecov.io/gh/apache/superset/commit/62d8ab7f9c044e2887de794484c30c2108213e0b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62d8ab7) will **increase** coverage by `0.11%`.
> The diff coverage is `90.41%`.
> :exclamation: Current head 2127177 differs from pull request most recent head d500272. Consider uploading reports for the commit d500272 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
+ Coverage 76.63% 76.75% +0.11%
==========================================
Files 1002 1015 +13
Lines 53680 54155 +475
Branches 6852 6852
==========================================
+ Hits 41138 41567 +429
- Misses 12305 12351 +46
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.38% <90.41%> (+0.10%)` | :arrow_up: |
| mysql | `?` | |
| postgres | `?` | |
| presto | `81.52% <90.41%> (?)` | |
| python | `82.05% <90.41%> (+0.14%)` | :arrow_up: |
| sqlite | `81.29% <90.23%> (+0.10%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `77.96% <77.96%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [36 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [62d8ab7...d500272](https://codecov.io/gh/apache/superset/pull/14015?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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (07792fa) into [master](https://codecov.io/gh/apache/superset/commit/1ffd73d03b5c89d2206cadee42bb0f761233dfe2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1ffd73d) will **decrease** coverage by `0.09%`.
> The diff coverage is `90.41%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.70% 76.60% -0.10%
==========================================
Files 1002 1015 +13
Lines 53680 54155 +475
Branches 6852 6852
==========================================
+ Hits 41177 41488 +311
- Misses 12266 12430 +164
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.62% <90.23%> (+0.13%)` | :arrow_up: |
| postgres | `81.69% <90.41%> (+0.14%)` | :arrow_up: |
| presto | `?` | |
| python | `81.77% <90.41%> (-0.29%)` | :arrow_down: |
| sqlite | `81.29% <90.23%> (+0.10%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `77.96% <77.96%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [31 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1ffd73d...07792fa](https://codecov.io/gh/apache/superset/pull/14015?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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-821278850
⚠️ @ofekisr 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.
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-884393501
⚠️ @ofekisr 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] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fd6deca) into [master](https://codecov.io/gh/apache/superset/commit/1cc726364a5f4ec61bb8af673cc0cd27c044ba87?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1cc7263) will **decrease** coverage by `0.07%`.
> The diff coverage is `90.20%`.
> :exclamation: Current head fd6deca differs from pull request most recent head 4b0a9ed. Consider uploading reports for the commit 4b0a9ed to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.93% 76.85% -0.08%
==========================================
Files 1005 1020 +15
Lines 54049 54605 +556
Branches 7339 7339
==========================================
+ Hits 41581 41966 +385
- Misses 12228 12399 +171
Partials 240 240
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.82% <90.00%> (+0.20%)` | :arrow_up: |
| postgres | `81.85% <90.20%> (+0.13%)` | :arrow_up: |
| presto | `?` | |
| python | `81.97% <90.20%> (-0.26%)` | :arrow_down: |
| sqlite | `81.46% <90.00%> (+0.17%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [45 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1cc7263...4b0a9ed](https://codecov.io/gh/apache/superset/pull/14015?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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-883792294
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-893557099
⚠️ @ofekisr 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] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (51e2084) into [master](https://codecov.io/gh/apache/superset/commit/997320ac1a969bbdf97c4a5cb9cef04fff590f3a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (997320a) will **decrease** coverage by `0.10%`.
> The diff coverage is `90.14%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.89% 76.78% -0.11%
==========================================
Files 1005 1018 +13
Lines 54169 54660 +491
Branches 7456 7456
==========================================
+ Hits 41653 41973 +320
- Misses 12272 12443 +171
Partials 244 244
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.88% <89.93%> (+0.16%)` | :arrow_up: |
| postgres | `81.94% <90.14%> (+0.13%)` | :arrow_up: |
| presto | `?` | |
| python | `82.02% <90.14%> (-0.30%)` | :arrow_down: |
| sqlite | `81.56% <89.93%> (+0.13%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `93.87% <93.87%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [31 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [997320a...51e2084](https://codecov.io/gh/apache/superset/pull/14015?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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-916266596
⚠️ @ofekisr 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] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2127177) into [master](https://codecov.io/gh/apache/superset/commit/1ffd73d03b5c89d2206cadee42bb0f761233dfe2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1ffd73d) will **increase** coverage by `0.26%`.
> The diff coverage is `90.41%`.
> :exclamation: Current head 2127177 differs from pull request most recent head 6762eb5. Consider uploading reports for the commit 6762eb5 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
+ Coverage 76.49% 76.75% +0.26%
==========================================
Files 1002 1015 +13
Lines 53680 54155 +475
Branches 6852 6852
==========================================
+ Hits 41061 41567 +506
+ Misses 12382 12351 -31
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.38% <90.41%> (?)` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `81.52% <90.41%> (?)` | |
| python | `82.05% <90.41%> (+0.42%)` | :arrow_up: |
| sqlite | `81.29% <90.23%> (+0.10%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `77.96% <77.96%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [40 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1ffd73d...6762eb5](https://codecov.io/gh/apache/superset/pull/14015?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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-820492294
⚠️ @ofekisr 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r701338423
##########
File path: tests/integration_tests/dashboards/superset_factory_util.py
##########
@@ -145,11 +160,14 @@ def create_datasource_table_to_db(
def create_datasource_table(
name: Optional[str] = None,
db_id: Optional[int] = None,
+ database: Optional[Database] = None,
owners: Optional[List[User]] = None,
) -> SqlaTable:
- name = name or random_str()
- owners = owners or []
- db_id = db_id or create_database_to_db(name=name + "_db").id
+ name = name if name is not None else random_str()
+ owners = owners if owners is not None else []
Review comment:
I see a lot of these changes from an 'or 'statement to explicitly checking for None. It doesn't look like there are other falsy values that you would have, I'm curious what the advantage of this change is?
--
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] suddjian commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r699533208
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
We should avoid including the `json` type as part of the column/variable name. Also, if possible can we find a more descriptive name than `metadata`? I'm not sure what all is contained in `metadata`. Perhaps `configuration` or `definition`?
--
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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r705444055
##########
File path: superset/models/filter_set.py
##########
@@ -0,0 +1,99 @@
+# 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 __future__ import annotations
+
+import logging
+from typing import Any, Dict
+
+from flask_appbuilder import Model
+from sqlalchemy import Column, ForeignKey, Integer, JSON, MetaData, String, Text
+from sqlalchemy.orm import relationship
+from sqlalchemy_utils import generic_relationship
+
+from superset import app, db
+from superset.models.helpers import AuditMixinNullable
+
+metadata = Model.metadata # pylint: disable=no-member
+config = app.config
+logger = logging.getLogger(__name__)
+
+
+class FilterSet(Model, AuditMixinNullable):
+ __tablename__ = "filter_sets"
+ id = Column(Integer, primary_key=True)
+ name = Column(String(500), nullable=False, unique=True)
+ description = Column(Text, nullable=True)
+ json_metadata = Column(JSON, nullable=False)
Review comment:
I didn't investigate what is better, but if you think Text is better then JSON, it should be in separate PR that change all JSON to TXT and not only here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-884493300
⚠️ @ofekisr 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] dpgaspar commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r612550418
##########
File path: superset/dashboards/filter_sets/api.py
##########
@@ -0,0 +1,147 @@
+# 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 Response, request, g
+from flask_appbuilder.api import expose, protect, safe, rison, permission_name, merge_response_func, \
+ get_list_schema, API_ORDER_COLUMNS_RIS_KEY, API_LABEL_COLUMNS_RIS_KEY, \
+ API_DESCRIPTION_COLUMNS_RIS_KEY, API_LIST_COLUMNS_RIS_KEY, API_LIST_TITLE_RIS_KEY, ModelRestApi
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from marshmallow import ValidationError
+from superset import is_feature_enabled
+from superset.commands.exceptions import ObjectNotFoundError
+from superset.dashboards.commands.exceptions import DashboardNotFoundError
+from superset.dashboards.filter_sets.commands.exceptions import FilterSetForbiddenError, FilterSetUpdateFailedError, FilterSetDeleteFailedError, FilterSetCreateFailedError, UserIsNotDashboardOwnerError
+from superset.dashboards.filter_sets.commands.create import CreateFilterSetCommand
+from superset.dashboards.filter_sets.commands.update import UpdateFilterSetCommand
+from superset.dashboards.filter_sets.filters import FilterSetFilter
+from superset.dashboards.filter_sets.schemas import FilterSetPostSchema, FilterSetPutSchema
+from superset.extensions import event_logger
+from superset.models.filter_set import FilterSet
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_sets.consts import OWNER_OBJECT_FIELD, DASHBOARD_FIELD, \
+ FILTER_SET_API_PERMISSIONS_NAME, NAME_FIELD, DESCRIPTION_FIELD, OWNER_TYPE_FIELD, OWNER_ID_FIELD, DASHBOARD_ID_FIELD, JSON_METADATA_FIELD
+
+logger = logging.getLogger(__name__)
+
+
+class FilterSetRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(FilterSet)
+ resource_name = "dashboard"
+ class_permission_name = FILTER_SET_API_PERMISSIONS_NAME
+ allow_browser_login = True
+ csrf_exempt = True
+ add_exclude_columns = ['id', OWNER_OBJECT_FIELD, DASHBOARD_FIELD]
+ add_model_schema = FilterSetPostSchema()
+ edit_model_schema = FilterSetPutSchema()
+ edit_exclude_columns = ['id', OWNER_OBJECT_FIELD, DASHBOARD_FIELD]
+ list_columns = ['created_on', 'changed_on', 'created_by_fk', 'changed_by_fk', NAME_FIELD,
Review comment:
we also need to add `method_permission_name = MODEL_API_RW_METHOD_PERMISSION_MAP`
##########
File path: superset/dashboards/filter_sets/api.py
##########
@@ -0,0 +1,147 @@
+# 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 Response, request, g
+from flask_appbuilder.api import expose, protect, safe, rison, permission_name, merge_response_func, \
+ get_list_schema, API_ORDER_COLUMNS_RIS_KEY, API_LABEL_COLUMNS_RIS_KEY, \
+ API_DESCRIPTION_COLUMNS_RIS_KEY, API_LIST_COLUMNS_RIS_KEY, API_LIST_TITLE_RIS_KEY, ModelRestApi
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from marshmallow import ValidationError
+from superset import is_feature_enabled
+from superset.commands.exceptions import ObjectNotFoundError
+from superset.dashboards.commands.exceptions import DashboardNotFoundError
+from superset.dashboards.filter_sets.commands.exceptions import FilterSetForbiddenError, FilterSetUpdateFailedError, FilterSetDeleteFailedError, FilterSetCreateFailedError, UserIsNotDashboardOwnerError
+from superset.dashboards.filter_sets.commands.create import CreateFilterSetCommand
+from superset.dashboards.filter_sets.commands.update import UpdateFilterSetCommand
+from superset.dashboards.filter_sets.filters import FilterSetFilter
+from superset.dashboards.filter_sets.schemas import FilterSetPostSchema, FilterSetPutSchema
+from superset.extensions import event_logger
+from superset.models.filter_set import FilterSet
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_sets.consts import OWNER_OBJECT_FIELD, DASHBOARD_FIELD, \
+ FILTER_SET_API_PERMISSIONS_NAME, NAME_FIELD, DESCRIPTION_FIELD, OWNER_TYPE_FIELD, OWNER_ID_FIELD, DASHBOARD_ID_FIELD, JSON_METADATA_FIELD
+
+logger = logging.getLogger(__name__)
+
+
+class FilterSetRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(FilterSet)
+ resource_name = "dashboard"
+ class_permission_name = FILTER_SET_API_PERMISSIONS_NAME
+ allow_browser_login = True
+ csrf_exempt = True
+ add_exclude_columns = ['id', OWNER_OBJECT_FIELD, DASHBOARD_FIELD]
+ add_model_schema = FilterSetPostSchema()
+ edit_model_schema = FilterSetPutSchema()
+ edit_exclude_columns = ['id', OWNER_OBJECT_FIELD, DASHBOARD_FIELD]
+ list_columns = ['created_on', 'changed_on', 'created_by_fk', 'changed_by_fk', NAME_FIELD,
+ DESCRIPTION_FIELD, OWNER_TYPE_FIELD, OWNER_ID_FIELD, DASHBOARD_ID_FIELD, JSON_METADATA_FIELD]
+ show_exclude_columns = [OWNER_OBJECT_FIELD, DASHBOARD_FIELD]
+ search_columns = ['id', NAME_FIELD, OWNER_ID_FIELD, DASHBOARD_ID_FIELD]
+ base_filters = [[OWNER_ID_FIELD, FilterSetFilter, '']]
+
+ def __init__(self) -> None:
+ self.datamodel.get_search_columns_list = lambda: []
+ if is_feature_enabled("THUMBNAILS"):
+ self.include_route_methods = self.include_route_methods | {"thumbnail"}
+ super().__init__()
+
+ def _init_properties(self) -> None:
+ super(BaseSupersetModelRestApi, self)._init_properties()
+
+ @expose("/<dashboard_id>/filtersets", methods=["GET"])
+ @protect()
+ @safe
+ @permission_name("get")
+ @rison(get_list_schema)
+ @merge_response_func(ModelRestApi.merge_order_columns, API_ORDER_COLUMNS_RIS_KEY)
Review comment:
we don't actually need these on superset
##########
File path: superset/dashboards/filter_sets/api.py
##########
@@ -0,0 +1,147 @@
+# 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 Response, request, g
+from flask_appbuilder.api import expose, protect, safe, rison, permission_name, merge_response_func, \
+ get_list_schema, API_ORDER_COLUMNS_RIS_KEY, API_LABEL_COLUMNS_RIS_KEY, \
+ API_DESCRIPTION_COLUMNS_RIS_KEY, API_LIST_COLUMNS_RIS_KEY, API_LIST_TITLE_RIS_KEY, ModelRestApi
+from flask_appbuilder.models.sqla.interface import SQLAInterface
+from marshmallow import ValidationError
+from superset import is_feature_enabled
+from superset.commands.exceptions import ObjectNotFoundError
+from superset.dashboards.commands.exceptions import DashboardNotFoundError
+from superset.dashboards.filter_sets.commands.exceptions import FilterSetForbiddenError, FilterSetUpdateFailedError, FilterSetDeleteFailedError, FilterSetCreateFailedError, UserIsNotDashboardOwnerError
+from superset.dashboards.filter_sets.commands.create import CreateFilterSetCommand
+from superset.dashboards.filter_sets.commands.update import UpdateFilterSetCommand
+from superset.dashboards.filter_sets.filters import FilterSetFilter
+from superset.dashboards.filter_sets.schemas import FilterSetPostSchema, FilterSetPutSchema
+from superset.extensions import event_logger
+from superset.models.filter_set import FilterSet
+from superset.views.base_api import BaseSupersetModelRestApi, statsd_metrics
+from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_sets.consts import OWNER_OBJECT_FIELD, DASHBOARD_FIELD, \
+ FILTER_SET_API_PERMISSIONS_NAME, NAME_FIELD, DESCRIPTION_FIELD, OWNER_TYPE_FIELD, OWNER_ID_FIELD, DASHBOARD_ID_FIELD, JSON_METADATA_FIELD
+
+logger = logging.getLogger(__name__)
+
+
+class FilterSetRestApi(BaseSupersetModelRestApi):
+ datamodel = SQLAInterface(FilterSet)
+ resource_name = "dashboard"
+ class_permission_name = FILTER_SET_API_PERMISSIONS_NAME
+ allow_browser_login = True
+ csrf_exempt = True
Review comment:
`csrf_exempt` should be false
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d500272) into [master](https://codecov.io/gh/apache/superset/commit/62d8ab7f9c044e2887de794484c30c2108213e0b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (62d8ab7) will **decrease** coverage by `0.04%`.
> The diff coverage is `90.23%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.63% 76.58% -0.05%
==========================================
Files 1002 1015 +13
Lines 53680 54103 +423
Branches 6852 6852
==========================================
+ Hits 41138 41437 +299
- Misses 12305 12429 +124
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.66% <90.23%> (+0.17%)` | :arrow_up: |
| postgres | `81.66% <90.23%> (+0.11%)` | :arrow_up: |
| python | `81.74% <90.23%> (-0.17%)` | :arrow_down: |
| sqlite | `81.29% <90.23%> (+0.10%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `72.76% <52.63%> (-3.93%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `75.00% <75.00%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [42 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [62d8ab7...d500272](https://codecov.io/gh/apache/superset/pull/14015?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] amitmiran137 closed pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
amitmiran137 closed pull request #14015:
URL: https://github.com/apache/superset/pull/14015
--
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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r707500847
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
Dashboard model uses JSON as well and it is very convenient to use... If I expect to Json object why not use JSON?
why do you use int and not everything as string?
Please explain to us why JSON is bad practice? how are you sure Text is ok? If you trust sqlalchemy you should use their interface.
Should Superset support as administration Database others then MySQL, Postgress, SQLite and Microsoft SQL Server 2016 ? if yes how does Superset support it until now since Dashboard model uses Json as well ?
--
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] amitmiran137 closed pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
amitmiran137 closed pull request #14015:
URL: https://github.com/apache/superset/pull/14015
--
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] stevetracvc commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
stevetracvc commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-926801049
Crap, I did this myself already (although mine is way more simple/naive). Any idea when the UI might get merged in? My names apparently conflict with these and break Superset, so if it's going to be a while I'll rename all of my stuff, and I might be able to help a bit with the UI.
Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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] villebro commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r709322817
##########
File path: tests/integration_tests/dashboards/filter_sets/conftest.py
##########
@@ -0,0 +1,322 @@
+# 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 __future__ import annotations
+
+import json
+from typing import Any, Dict, Generator, List, TYPE_CHECKING
+
+import pytest
+
+from superset import security_manager as sm
+from superset.dashboards.filter_sets.consts import (
+ DESCRIPTION_FIELD,
+ JSON_METADATA_FIELD,
+ NAME_FIELD,
+ OWNER_ID_FIELD,
+ OWNER_TYPE_FIELD,
+ USER_OWNER_TYPE,
+)
+from superset.models.dashboard import Dashboard
+from superset.models.filter_set import FilterSet
+from tests.integration_tests.dashboards.filter_sets.consts import (
+ ADMIN_USERNAME_FOR_TEST,
+ DASHBOARD_OWNER_USERNAME,
+ FILTER_SET_OWNER_USERNAME,
+ REGULAR_USER,
+)
+from tests.integration_tests.dashboards.superset_factory_util import (
+ create_dashboard,
+ create_database,
+ create_datasource_table,
+ create_slice,
+)
+from tests.integration_tests.test_app import app
+
+if TYPE_CHECKING:
+ from flask.ctx import AppContext
+ from flask.testing import FlaskClient
+ from flask_appbuilder.security.sqla.models import (
+ Role,
+ User,
+ ViewMenu,
+ PermissionView,
+ )
+ from flask_appbuilder.security.manager import BaseSecurityManager
+ from sqlalchemy.orm import Session
+ from superset.models.slice import Slice
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.core import Database
+
+
+security_manager: BaseSecurityManager = sm
+
+
+# @pytest.fixture(autouse=True, scope="session")
+# def setup_sample_data() -> Any:
+# pass
+
+
+@pytest.fixture(autouse=True)
+def expire_on_commit_true() -> Generator[None, None, None]:
+ ctx: AppContext
+ with app.app_context() as ctx:
+ ctx.app.appbuilder.get_session.configure(expire_on_commit=False)
+ yield
+ ctx.app.appbuilder.get_session.configure(expire_on_commit=True)
+
+
+@pytest.fixture(autouse=True, scope="module")
+def test_users() -> Generator[Dict[str, int], None, None]:
+ usernames = [
+ ADMIN_USERNAME_FOR_TEST,
+ DASHBOARD_OWNER_USERNAME,
+ FILTER_SET_OWNER_USERNAME,
+ REGULAR_USER,
+ ]
+ with app.app_context():
+ filter_set_role = build_filter_set_role()
+ admin_role: Role = security_manager.find_role("Admin")
+ usernames_to_ids = create_test_users(admin_role, filter_set_role, usernames)
+ yield usernames_to_ids
+ ctx: AppContext
+ delete_users(usernames_to_ids)
+
+
+def delete_users(usernames_to_ids: Dict[str, int]) -> None:
+ with app.app_context() as ctx:
+ session: Session = ctx.app.appbuilder.get_session
+ for username in usernames_to_ids.keys():
+ session.delete(security_manager.find_user(username))
+ session.commit()
+
+
+def create_test_users(
+ admin_role: Role, filter_set_role: Role, usernames: List[str]
+) -> Dict[str, int]:
+ users: List[User] = []
+ for username in usernames:
+ user = build_user(username, filter_set_role, admin_role)
+ users.append(user)
+ return {user.username: user.id for user in users}
+
+
+def build_user(username: str, filter_set_role: Role, admin_role: Role) -> User:
+ roles_to_add = (
+ [admin_role] if username == ADMIN_USERNAME_FOR_TEST else [filter_set_role]
+ )
+ user: User = security_manager.add_user(
+ username, "test", "test", username, roles_to_add, password="general"
+ )
+ if not user:
+ user = security_manager.find_user(username)
+ if user is None:
+ raise Exception("Failed to build the user {}".format(username))
+ return user
+
+
+def build_filter_set_role() -> Role:
+ filter_set_role: Role = security_manager.add_role("filter_set_role")
+ filterset_view_name: ViewMenu = security_manager.find_view_menu("FilterSets")
+ all_datasource_view_name: ViewMenu = security_manager.find_view_menu(
+ "all_datasource_access"
+ )
+ pvms: List[PermissionView] = security_manager.find_permissions_view_menu(
+ filterset_view_name
+ ) + security_manager.find_permissions_view_menu(all_datasource_view_name)
+ for pvm in pvms:
+ security_manager.add_permission_role(filter_set_role, pvm)
+ return filter_set_role
+
+
+@pytest.fixture
+def client() -> Generator[FlaskClient[Any], None, None]:
+ with app.test_client() as client:
+ yield client
+
+
+@pytest.fixture
+def dashboard() -> Generator[Dashboard, None, None]:
+ dashboard: Dashboard
+ slice_: Slice
+ datasource: SqlaTable
+ database: Database
+ session: Session
+ try:
+ with app.app_context() as ctx:
+ dashboard_owner_user = security_manager.find_user(DASHBOARD_OWNER_USERNAME)
+ database = create_database("test_database_filter_sets")
+ datasource = create_datasource_table(
+ name="test_datasource", database=database, owners=[dashboard_owner_user]
+ )
+ slice_ = create_slice(
+ datasource=datasource, name="test_slice", owners=[dashboard_owner_user]
+ )
+ dashboard = create_dashboard(
+ dashboard_title="test_dashboard",
+ published=True,
+ slices=[slice_],
+ owners=[dashboard_owner_user],
+ )
+ session = ctx.app.appbuilder.get_session
+ session.add(dashboard)
+ session.commit()
+ yield dashboard
+ except Exception as ex:
+ print(str(ex))
+ yield Dashboard.get(dashboard.slug)
Review comment:
I don't understand this logic - is this to work around flakiness? I'd prefer for this step to fail if there's an exception (also I don't usually like catching the broad `Exception` type)
##########
File path: tests/integration_tests/dashboards/filter_sets/conftest.py
##########
@@ -0,0 +1,322 @@
+# 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 __future__ import annotations
+
+import json
+from typing import Any, Dict, Generator, List, TYPE_CHECKING
+
+import pytest
+
+from superset import security_manager as sm
+from superset.dashboards.filter_sets.consts import (
+ DESCRIPTION_FIELD,
+ JSON_METADATA_FIELD,
+ NAME_FIELD,
+ OWNER_ID_FIELD,
+ OWNER_TYPE_FIELD,
+ USER_OWNER_TYPE,
+)
+from superset.models.dashboard import Dashboard
+from superset.models.filter_set import FilterSet
+from tests.integration_tests.dashboards.filter_sets.consts import (
+ ADMIN_USERNAME_FOR_TEST,
+ DASHBOARD_OWNER_USERNAME,
+ FILTER_SET_OWNER_USERNAME,
+ REGULAR_USER,
+)
+from tests.integration_tests.dashboards.superset_factory_util import (
+ create_dashboard,
+ create_database,
+ create_datasource_table,
+ create_slice,
+)
+from tests.integration_tests.test_app import app
+
+if TYPE_CHECKING:
+ from flask.ctx import AppContext
+ from flask.testing import FlaskClient
+ from flask_appbuilder.security.sqla.models import (
+ Role,
+ User,
+ ViewMenu,
+ PermissionView,
+ )
+ from flask_appbuilder.security.manager import BaseSecurityManager
+ from sqlalchemy.orm import Session
+ from superset.models.slice import Slice
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.core import Database
+
+
+security_manager: BaseSecurityManager = sm
+
+
+# @pytest.fixture(autouse=True, scope="session")
+# def setup_sample_data() -> Any:
+# pass
+
+
+@pytest.fixture(autouse=True)
+def expire_on_commit_true() -> Generator[None, None, None]:
+ ctx: AppContext
+ with app.app_context() as ctx:
+ ctx.app.appbuilder.get_session.configure(expire_on_commit=False)
+ yield
+ ctx.app.appbuilder.get_session.configure(expire_on_commit=True)
+
+
+@pytest.fixture(autouse=True, scope="module")
+def test_users() -> Generator[Dict[str, int], None, None]:
+ usernames = [
+ ADMIN_USERNAME_FOR_TEST,
+ DASHBOARD_OWNER_USERNAME,
+ FILTER_SET_OWNER_USERNAME,
+ REGULAR_USER,
+ ]
+ with app.app_context():
+ filter_set_role = build_filter_set_role()
+ admin_role: Role = security_manager.find_role("Admin")
+ usernames_to_ids = create_test_users(admin_role, filter_set_role, usernames)
+ yield usernames_to_ids
+ ctx: AppContext
+ delete_users(usernames_to_ids)
+
+
+def delete_users(usernames_to_ids: Dict[str, int]) -> None:
+ with app.app_context() as ctx:
+ session: Session = ctx.app.appbuilder.get_session
+ for username in usernames_to_ids.keys():
+ session.delete(security_manager.find_user(username))
+ session.commit()
+
+
+def create_test_users(
+ admin_role: Role, filter_set_role: Role, usernames: List[str]
+) -> Dict[str, int]:
+ users: List[User] = []
+ for username in usernames:
+ user = build_user(username, filter_set_role, admin_role)
+ users.append(user)
+ return {user.username: user.id for user in users}
+
+
+def build_user(username: str, filter_set_role: Role, admin_role: Role) -> User:
+ roles_to_add = (
+ [admin_role] if username == ADMIN_USERNAME_FOR_TEST else [filter_set_role]
+ )
+ user: User = security_manager.add_user(
+ username, "test", "test", username, roles_to_add, password="general"
+ )
+ if not user:
+ user = security_manager.find_user(username)
+ if user is None:
+ raise Exception("Failed to build the user {}".format(username))
+ return user
+
+
+def build_filter_set_role() -> Role:
+ filter_set_role: Role = security_manager.add_role("filter_set_role")
+ filterset_view_name: ViewMenu = security_manager.find_view_menu("FilterSets")
+ all_datasource_view_name: ViewMenu = security_manager.find_view_menu(
+ "all_datasource_access"
+ )
+ pvms: List[PermissionView] = security_manager.find_permissions_view_menu(
+ filterset_view_name
+ ) + security_manager.find_permissions_view_menu(all_datasource_view_name)
+ for pvm in pvms:
+ security_manager.add_permission_role(filter_set_role, pvm)
+ return filter_set_role
+
+
+@pytest.fixture
+def client() -> Generator[FlaskClient[Any], None, None]:
+ with app.test_client() as client:
+ yield client
+
+
+@pytest.fixture
+def dashboard() -> Generator[Dashboard, None, None]:
+ dashboard: Dashboard
+ slice_: Slice
+ datasource: SqlaTable
+ database: Database
+ session: Session
+ try:
+ with app.app_context() as ctx:
+ dashboard_owner_user = security_manager.find_user(DASHBOARD_OWNER_USERNAME)
+ database = create_database("test_database_filter_sets")
+ datasource = create_datasource_table(
+ name="test_datasource", database=database, owners=[dashboard_owner_user]
+ )
+ slice_ = create_slice(
+ datasource=datasource, name="test_slice", owners=[dashboard_owner_user]
+ )
+ dashboard = create_dashboard(
+ dashboard_title="test_dashboard",
+ published=True,
+ slices=[slice_],
+ owners=[dashboard_owner_user],
+ )
+ session = ctx.app.appbuilder.get_session
+ session.add(dashboard)
+ session.commit()
+ yield dashboard
+ except Exception as ex:
+ print(str(ex))
+ yield Dashboard.get(dashboard.slug)
+ finally:
+ with app.app_context() as ctx:
+ session = ctx.app.appbuilder.get_session
+ try:
+ dashboard.owners = []
+ slice_.owners = []
+ datasource.owners = []
+ session.merge(dashboard)
+ session.merge(slice_)
+ session.merge(datasource)
+ session.commit()
+ session.delete(dashboard)
+ session.delete(slice_)
+ session.delete(datasource)
+ session.delete(database)
+ session.commit()
+ except Exception as ex:
+ print(str(ex))
+
+
+@pytest.fixture
+def dashboard_id(dashboard) -> int:
+ return dashboard.id
Review comment:
Do we need this fixture?
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-882958194
⚠️ @ofekisr 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] suddjian commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-909434172
/testenv up SUPERSET_FEATURE_DASHBOARD_NATIVE_FILTERS=true SUPERSET_FEATURE_DASHBOARD_NATIVE_FILTERS_SET=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] amitmiran137 commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-911968623
Zero ui changed and will be in followup PRs which this is big already
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-881071010
⚠️ @ofekisr 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] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4b0a9ed) into [master](https://codecov.io/gh/apache/superset/commit/1cc726364a5f4ec61bb8af673cc0cd27c044ba87?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1cc7263) will **decrease** coverage by `0.07%`.
> The diff coverage is `90.20%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.93% 76.85% -0.08%
==========================================
Files 1005 1020 +15
Lines 54049 54605 +556
Branches 7339 7339
==========================================
+ Hits 41581 41966 +385
- Misses 12228 12399 +171
Partials 240 240
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.78% <90.00%> (+0.16%)` | :arrow_up: |
| postgres | `81.88% <90.20%> (+0.17%)` | :arrow_up: |
| presto | `?` | |
| python | `81.97% <90.20%> (-0.26%)` | :arrow_down: |
| sqlite | `81.50% <90.00%> (+0.21%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [45 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1cc7263...4b0a9ed](https://codecov.io/gh/apache/superset/pull/14015?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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r707500847
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
Dashboard model uses JSON as well and it is very convenient to use... If I expect to Json object why not use JSON?
why do you use int and not everything as string?
Please explain to us why JSON is bad practice? how are you sure Text is ok? If you trust sqlalchemy you should use their interface.
Should Superset support as administration Database others then MySQL, Postgress, SQLite and Microsoft SQL Server 2016 but others? if yes how does Superset support it until now since Dashboard model uses Json as well ?
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-891976430
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-829613313
⚠️ @ofekisr 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.
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] suddjian commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r699533208
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
We should avoid including the type as part of the column/variable name. Also, if possible can we find a more descriptive name than `metadata`? I'm not sure what all is contained in `metadata`. Perhaps `configuration` or `definition`?
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
We should avoid including the `json` type as part of the column/variable name. Also, if possible can we find a more descriptive name than `metadata`? I'm not sure what all is contained in `metadata`. Perhaps `configuration` or `definition`?
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-857139507
⚠️ @ofekisr 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.
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] suddjian commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-909434172
/testenv up SUPERSET_FEATURE_DASHBOARD_NATIVE_FILTERS=true SUPERSET_FEATURE_DASHBOARD_NATIVE_FILTERS_SET=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] codecov[bot] edited a comment on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (414b0c0) into [master](https://codecov.io/gh/apache/superset/commit/1cc726364a5f4ec61bb8af673cc0cd27c044ba87?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1cc7263) will **increase** coverage by `0.08%`.
> The diff coverage is `90.12%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
+ Coverage 76.93% 77.01% +0.08%
==========================================
Files 1005 1020 +15
Lines 54049 54618 +569
Branches 7339 7339
==========================================
+ Hits 41581 42066 +485
- Misses 12228 12312 +84
Partials 240 240
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.49% <90.12%> (+0.22%)` | :arrow_up: |
| mysql | `81.82% <89.91%> (+0.21%)` | :arrow_up: |
| postgres | `81.93% <90.12%> (+0.21%)` | :arrow_up: |
| presto | `?` | |
| python | `82.28% <90.12%> (+0.06%)` | :arrow_up: |
| sqlite | `81.50% <89.91%> (+0.21%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `93.87% <93.87%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [67 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1cc7263...414b0c0](https://codecov.io/gh/apache/superset/pull/14015?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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (55dbfa4) into [master](https://codecov.io/gh/apache/superset/commit/1ffd73d03b5c89d2206cadee42bb0f761233dfe2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1ffd73d) will **increase** coverage by `0.08%`.
> The diff coverage is `90.41%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
+ Coverage 76.70% 76.79% +0.08%
==========================================
Files 1002 1016 +14
Lines 53680 54324 +644
Branches 6852 6852
==========================================
+ Hits 41177 41718 +541
- Misses 12266 12369 +103
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.26% <90.41%> (+0.02%)` | :arrow_up: |
| mysql | `81.71% <90.23%> (+0.22%)` | :arrow_up: |
| postgres | `81.74% <90.41%> (+0.19%)` | :arrow_up: |
| presto | `?` | |
| python | `82.10% <90.41%> (+0.04%)` | :arrow_up: |
| sqlite | `81.35% <90.23%> (+0.15%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `77.96% <77.96%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [39 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1ffd73d...55dbfa4](https://codecov.io/gh/apache/superset/pull/14015?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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r707500847
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
Dashboard model uses JSON as well and it is very convenient to use... If I expect to Json object why not use JSON?
why do you use int and not everything as string?
Please explain to us why JSON is bad practice? how are you sure Text is ok? If you trust sqlalchemy you should use their interface.
--
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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r705441985
##########
File path: superset/models/dashboard.py
##########
@@ -397,6 +427,11 @@ def get(cls, id_or_slug: str) -> Dashboard:
qry = session.query(Dashboard).filter(id_or_slug_filter(id_or_slug))
return qry.one_or_none()
+ def am_i_owner(self) -> bool:
+ if g.user is None or g.user.is_anonymous or not g.user.is_authenticated:
+ return False
+ return g.user.id in set(map(lambda user: user.id, self.owners))
Review comment:
> IMO `is_actor_owner()` would be a more appropriate name for this method.
agree
--
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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r707500847
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
Dashboard model uses JSON as well and it is very convenient to use... If I expect to Json object why not use JSON?
why do you use int and not everything as string?
Please explain to us why JSON is bad practice? how are you sure Text is ok? If you trust sqlalchemy you should use their interface.
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-880998207
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-816373643
⚠️ @ofekisr 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.
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-925366261
⚠️ @ofekisr 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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r705441985
##########
File path: superset/models/dashboard.py
##########
@@ -397,6 +427,11 @@ def get(cls, id_or_slug: str) -> Dashboard:
qry = session.query(Dashboard).filter(id_or_slug_filter(id_or_slug))
return qry.one_or_none()
+ def am_i_owner(self) -> bool:
+ if g.user is None or g.user.is_anonymous or not g.user.is_authenticated:
+ return False
+ return g.user.id in set(map(lambda user: user.id, self.owners))
Review comment:
> IMO `is_actor_owner()` would be a more appropriate name for this method.
agree
@amitmiran137
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (55dbfa4) into [master](https://codecov.io/gh/apache/superset/commit/1ffd73d03b5c89d2206cadee42bb0f761233dfe2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1ffd73d) will **decrease** coverage by `0.05%`.
> The diff coverage is `90.41%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.70% 76.65% -0.06%
==========================================
Files 1002 1016 +14
Lines 53680 54324 +644
Branches 6852 6852
==========================================
+ Hits 41177 41641 +464
- Misses 12266 12446 +180
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.71% <90.23%> (+0.22%)` | :arrow_up: |
| postgres | `81.74% <90.41%> (+0.19%)` | :arrow_up: |
| presto | `?` | |
| python | `81.82% <90.41%> (-0.23%)` | :arrow_down: |
| sqlite | `81.35% <90.23%> (+0.15%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `77.96% <77.96%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [44 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1ffd73d...55dbfa4](https://codecov.io/gh/apache/superset/pull/14015?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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4b0a9ed) into [master](https://codecov.io/gh/apache/superset/commit/1cc726364a5f4ec61bb8af673cc0cd27c044ba87?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1cc7263) will **decrease** coverage by `0.07%`.
> The diff coverage is `90.20%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.93% 76.85% -0.08%
==========================================
Files 1005 1020 +15
Lines 54049 54605 +556
Branches 7339 7339
==========================================
+ Hits 41581 41966 +385
- Misses 12228 12399 +171
Partials 240 240
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.78% <90.00%> (+0.16%)` | :arrow_up: |
| postgres | `81.89% <90.20%> (+0.17%)` | :arrow_up: |
| presto | `?` | |
| python | `81.97% <90.20%> (-0.26%)` | :arrow_down: |
| sqlite | `81.50% <90.00%> (+0.21%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [45 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1cc7263...4b0a9ed](https://codecov.io/gh/apache/superset/pull/14015?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] ofekisr commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
ofekisr commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r711709875
##########
File path: superset/dashboards/filter_sets/commands/base.py
##########
@@ -0,0 +1,95 @@
+# 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 cast, Optional
+
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+
+from superset.commands.base import BaseCommand
+from superset.common.not_authrized_object import NotAuthorizedException
+from superset.common.request_contexed_based import is_user_admin
+from superset.dashboards.commands.exceptions import DashboardNotFoundError
+from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_sets.commands.exceptions import (
+ FilterSetForbiddenError,
+ FilterSetNotFoundError,
+)
+from superset.dashboards.filter_sets.consts import USER_OWNER_TYPE
+from superset.models.dashboard import Dashboard
+from superset.models.filter_set import FilterSet
+
+logger = logging.getLogger(__name__)
+
+
+class BaseFilterSetCommand(BaseCommand):
+ # pylint: disable=C0103
+ _dashboard: Dashboard
+ _filter_set_id: Optional[int]
+ _filter_set: Optional[FilterSet]
+
+ def __init__(self, user: User, dashboard_id: int):
+ self._actor = user
+ self._is_actor_admin = is_user_admin()
+ self._dashboard_id = dashboard_id
+
+ def run(self) -> Model:
+ pass
+
+ def validate(self) -> None:
+ self._validate_filterset_dashboard_exists()
+
+ def _validate_filterset_dashboard_exists(self) -> None:
+ self._dashboard = DashboardDAO.get_by_id_or_slug(str(self._dashboard_id))
+ if not self._dashboard:
+ raise DashboardNotFoundError()
Review comment:
As written In the Clean Code book "Always try to explain yourself in code."
and you should not use comments.
but I removed the BaseCommand so abstraction was removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (51e2084) into [master](https://codecov.io/gh/apache/superset/commit/997320ac1a969bbdf97c4a5cb9cef04fff590f3a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (997320a) will **increase** coverage by `0.11%`.
> The diff coverage is `90.14%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
+ Coverage 76.89% 77.00% +0.11%
==========================================
Files 1005 1018 +13
Lines 54169 54660 +491
Branches 7456 7456
==========================================
+ Hits 41653 42093 +440
- Misses 12272 12323 +51
Partials 244 244
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `81.45% <90.14%> (+0.10%)` | :arrow_up: |
| mysql | `81.88% <89.93%> (+0.16%)` | :arrow_up: |
| postgres | `81.94% <90.14%> (+0.13%)` | :arrow_up: |
| presto | `81.82% <90.14%> (+0.13%)` | :arrow_up: |
| python | `82.45% <90.14%> (+0.12%)` | :arrow_up: |
| sqlite | `81.56% <89.93%> (+0.13%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `78.46% <78.46%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `93.87% <93.87%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [23 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [997320a...51e2084](https://codecov.io/gh/apache/superset/pull/14015?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] dpgaspar commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
dpgaspar commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r638745897
##########
File path: superset/app.py
##########
@@ -549,6 +549,10 @@ def init_views(self) -> None:
"Data", cond=lambda: bool(self.config["DRUID_IS_ACTIVE"])
)
+ from superset.dashboards.filter_sets.api import FilterSetRestApi
Review comment:
nit: import is done upstream
##########
File path: superset/common/request_contexed_based.py
##########
@@ -0,0 +1,38 @@
+# 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 __future__ import annotations
+
+from typing import List, TYPE_CHECKING
+
+from flask import g
+
+from superset import conf, security_manager
+
+if TYPE_CHECKING:
+ from flask_appbuilder.security.sqla.models import Role
+
+
+def get_user_roles() -> List[Role]:
+ if g.user.is_anonymous:
+ public_role = conf.get("AUTH_ROLE_PUBLIC")
+ return [security_manager.find_role(public_role)] if public_role else []
+ return g.user.roles
+
+
+def is_user_admin() -> bool:
+ user_roles = [role.name.lower() for role in list(get_user_roles())]
Review comment:
theres also this config key `AUTH_ROLE_ADMIN`
##########
File path: superset/dashboards/filter_sets/commands/exceptions.py
##########
@@ -0,0 +1,94 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Optional
+
+from flask_babel import lazy_gettext as _
+
+from superset.commands.exceptions import (
+ CreateFailedError,
+ DeleteFailedError,
+ ForbiddenError,
+ ObjectNotFoundError,
+ UpdateFailedError,
+)
+
+
+class FilterSetNotFoundError(ObjectNotFoundError):
+ def __init__(
+ self, filterset_id: Optional[str] = None, exception: Optional[Exception] = None
+ ) -> None:
+ super().__init__("FilterSet", filterset_id, exception)
+
+
+class FilterSetCreateFailedError(CreateFailedError):
+ base_massage = 'CreateFilterSetCommand of dashboard "%s" failed: '
+
+ def __init__(
+ self, dashboard_id: str, reason: str = "", exception: Optional[Exception] = None
+ ) -> None:
+ super().__init__((self.base_massage % dashboard_id) + reason, exception)
+
+
+class FilterSetUpdateFailedError(UpdateFailedError):
+ base_massage = 'UpdateFilterSetCommand of filter_set "%s" failed: '
+
+ def __init__(
+ self, filterset_id: str, reason: str = "", exception: Optional[Exception] = None
+ ) -> None:
+ super().__init__((self.base_massage % filterset_id) + reason, exception)
+
+
+class FilterSetDeleteFailedError(DeleteFailedError):
+ base_massage = 'DeleteFilterSetCommand of filter_set "%s" failed: '
Review comment:
copy/paste typo
##########
File path: superset/dashboards/filter_sets/commands/exceptions.py
##########
@@ -0,0 +1,94 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Optional
+
+from flask_babel import lazy_gettext as _
+
+from superset.commands.exceptions import (
+ CreateFailedError,
+ DeleteFailedError,
+ ForbiddenError,
+ ObjectNotFoundError,
+ UpdateFailedError,
+)
+
+
+class FilterSetNotFoundError(ObjectNotFoundError):
+ def __init__(
+ self, filterset_id: Optional[str] = None, exception: Optional[Exception] = None
+ ) -> None:
+ super().__init__("FilterSet", filterset_id, exception)
+
+
+class FilterSetCreateFailedError(CreateFailedError):
+ base_massage = 'CreateFilterSetCommand of dashboard "%s" failed: '
+
+ def __init__(
+ self, dashboard_id: str, reason: str = "", exception: Optional[Exception] = None
+ ) -> None:
+ super().__init__((self.base_massage % dashboard_id) + reason, exception)
+
+
+class FilterSetUpdateFailedError(UpdateFailedError):
+ base_massage = 'UpdateFilterSetCommand of filter_set "%s" failed: '
Review comment:
copy/paste typo
##########
File path: superset/dashboards/filter_sets/commands/exceptions.py
##########
@@ -0,0 +1,94 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from typing import Optional
+
+from flask_babel import lazy_gettext as _
+
+from superset.commands.exceptions import (
+ CreateFailedError,
+ DeleteFailedError,
+ ForbiddenError,
+ ObjectNotFoundError,
+ UpdateFailedError,
+)
+
+
+class FilterSetNotFoundError(ObjectNotFoundError):
+ def __init__(
+ self, filterset_id: Optional[str] = None, exception: Optional[Exception] = None
+ ) -> None:
+ super().__init__("FilterSet", filterset_id, exception)
+
+
+class FilterSetCreateFailedError(CreateFailedError):
+ base_massage = 'CreateFilterSetCommand of dashboard "%s" failed: '
Review comment:
typo: `base_message`
##########
File path: superset/common/request_contexed_based.py
##########
@@ -0,0 +1,38 @@
+# 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 __future__ import annotations
+
+from typing import List, TYPE_CHECKING
+
+from flask import g
+
+from superset import conf, security_manager
+
+if TYPE_CHECKING:
+ from flask_appbuilder.security.sqla.models import Role
+
+
+def get_user_roles() -> List[Role]:
+ if g.user.is_anonymous:
+ public_role = conf.get("AUTH_ROLE_PUBLIC")
+ return [security_manager.find_role(public_role)] if public_role else []
Review comment:
can you use `security_manager.get_public_role` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] villebro commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r703438821
##########
File path: superset/commands/exceptions.py
##########
@@ -31,6 +31,26 @@ def __repr__(self) -> str:
return repr(self)
+class ObjectNotFoundError(CommandException):
+ status = 404
+ message_format = "{} {}not found."
+
+ def __init__(
+ self,
+ object_type: str,
+ object_id: Optional[str] = None,
+ exception: Optional[Exception] = None,
+ ) -> None:
+ super().__init__(
+ _(
+ self.message_format.format(
+ object_type, '"%s" ' % object_id if object_id else ""
+ )
+ ),
Review comment:
I doubt this will work. `_()` needs to be called with a static message, with variable pieces handled as kwargs:
```python
_("This is my message: %(message)s", message="abc")
```
##########
File path: superset/commands/export.py
##########
@@ -34,7 +34,7 @@
class ExportModelsCommand(BaseCommand):
dao = BaseDAO
- not_found = CommandException
+ not_found: Type[CommandException] = CommandException
Review comment:
Not sure this is needed
##########
File path: superset/models/dashboard.py
##########
@@ -397,6 +427,11 @@ def get(cls, id_or_slug: str) -> Dashboard:
qry = session.query(Dashboard).filter(id_or_slug_filter(id_or_slug))
return qry.one_or_none()
+ def am_i_owner(self) -> bool:
+ if g.user is None or g.user.is_anonymous or not g.user.is_authenticated:
+ return False
+ return g.user.id in set(map(lambda user: user.id, self.owners))
Review comment:
IMO `is_actor_owner()` would be a more appropriate name for this method.
##########
File path: superset/common/request_contexed_based.py
##########
@@ -0,0 +1,39 @@
+# 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 __future__ import annotations
+
+from typing import List, TYPE_CHECKING
+
+from flask import g
+
+from superset import conf, security_manager
+
+if TYPE_CHECKING:
+ from flask_appbuilder.security.sqla.models import Role
+
+
+def get_user_roles() -> List[Role]:
+ if g.user.is_anonymous:
+ public_role = conf.get("AUTH_ROLE_PUBLIC")
+ return [security_manager.get_public_role()] if public_role else []
+ return g.user.roles
+
+
+def is_user_admin() -> bool:
+ user_roles = [role.name.lower() for role in list(get_user_roles())]
Review comment:
nit: not sure we need to encapsulate this in `list()`
```suggestion
def is_user_admin() -> bool:
user_roles = [role.name.lower() for role in get_user_roles()]
```
##########
File path: superset/dashboards/filter_sets/commands/base.py
##########
@@ -0,0 +1,95 @@
+# 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 cast, Optional
+
+from flask_appbuilder.models.sqla import Model
+from flask_appbuilder.security.sqla.models import User
+
+from superset.commands.base import BaseCommand
+from superset.common.not_authrized_object import NotAuthorizedException
+from superset.common.request_contexed_based import is_user_admin
+from superset.dashboards.commands.exceptions import DashboardNotFoundError
+from superset.dashboards.dao import DashboardDAO
+from superset.dashboards.filter_sets.commands.exceptions import (
+ FilterSetForbiddenError,
+ FilterSetNotFoundError,
+)
+from superset.dashboards.filter_sets.consts import USER_OWNER_TYPE
+from superset.models.dashboard import Dashboard
+from superset.models.filter_set import FilterSet
+
+logger = logging.getLogger(__name__)
+
+
+class BaseFilterSetCommand(BaseCommand):
+ # pylint: disable=C0103
+ _dashboard: Dashboard
+ _filter_set_id: Optional[int]
+ _filter_set: Optional[FilterSet]
+
+ def __init__(self, user: User, dashboard_id: int):
+ self._actor = user
+ self._is_actor_admin = is_user_admin()
+ self._dashboard_id = dashboard_id
+
+ def run(self) -> Model:
+ pass
+
+ def validate(self) -> None:
+ self._validate_filterset_dashboard_exists()
+
+ def _validate_filterset_dashboard_exists(self) -> None:
+ self._dashboard = DashboardDAO.get_by_id_or_slug(str(self._dashboard_id))
+ if not self._dashboard:
+ raise DashboardNotFoundError()
Review comment:
I don't understand why we need a private method `_validate_filterset_dashboard_exists()` that is only called once by a one-liner wrapper `validate()`?
##########
File path: superset/models/filter_set.py
##########
@@ -0,0 +1,99 @@
+# 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 __future__ import annotations
+
+import logging
+from typing import Any, Dict
+
+from flask_appbuilder import Model
+from sqlalchemy import Column, ForeignKey, Integer, JSON, MetaData, String, Text
+from sqlalchemy.orm import relationship
+from sqlalchemy_utils import generic_relationship
+
+from superset import app, db
+from superset.models.helpers import AuditMixinNullable
+
+metadata = Model.metadata # pylint: disable=no-member
+config = app.config
+logger = logging.getLogger(__name__)
+
+
+class FilterSet(Model, AuditMixinNullable):
+ __tablename__ = "filter_sets"
+ id = Column(Integer, primary_key=True)
+ name = Column(String(500), nullable=False, unique=True)
+ description = Column(Text, nullable=True)
+ json_metadata = Column(JSON, nullable=False)
Review comment:
same here (change to `Text`)
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
I suggest we not use `sa.JSON()` here but rather go with `sa.Text()` as we have until now, as with `sa.JSON()` we're not sure how it behaves on various metadata engines. So let's just keep it `Text()` and assume we need to parse/format when deserializing/serializing.
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-907763623
# [Codecov](https://codecov.io/gh/apache/superset/pull/14015?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 [#14015](https://codecov.io/gh/apache/superset/pull/14015?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (07792fa) into [master](https://codecov.io/gh/apache/superset/commit/1ffd73d03b5c89d2206cadee42bb0f761233dfe2?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1ffd73d) will **decrease** coverage by `0.11%`.
> The diff coverage is `90.41%`.
[](https://codecov.io/gh/apache/superset/pull/14015?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 #14015 +/- ##
==========================================
- Coverage 76.70% 76.58% -0.12%
==========================================
Files 1002 1015 +13
Lines 53680 54155 +475
Branches 6852 6852
==========================================
+ Hits 41177 41477 +300
- Misses 12266 12441 +175
Partials 237 237
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `?` | |
| mysql | `81.62% <90.23%> (+0.13%)` | :arrow_up: |
| postgres | `81.65% <90.41%> (+0.10%)` | :arrow_up: |
| presto | `?` | |
| python | `81.73% <90.41%> (-0.32%)` | :arrow_down: |
| sqlite | `81.29% <90.23%> (+0.10%)` | :arrow_up: |
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/14015?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/models/dashboard.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2Rhc2hib2FyZC5weQ==) | `75.09% <57.14%> (-1.60%)` | :arrow_down: |
| [superset/common/not\_authrized\_object.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL25vdF9hdXRocml6ZWRfb2JqZWN0LnB5) | `66.66% <66.66%> (ø)` | |
| [superset/models/filter\_set.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvbW9kZWxzL2ZpbHRlcl9zZXQucHk=) | `77.96% <77.96%> (ø)` | |
| [superset/common/request\_contexed\_based.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvY29tbW9uL3JlcXVlc3RfY29udGV4ZWRfYmFzZWQucHk=) | `80.00% <80.00%> (ø)` | |
| [...rset/dashboards/filter\_sets/commands/exceptions.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9leGNlcHRpb25zLnB5) | `90.00% <90.00%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/delete.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9kZWxldGUucHk=) | `92.30% <92.30%> (ø)` | |
| [superset/dashboards/filter\_sets/schemas.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9zY2hlbWFzLnB5) | `92.85% <92.85%> (ø)` | |
| [superset/dashboards/filter\_sets/filters.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9maWx0ZXJzLnB5) | `94.11% <94.11%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/base.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy9iYXNlLnB5) | `94.23% <94.23%> (ø)` | |
| [superset/dashboards/filter\_sets/commands/update.py](https://codecov.io/gh/apache/superset/pull/14015/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-c3VwZXJzZXQvZGFzaGJvYXJkcy9maWx0ZXJfc2V0cy9jb21tYW5kcy91cGRhdGUucHk=) | `96.15% <96.15%> (ø)` | |
| ... and [33 more](https://codecov.io/gh/apache/superset/pull/14015/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/14015?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/14015?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 [1ffd73d...07792fa](https://codecov.io/gh/apache/superset/pull/14015?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] amitmiran137 commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r699559909
##########
File path: superset/migrations/versions/3ebe0993c770_filterset_table.py
##########
@@ -0,0 +1,56 @@
+# 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.
+"""add filter set model
+
+Revision ID: 3ebe0993c770
+Revises: 07071313dd52
+Create Date: 2021-03-29 11:15:48.831225
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "3ebe0993c770"
+down_revision = "07071313dd52"
+
+import sqlalchemy as sa
+from alembic import op
+
+
+def upgrade():
+ op.create_table(
+ "filter_sets",
+ sa.Column("created_on", sa.DateTime(), nullable=True),
+ sa.Column("changed_on", sa.DateTime(), nullable=True),
+ sa.Column("id", sa.Integer(), nullable=False),
+ sa.Column("name", sa.VARCHAR(500), nullable=False),
+ sa.Column("description", sa.Text(), nullable=True),
+ sa.Column("json_metadata", sa.JSON(), nullable=False),
Review comment:
Ok so a bit context here .
The purpose of this field is for storing the list of native-filters config as they are stored today under the Dashboard metadata.
About the field name , I'm ok with anything we decide .
Let's see what others like .
@villebro any thoughts?
--
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-900697612
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-831230434
⚠️ @ofekisr 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.
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 #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-896411740
⚠️ @ofekisr 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] github-actions[bot] commented on pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14015:
URL: https://github.com/apache/superset/pull/14015#issuecomment-820731684
⚠️ @ofekisr 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.
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] amitmiran137 commented on a change in pull request #14015: feat(filter-set): Add filterset resource
Posted by GitBox <gi...@apache.org>.
amitmiran137 commented on a change in pull request #14015:
URL: https://github.com/apache/superset/pull/14015#discussion_r613112547
##########
File path: superset/app.py
##########
@@ -523,6 +523,9 @@ def init_views(self) -> None:
icon="fa-cog",
)
appbuilder.add_separator("Data")
+ from superset.dashboards.filter_sets.api import FilterSetRestApi
+
+ appbuilder.add_api(FilterSetRestApi)
Review comment:
```suggestion
if feature_flag_manager.is_feature_enabled("DASHBOARD_NATIVE_FILTERS_SET"):
from superset.dashboards.filter_sets.api import FilterSetRestApi
appbuilder.add_api(FilterSetRestApi)
```
Lets add it under the feature flag
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org