feat: Add Query Peer Review System

plugins/query_review_handler_plugin/__init__.py

@@ -0,0 +1 @@
+PLUGIN_QUERY_REVIEW_HANDLER = None

querybook/config/querybook_public_config.yaml

@@ -44,3 +44,32 @@ table_sampling:
 
 github_integration:
     enabled: false
+
+peer_review:
+    enabled: true
+    request_texts:  # When users request reviews
+        description: |
+            This peer review system helps ensure query quality and data safety.
+
+            Note: Queries that fail after approval will require a new review request.
+
+            Important Guidelines:
+            • Choose reviewers familiar with the affected data
+            • Include relevant references in query title or justification
+            • Document query purpose and impact
+        guide_link: 'https://www.querybook.org/'
+        tip: |
+            Before Submitting:
+            • Run query to verify syntax
+            • Validate table names and fields
+            • Check query performance
+    reviewer_texts:  # When reviewers take actions
+        approve_message: |
+            As a reviewer, you are responsible for validating:
+
+            • Query purpose and necessity
+            • Potential data impact
+            • Compliance with data policies
+
+            Request clarification if more context is needed.
+            Approval cannot be reversed.

querybook/migrations/versions/d3582302cf48_add_query_review_table.py

@@ -0,0 +1,132 @@
+"""Add Query Review table
+
+Revision ID: d3582302cf48
+Revises: aa328ae9dced
+Create Date: 2024-12-17 22:41:28.700769
+
+"""
+
+from alembic import op
+import sqlalchemy as sa
+
+# revision identifiers, used by Alembic.
+revision = "d3582302cf48"
+down_revision = "aa328ae9dced"
+branch_labels = None
+depends_on = None
+
+# Define the old and new QueryExecutionStatus enum types for non-PostgreSQL databases
+old_status_enum = sa.Enum(
+    "INITIALIZED",
+    "DELIVERED",
+    "RUNNING",
+    "DONE",
+    "ERROR",
+    "CANCEL",
+    name="queryexecutionstatus",
+)
+
+new_status_enum = sa.Enum(
+    "INITIALIZED",
+    "DELIVERED",
+    "RUNNING",
+    "DONE",
+    "ERROR",
+    "CANCEL",
+    "PENDING_REVIEW",
+    "REJECTED",
+    name="queryexecutionstatus",
+)
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+
+    # ### Enum Modifications ###
+    bind = op.get_bind()
+    dialect = bind.dialect.name
+
+    if dialect == "postgresql":
+        # PostgreSQL: Add new enum values to existing 'queryexecutionstatus' enum
+        op.execute("ALTER TYPE queryexecutionstatus ADD VALUE 'PENDING_REVIEW'")
+        op.execute("ALTER TYPE queryexecutionstatus ADD VALUE 'REJECTED'")
+    else:
+        # Other Databases (e.g., MySQL, SQLite): Alter 'query_execution.status' column to use the new enum
+        op.alter_column(
+            "query_execution",
+            "status",
+            existing_type=old_status_enum,
+            type_=new_status_enum,
+            existing_nullable=True,
+            postgresql_using=None,
+        )
+
+    # ### Table Creation ###
+    op.create_table(
+        "query_review",
+        sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
+        sa.Column("query_execution_id", sa.Integer(), nullable=False),
+        sa.Column("reviewed_by", sa.Integer(), nullable=True),
+        sa.Column("request_reason", sa.String(length=5000), nullable=False),
+        sa.Column("rejection_reason", sa.String(length=5000), nullable=True),
+        sa.Column("created_at", sa.DateTime(), nullable=False),
+        sa.Column("updated_at", sa.DateTime(), nullable=False),
+        sa.ForeignKeyConstraint(
+            ["query_execution_id"], ["query_execution.id"], ondelete="CASCADE"
+        ),
+        sa.ForeignKeyConstraint(["reviewed_by"], ["user.id"], ondelete="SET NULL"),
+        sa.PrimaryKeyConstraint("id"),
+        sa.UniqueConstraint("query_execution_id"),
+        mysql_charset="utf8mb4",
+        mysql_engine="InnoDB",
+    )
+    op.create_table(
+        "query_execution_reviewer",
+        sa.Column("id", sa.Integer(), autoincrement=True, nullable=False),
+        sa.Column("query_review_id", sa.Integer(), nullable=False),
+        sa.Column("uid", sa.Integer(), nullable=False),
+        sa.Column("created_at", sa.DateTime(), nullable=False),
+        sa.ForeignKeyConstraint(
+            ["query_review_id"], ["query_review.id"], ondelete="CASCADE"
+        ),
+        sa.ForeignKeyConstraint(["uid"], ["user.id"], ondelete="CASCADE"),
+        sa.PrimaryKeyConstraint("id"),
+        sa.UniqueConstraint(
+            "query_review_id", "uid", name="unique_query_execution_reviewer"
+        ),
+        mysql_charset="utf8mb4",
+        mysql_engine="InnoDB",
+    )
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+
+    # ### Table Dropping ###
+    op.drop_table("query_execution_reviewer")
+    op.drop_table("query_review")
+
+    # ### Enum Modifications ###
+    bind = op.get_bind()
+    dialect = bind.dialect.name
+
+    if dialect == "postgresql":
+        # PostgreSQL: does not support removing enum values directly
+        op.execute("ALTER TYPE queryexecutionstatus RENAME TO queryexecutionstatus_old")
+        old_status_enum.create(bind, checkfirst=True)
+        op.execute(
+            "ALTER TABLE query_execution ALTER COLUMN status TYPE queryexecutionstatus USING status::text::queryexecutionstatus"
+        )
+        op.execute("DROP TYPE queryexecutionstatus_old")
+    else:
+        # Other Databases (e.g., MySQL, SQLite): Revert 'query_execution.status' column to the old enum
+        op.alter_column(
+            "query_execution",
+            "status",
+            existing_type=new_status_enum,
+            type_=old_status_enum,
+            existing_nullable=True,
+            postgresql_using=None,
+        )
+    # ### end Alembic commands ###

querybook/notification_templates/query_review_approved.md

@@ -0,0 +1,3 @@
+Your query review has been approved by {{ reviewer_name }} on {{ approved_at }}.
+
+**Here is the url to view the running query: <{{ query_execution_url }}>**

querybook/notification_templates/query_review_rejected.md

@@ -0,0 +1,8 @@
+Your query review (Query ID: {{ query_execution_id }}) has been rejected by {{ reviewer_name }} on {{ rejected_at }}.
+
+**Rejection reason provided:**
+{{ rejection_reason }}
+
+**Here is the url to view the query: <{{ query_execution_url }}>**
+
+Please address the feedback and submit a new review request once changes are made.

querybook/notification_templates/query_review_request.md

@@ -0,0 +1,9 @@
+New Query Review Required: Request from {{ requested_by }}
+
+Query review requested on {{ requested_at }}
+
+**Request Reason:** {{ review_request_reason }}
+
+**Here is the url for review: <{{ query_execution_url }}>**
+
+Please review the query at your earliest convenience to ensure it meets standards and requirements.

querybook/server/const/query_execution.py

@@ -1,4 +1,5 @@
 from enum import Enum
+from typing import List, TypedDict
 
 # Keep these the same as const/queryExecution.ts
 
@@ -15,6 +16,8 @@ class QueryExecutionStatus(Enum):
     DONE = 3
     ERROR = 4
     CANCEL = 5
+    PENDING_REVIEW = 6
+    REJECTED = 7
 
 
 class StatementExecutionStatus(Enum):
@@ -45,4 +48,9 @@ class QueryEngineStatus(Enum):
     ERROR = 3  # Executor is down, queries cannot be issued
 
 
+class PeerReviewParamsDict(TypedDict):
+    reviewer_ids: List[int]
+    request_reason: str
+
+
 QUERY_EXECUTION_NAMESPACE = "/query_execution"

querybook/server/datasources/__init__.py

@@ -19,7 +19,7 @@
 from . import survey
 from . import query_transform
 from . import github
-
+from . import query_review
 
 # Keep this at the end of imports to make sure the plugin APIs override the default ones
 try:
@@ -50,3 +50,4 @@
 query_transform
 api_plugin
 github
+query_review