Skip to content

Commit

Permalink
feat: Add peer review system for queries (#1535)
Browse files Browse the repository at this point in the history
feat: Add peer review system for queries 
---------

Co-authored-by: Victor <rongzhang>
  • Loading branch information
zhangvi7 authored Feb 5, 2025
1 parent 6ee8041 commit 34e1cd2
Show file tree
Hide file tree
Showing 60 changed files with 3,359 additions and 126 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "querybook",
"version": "3.38.0",
"version": "3.39.0",
"description": "A Big Data Webapp",
"private": true,
"scripts": {
Expand Down
Empty file.
1 change: 1 addition & 0 deletions plugins/query_review_handler_plugin/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
PLUGIN_QUERY_REVIEW_HANDLER = None
29 changes: 29 additions & 0 deletions querybook/config/querybook_public_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,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.
132 changes: 132 additions & 0 deletions querybook/migrations/versions/d3582302cf48_add_query_review_table.py
Original file line number Diff line number Diff line change
@@ -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 ###
3 changes: 3 additions & 0 deletions querybook/notification_templates/query_review_approved.md
Original file line number Diff line number Diff line change
@@ -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 }}>**
8 changes: 8 additions & 0 deletions querybook/notification_templates/query_review_rejected.md
Original file line number Diff line number Diff line change
@@ -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.
9 changes: 9 additions & 0 deletions querybook/notification_templates/query_review_request.md
Original file line number Diff line number Diff line change
@@ -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.
8 changes: 8 additions & 0 deletions querybook/server/const/query_execution.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from enum import Enum
from typing import List, TypedDict

# Keep these the same as const/queryExecution.ts

Expand All @@ -15,6 +16,8 @@ class QueryExecutionStatus(Enum):
DONE = 3
ERROR = 4
CANCEL = 5
PENDING_REVIEW = 6
REJECTED = 7


class StatementExecutionStatus(Enum):
Expand Down Expand Up @@ -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"
3 changes: 2 additions & 1 deletion querybook/server/datasources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -50,3 +50,4 @@
query_transform
api_plugin
github
query_review
Loading

0 comments on commit 34e1cd2

Please sign in to comment.