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/10/11 14:54:37 UTC
[GitHub] [superset] ofekisr opened a new pull request #17057: feat(dashboard-rbac): switch to object_roles generic table
ofekisr opened a new pull request #17057:
URL: https://github.com/apache/superset/pull/17057
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] github-actions[bot] commented on pull request #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1020441443
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-954885419
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-968336998
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1050166645
⚠️ @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 a change in pull request #17057: feat(dashboard-rbac): switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #17057:
URL: https://github.com/apache/superset/pull/17057#discussion_r726586790
##########
File path: superset/migrations/versions/d96513202d2a_initial_migration.py
##########
@@ -0,0 +1,159 @@
+# 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.
+"""Initial migration.
+
+Revision ID: d96513202d2a
+Revises: 60dc453f4e2e
+Create Date: 2021-10-10 18:31:58.279446
+
+"""
+import logging
+from typing import Callable, List
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy import inspect
+
+# revision identifiers, used by Alembic.
+revision = "d96513202d2a"
+down_revision = "60dc453f4e2e"
+
+logger = logging.getLogger("alembic.runtime.migration")
+
+OLD_TABLE_NAME = "dashboard_roles"
+NEW_TABLE_NAME = "object_roles"
+DASHBOARD_ROLES_CONSTRAINT_NAME = "dashboard_roles_ibfk_1"
+UNIQUE_CONSTRAINT_NAME = "object_roles_unique_rows"
+OBJECT_TYPE_COLUMN_NAME = "object_type"
+
+
+def upgrade() -> None:
+ _run_migrations(create_upgrade_steps(), create_downgrade_steps())
+
+
+def downgrade() -> None:
+ _run_migrations(create_downgrade_steps(), create_upgrade_steps())
+
+
+def _get_dashboard_constraint_name() -> str:
+ from superset import db
+
+ logger.info("_get_dashboard_constraint_name")
+ inspector = inspect(db.engine)
+ logger.info(str(inspector))
+
+ for constraint in inspector.get_foreign_keys(OLD_TABLE_NAME):
+ if constraint["constrained_columns"] == ["dashboard_id"]:
+ return constraint["name"]
+
+
+def _run_migrations(
+ migration_steps: List[Callable], rollback_steps: List[Callable]
+) -> None:
+ run_on_error = []
+ try:
+ for step in migration_steps:
+ step()
+ run_on_error.append(rollback_steps.pop())
+ except Exception as original_exception:
+ _on_error(original_exception, run_on_error)
+
+
+def _on_error(original_exception, run_on_error):
+ logger.error(
+ "Failed to run migrations, trying to run rollback steps: %s",
+ original_exception,
+ exc_info=original_exception,
+ )
+ try:
+ for step in run_on_error:
+ step()
+ except Exception as inner_exception:
+ logger.error("Failed to run rollback steps: %s", inner_exception)
+ finally:
+ raise original_exception
+
+
+def create_upgrade_steps() -> List[Callable]:
+ def new_column_operation() -> None:
+ logger.info("add column")
+ op.add_column(
+ OLD_TABLE_NAME,
+ sa.Column(OBJECT_TYPE_COLUMN_NAME, sa.String(length=256), nullable=True),
+ )
+ logger.info("set values")
+ op.execute(
+ "UPDATE {table_name} SET object_type = {object_type}".format(
+ table_name=OLD_TABLE_NAME, object_type="'Dashboard'"
+ )
+ )
+ logger.info("set not null")
Review comment:
Also the logs in this function
--
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 pull request #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
ofekisr commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-950936188
description was edited with current status
--
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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1082640147
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1011282882
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1083850261
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1020441443
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1059333809
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-977799082
⚠️ @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 #17057: feat(dashboard-rbac): switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-941915368
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-992908673
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1032331645
⚠️ @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 a change in pull request #17057: feat(dashboard-rbac): switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #17057:
URL: https://github.com/apache/superset/pull/17057#discussion_r726588180
##########
File path: superset/migrations/versions/d96513202d2a_initial_migration.py
##########
@@ -0,0 +1,137 @@
+# 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.
+"""Initial migration.
+
+Revision ID: d96513202d2a
+Revises: 60dc453f4e2e
+Create Date: 2021-10-10 18:31:58.279446
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "d96513202d2a"
+down_revision = "60dc453f4e2e"
+import logging
+from typing import Callable, List
+
+import sqlalchemy as sa
+from alembic import op
+
+logger = logging.getLogger(__name__)
+
+OLD_TABLE_NAME = "dashboard_roles"
+NEW_TABLE_NAME = "object_roles"
+DASHBOARD_ROLES_CONSTRAINT_NAME = "dashboard_roles_ibfk_1"
+UNIQUE_CONSTRAINT_NAME = "object_roles_unique_rows"
+OBJECT_TYPE_COLUMN_NAME = "object_type"
+
+
+def upgrade() -> None:
+ _run_migrations(create_upgrade_steps(), create_downgrade_steps())
Review comment:
Agree with this, but also I feel like if this much complexity is required for a migration, some of it should be pulled out into helper functions with tests, which should also increase clarity.
--
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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-995319562
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-998898902
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-999838255
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-985737616
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1069733699
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-992970823
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1058445620
⚠️ @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 removed a comment on pull request #17057: feat(dashboard-rbac): switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
suddjian removed a comment on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-940472076
It would be nice to have a summary explaining the motivations and/or plans around this change.
--
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 #17057: feat(dashboard-rbac): switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
suddjian commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-940472076
It would be nice to have a summary explaining the motivations and/or plans around this change.
--
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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-950795216
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1071484021
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-979862891
⚠️ @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 #17057: feat(dashboard-rbac): switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-941915368
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1005324430
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1082170053
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1071484021
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1057239183
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1058720411
⚠️ @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] craig-rueda commented on a change in pull request #17057: feat(dashboard-rbac): switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
craig-rueda commented on a change in pull request #17057:
URL: https://github.com/apache/superset/pull/17057#discussion_r726484917
##########
File path: superset/migrations/versions/d96513202d2a_initial_migration.py
##########
@@ -0,0 +1,137 @@
+# 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.
+"""Initial migration.
+
+Revision ID: d96513202d2a
+Revises: 60dc453f4e2e
+Create Date: 2021-10-10 18:31:58.279446
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = "d96513202d2a"
+down_revision = "60dc453f4e2e"
+import logging
+from typing import Callable, List
+
+import sqlalchemy as sa
+from alembic import op
+
+logger = logging.getLogger(__name__)
+
+OLD_TABLE_NAME = "dashboard_roles"
+NEW_TABLE_NAME = "object_roles"
+DASHBOARD_ROLES_CONSTRAINT_NAME = "dashboard_roles_ibfk_1"
+UNIQUE_CONSTRAINT_NAME = "object_roles_unique_rows"
+OBJECT_TYPE_COLUMN_NAME = "object_type"
+
+
+def upgrade() -> None:
+ _run_migrations(create_upgrade_steps(), create_downgrade_steps())
Review comment:
Can we just have the up/dn steps in the body of these two methods? It's a little hard to follow what's going on 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] suddjian commented on a change in pull request #17057: feat(dashboard-rbac): switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
suddjian commented on a change in pull request #17057:
URL: https://github.com/apache/superset/pull/17057#discussion_r726586412
##########
File path: superset/migrations/versions/d96513202d2a_initial_migration.py
##########
@@ -0,0 +1,159 @@
+# 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.
+"""Initial migration.
+
+Revision ID: d96513202d2a
+Revises: 60dc453f4e2e
+Create Date: 2021-10-10 18:31:58.279446
+
+"""
+import logging
+from typing import Callable, List
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy import inspect
+
+# revision identifiers, used by Alembic.
+revision = "d96513202d2a"
+down_revision = "60dc453f4e2e"
+
+logger = logging.getLogger("alembic.runtime.migration")
+
+OLD_TABLE_NAME = "dashboard_roles"
+NEW_TABLE_NAME = "object_roles"
+DASHBOARD_ROLES_CONSTRAINT_NAME = "dashboard_roles_ibfk_1"
+UNIQUE_CONSTRAINT_NAME = "object_roles_unique_rows"
+OBJECT_TYPE_COLUMN_NAME = "object_type"
+
+
+def upgrade() -> None:
+ _run_migrations(create_upgrade_steps(), create_downgrade_steps())
+
+
+def downgrade() -> None:
+ _run_migrations(create_downgrade_steps(), create_upgrade_steps())
+
+
+def _get_dashboard_constraint_name() -> str:
+ from superset import db
+
+ logger.info("_get_dashboard_constraint_name")
+ inspector = inspect(db.engine)
+ logger.info(str(inspector))
Review comment:
Let's remove these logs
--
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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-994560649
⚠️ @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 #17057: feat(dashboard-rbac): POC switch to object_roles generic table
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17057:
URL: https://github.com/apache/superset/pull/17057#issuecomment-1051108332
⚠️ @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