diff --git a/showtime/cli.py b/showtime/cli.py index 348fbcc..f19c9b5 100644 --- a/showtime/cli.py +++ b/showtime/cli.py @@ -435,6 +435,78 @@ def list( raise typer.Exit(1) from e +@app.command() +def flags( + pr_number: int = typer.Argument(..., help="PR number to manage feature flags for"), + add: Optional[str] = typer.Option(None, "--add", help="Add flag: FLAG_NAME=true/false"), + remove: Optional[str] = typer.Option(None, "--remove", help="Remove flag by name: FLAG_NAME"), +) -> None: + """🚩 Manage Superset feature flags via GitHub labels""" + from .core.feature_flags import ( + create_feature_flag_label, + parse_feature_flag_label, + ) + + try: + pr = PullRequest.from_id(pr_number) + + if add: + if "=" not in add: + p(f"❌ Invalid flag format: {add}") + p("Expected: FLAG_NAME=true or FLAG_NAME=false") + raise typer.Exit(1) + + label = create_feature_flag_label(*add.split("=", 1)) + if not parse_feature_flag_label(label): + p(f"❌ Invalid flag: {add}") + p("Flag name must be uppercase with underscores, value must be true/false") + raise typer.Exit(1) + + pr.add_label(label) + p(f"🚩 Added: {label}") + p("Flag will take effect on next deployment (push or trigger-start)") + return + + if remove: + flag_name = remove.strip().upper() + removed = False + for val in ["true", "false"]: + label = create_feature_flag_label(flag_name, val) + if label in pr.labels: + pr.remove_label(label) + p(f"🚩 Removed: {label}") + removed = True + if not removed: + p(f"🚩 No feature flag label found for {flag_name}") + return + + # Default: list current feature flags + feature_flags = pr.get_feature_flags() + if not feature_flags: + p(f"🚩 No feature flags set for PR #{pr_number}") + p("Add flags with: showtime flags {pr} --add EMBEDDED_SUPERSET=true") + return + + table = Table(title=f"🚩 Feature Flags - PR #{pr_number}") + table.add_column("Flag", style="cyan") + table.add_column("Enabled", style="green") + table.add_column("Label", style="dim") + + for flag_name, enabled in sorted(feature_flags.items()): + status = "✅ true" if enabled else "❌ false" + label = create_feature_flag_label(flag_name, "true" if enabled else "false") + table.add_row(flag_name, status, label) + + p(table) + + except GitHubError as e: + p(f"❌ GitHub error: {e}") + raise typer.Exit(1) from e + except Exception as e: + p(f"❌ Error: {e}") + raise typer.Exit(1) from e + + @app.command() def labels() -> None: """🎪 Show complete circus tent label reference""" @@ -475,6 +547,17 @@ def labels() -> None: p(state_table) p() + # Feature Flag Labels + p("[bold magenta]🚩 Feature Flag Labels (PR-level, persist across rebuilds):[/bold magenta]") + flag_table = Table() + flag_table.add_column("Label", style="green") + flag_table.add_column("Description", style="dim") + flag_table.add_row("🎪 🚩 FLAG_NAME=true", "Enable a Superset feature flag") + flag_table.add_row("🎪 🚩 FLAG_NAME=false", "Disable a Superset feature flag") + p(flag_table) + p("[dim]💡 Use 'showtime flags ' to manage feature flags via CLI[/dim]") + p() + # Workflow Examples p("[bold magenta]🎪 Complete Workflow Examples:[/bold magenta]") p() @@ -720,7 +803,9 @@ def cleanup( if max_age_cap_hours: p(f" (with max age cap of {max_age_cap_hours}h)") else: - p(f"🎪 [bold blue]Cleaning environments older than {default_max_age_hours}h...[/bold blue]") + p( + f"🎪 [bold blue]Cleaning environments older than {default_max_age_hours}h...[/bold blue]" + ) # Get all PRs with environments pr_numbers = PullRequest.find_all_with_environments() @@ -751,7 +836,9 @@ def cleanup( continue else: # Invalid/unparsable TTL label - warn and use default - p(f"⚠️ PR #{pr_number}: Invalid TTL '{ttl_value}', using default {default_max_age_hours}h") + p( + f"⚠️ PR #{pr_number}: Invalid TTL '{ttl_value}', using default {default_max_age_hours}h" + ) effective_max_age = default_max_age_hours else: # No TTL label - use default diff --git a/showtime/core/constants.py b/showtime/core/constants.py index 1619038..e1cdb73 100644 --- a/showtime/core/constants.py +++ b/showtime/core/constants.py @@ -8,3 +8,7 @@ # Maximum age for considering environments stale/orphaned DEFAULT_CLEANUP_AGE = "48h" + +# Feature flag label constants +FEATURE_FLAG_ENV_PREFIX = "SUPERSET_FEATURE_" +FEATURE_FLAG_LABEL_PREFIX = "🎪 🚩 " diff --git a/showtime/core/emojis.py b/showtime/core/emojis.py index ec94fa5..b1cd6e2 100644 --- a/showtime/core/emojis.py +++ b/showtime/core/emojis.py @@ -19,6 +19,7 @@ "🌐": "ip", # Globe for IP address "⌛": "ttl", # Hourglass for time-to-live "🤡": "requested_by", # Clown for who requested (circus theme!) + "🚩": "feature_flag", # Flag for Superset FEATURE_FLAGS } # Reverse mapping for creating labels diff --git a/showtime/core/feature_flags.py b/showtime/core/feature_flags.py new file mode 100644 index 0000000..b585a24 --- /dev/null +++ b/showtime/core/feature_flags.py @@ -0,0 +1,117 @@ +""" +🎪 Feature flag utilities for circus tent label management + +Parses and creates feature flag labels: 🎪 🚩 FLAG_NAME=value +Converts to AWS ECS environment variable format. +""" + +import re +from typing import Dict, List, Optional, Set, Tuple + +from .constants import FEATURE_FLAG_ENV_PREFIX, FEATURE_FLAG_LABEL_PREFIX + +# Regex for valid flag names: uppercase letters, digits, underscores, starting with a letter +_FLAG_NAME_RE = re.compile(r"^[A-Z][A-Z0-9_]*$") + + +def parse_feature_flag_label(label: str) -> Optional[Tuple[str, str]]: + """Parse a feature flag label into (flag_name, value). + + Args: + label: Label like "🎪 🚩 EMBEDDED_SUPERSET=true" + + Returns: + Tuple of (flag_name, value) like ("EMBEDDED_SUPERSET", "true") + or None if not a valid feature flag label. + """ + if not label.startswith(FEATURE_FLAG_LABEL_PREFIX): + return None + + flag_part = label[len(FEATURE_FLAG_LABEL_PREFIX) :].strip() + + if "=" not in flag_part: + return None + + flag_name, value = flag_part.split("=", 1) + flag_name = flag_name.strip().upper() + value = value.strip().lower() + + if not _FLAG_NAME_RE.match(flag_name): + return None + + if value not in ("true", "false"): + return None + + return (flag_name, value) + + +def is_feature_flag_label(label: str) -> bool: + """Check if a label is a feature flag label.""" + return label.startswith(FEATURE_FLAG_LABEL_PREFIX) + + +def create_feature_flag_label(flag_name: str, value: str = "true") -> str: + """Create a feature flag label string. + + Args: + flag_name: Flag name like "EMBEDDED_SUPERSET" + value: "true" or "false" + + Returns: + Label string like "🎪 🚩 EMBEDDED_SUPERSET=true" + """ + return f"{FEATURE_FLAG_LABEL_PREFIX}{flag_name.upper()}={value.lower()}" + + +def extract_feature_flags_from_labels(labels: Set[str]) -> Dict[str, bool]: + """Extract all feature flags from a set of PR labels. + + Args: + labels: Set of all PR labels + + Returns: + Dict mapping flag name to boolean value. + Example: {"EMBEDDED_SUPERSET": True, "DASHBOARD_NATIVE_FILTERS": False} + """ + flags: Dict[str, bool] = {} + for label in labels: + parsed = parse_feature_flag_label(label) + if parsed: + flag_name, value = parsed + flags[flag_name] = value == "true" + return flags + + +def feature_flags_to_aws_env(flags: Dict[str, bool]) -> List[Dict[str, str]]: + """Convert feature flags dict to AWS ECS environment variable format. + + Args: + flags: Dict like {"EMBEDDED_SUPERSET": True} + + Returns: + List like [{"name": "SUPERSET_FEATURE_EMBEDDED_SUPERSET", "value": "True"}] + This format matches what aws.create_environment() expects. + """ + env_vars: List[Dict[str, str]] = [] + for flag_name, enabled in sorted(flags.items()): + env_vars.append( + { + "name": f"{FEATURE_FLAG_ENV_PREFIX}{flag_name}", + "value": "True" if enabled else "False", + } + ) + return env_vars + + +def feature_flags_to_prefixed_dict(flags: Dict[str, bool]) -> Dict[str, bool]: + """Convert feature flags dict to SUPERSET_FEATURE_ prefixed dict. + + This format matches what aws.update_feature_flags() expects. + + Args: + flags: Dict like {"EMBEDDED_SUPERSET": True} + + Returns: + Dict like {"SUPERSET_FEATURE_EMBEDDED_SUPERSET": True} + """ + return {f"{FEATURE_FLAG_ENV_PREFIX}{name}": enabled for name, enabled in flags.items()} diff --git a/showtime/core/github_messages.py b/showtime/core/github_messages.py index d215715..9625a35 100644 --- a/showtime/core/github_messages.py +++ b/showtime/core/github_messages.py @@ -112,13 +112,17 @@ def start_comment(show: Show) -> str: def success_comment( - show: Show, feature_count: Optional[int] = None, ttl: Optional[str] = None + show: Show, + feature_count: Optional[int] = None, + feature_names: Optional[List[str]] = None, + ttl: Optional[str] = None, ) -> str: """Environment success comment Args: show: Show object with SHA, IP, etc. feature_count: Number of enabled feature flags (optional) + feature_names: List of enabled feature flag names (optional) ttl: Override TTL display (PR-level TTL takes precedence) """ links = _create_header_links(show.sha) @@ -131,7 +135,8 @@ def success_comment( ] if feature_count: - bullets.insert(-1, f"**Features:** {feature_count} enabled") + flag_list = f" ({', '.join(feature_names)})" if feature_names else "" + bullets.insert(-1, f"**Features:** {feature_count} flags enabled{flag_list}") bullets.append("**Updates:** New commits create fresh environments automatically") @@ -195,13 +200,19 @@ def rolling_start_comment(current_show: Show, new_sha: str) -> str: def rolling_success_comment( - old_show: Show, new_show: Show, ttl: Optional[str] = None + old_show: Show, + new_show: Show, + feature_count: Optional[int] = None, + feature_names: Optional[List[str]] = None, + ttl: Optional[str] = None, ) -> str: """Rolling update success comment Args: old_show: Previous Show object new_show: New Show object with updated IP, SHA + feature_count: Number of enabled feature flags (optional) + feature_names: List of enabled feature flag names (optional) ttl: TTL display string (required - TTL is now PR-level, not per-Show) """ from .constants import DEFAULT_TTL @@ -214,9 +225,14 @@ def rolling_success_comment( f"**Environment:** http://{new_show.ip}:8080 (admin/admin)", f"**Lifetime:** {effective_ttl} auto-cleanup", "**Deployment:** Zero-downtime blue-green", - "**Updates:** New commits create fresh environments automatically", ] + if feature_count: + flag_list = f" ({', '.join(feature_names)})" if feature_names else "" + bullets.append(f"**Features:** {feature_count} flags enabled{flag_list}") + + bullets.append("**Updates:** New commits create fresh environments automatically") + return _format_comment(header, bullets) diff --git a/showtime/core/label_colors.py b/showtime/core/label_colors.py index 2a92a8b..739570e 100644 --- a/showtime/core/label_colors.py +++ b/showtime/core/label_colors.py @@ -14,6 +14,8 @@ "status_building": "FFD700", # Bright yellow - in progress "status_failed": "dc3545", # Red - error/failed "status_updating": "fd7e14", # Orange - updating/transitioning + # Feature Flags + "feature_flag": "A855F7", # Purple - feature flag toggles } # Label Definitions with Colors and Descriptions @@ -56,6 +58,133 @@ "color": "FFE4B5", # Light orange "description": "Environment expires only when PR is closed", }, + # Feature Flag Labels (PR-level, reusable) + # Superset FEATURE_FLAGS from superset_config.py + # Note: ENABLE_DASHBOARD_DOWNLOAD_WEBDRIVER_SCREENSHOT excluded (exceeds 50-char GitHub limit) + "🎪 🚩 GLOBAL_ASYNC_QUERIES=true": { + "color": COLORS["feature_flag"], + "description": "Enable Global Async Queries", + }, + "🎪 🚩 ENABLE_EXTENSIONS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Extensions", + }, + "🎪 🚩 THUMBNAILS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Thumbnails", + }, + "🎪 🚩 THUMBNAILS_SQLA_LISTENERS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Thumbnails SQLA Listeners", + }, + "🎪 🚩 ENABLE_DASHBOARD_SCREENSHOT_ENDPOINTS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Dashboard Screenshot Endpoints", + }, + "🎪 🚩 ALERT_REPORTS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Alerts and Reports", + }, + "🎪 🚩 ALERT_REPORT_TABS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Alert Report Tabs", + }, + "🎪 🚩 ALERT_REPORTS_FILTER=true": { + "color": COLORS["feature_flag"], + "description": "Enable Alert Reports Filter", + }, + "🎪 🚩 DASHBOARD_NATIVE_FILTERS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Dashboard Native Filters", + }, + "🎪 🚩 DASHBOARD_VIRTUALIZATION=true": { + "color": COLORS["feature_flag"], + "description": "Enable Dashboard Virtualization", + }, + "🎪 🚩 HORIZONTAL_FILTER_BAR=true": { + "color": COLORS["feature_flag"], + "description": "Enable Horizontal Filter Bar", + }, + "🎪 🚩 DASHBOARD_CROSS_FILTERS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Dashboard Cross Filters", + }, + "🎪 🚩 EMBEDDED_SUPERSET=true": { + "color": COLORS["feature_flag"], + "description": "Enable Embedded Superset", + }, + "🎪 🚩 EMBEDDABLE_CHARTS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Embeddable Charts", + }, + "🎪 🚩 ENABLE_SNOWFLAKE_OAUTH=true": { + "color": COLORS["feature_flag"], + "description": "Enable Snowflake OAuth", + }, + "🎪 🚩 CSS_TEMPLATES=true": { + "color": COLORS["feature_flag"], + "description": "Enable CSS Templates", + }, + "🎪 🚩 TAGGING_SYSTEM=true": { + "color": COLORS["feature_flag"], + "description": "Enable Tagging System", + }, + "🎪 🚩 ENABLE_TEMPLATE_PROCESSING=true": { + "color": COLORS["feature_flag"], + "description": "Enable SQL Template Processing", + }, + "🎪 🚩 THEME_ENABLE_DARK_THEME_SWITCH=true": { + "color": COLORS["feature_flag"], + "description": "Enable Dark Theme Switch", + }, + "🎪 🚩 THEME_ALLOW_THEME_EDITOR_BETA=true": { + "color": COLORS["feature_flag"], + "description": "Enable Theme Editor Beta", + }, + "🎪 🚩 DBT_CLOUD_SYNC=true": { + "color": COLORS["feature_flag"], + "description": "Enable dbt Cloud Sync", + }, + "🎪 🚩 DATE_FILTER_CONTROL_ENHANCED=true": { + "color": COLORS["feature_flag"], + "description": "Enable Enhanced Date Filter Control", + }, + "🎪 🚩 MATRIXIFY=true": { + "color": COLORS["feature_flag"], + "description": "Enable Matrixify", + }, + "🎪 🚩 SQLLAB_BACKEND_PERSISTENCE=true": { + "color": COLORS["feature_flag"], + "description": "Enable SQL Lab Backend Persistence", + }, + "🎪 🚩 SSH_TUNNELING=true": { + "color": COLORS["feature_flag"], + "description": "Enable SSH Tunneling", + }, + "🎪 🚩 ALERT_REPORT_SLACK_V2=true": { + "color": COLORS["feature_flag"], + "description": "Enable Alert Report Slack V2", + }, + "🎪 🚩 DRILL_TO_DETAIL=true": { + "color": COLORS["feature_flag"], + "description": "Enable Drill to Detail", + }, + "🎪 🚩 TABLE_V2_TIME_COMPARISON_ENABLED=true": { + "color": COLORS["feature_flag"], + "description": "Enable Table V2 Time Comparison", + }, + "🎪 🚩 AG_GRID_TABLE_ENABLED=true": { + "color": COLORS["feature_flag"], + "description": "Enable AG Grid Table", + }, + "🎪 🚩 DASHBOARD_RBAC=true": { + "color": COLORS["feature_flag"], + "description": "Enable Dashboard RBAC", + }, + "🎪 🚩 PLAYWRIGHT_REPORTS_AND_THUMBNAILS=true": { + "color": COLORS["feature_flag"], + "description": "Enable Playwright Reports and Thumbnails", + }, } # Status-specific label patterns (generated dynamically) @@ -77,6 +206,10 @@ def get_label_color(label_text: str) -> str: if label_text in LABEL_DEFINITIONS: return LABEL_DEFINITIONS[label_text]["color"] + # Check for feature flag labels + if " 🚩 " in label_text: + return COLORS["feature_flag"] + # Check for status labels with dynamic SHA if " 🚦 " in label_text: status = label_text.split(" 🚦 ")[-1] @@ -97,6 +230,15 @@ def get_label_description(label_text: str) -> str: if label_text in LABEL_DEFINITIONS: return LABEL_DEFINITIONS[label_text]["description"] + # Feature flag labels + if " 🚩 " in label_text: + flag_part = label_text.replace("🎪 🚩 ", "").strip() + if "=" in flag_part: + flag_name, value = flag_part.split("=", 1) + state = "enabled" if value.lower() == "true" else "disabled" + return f"Superset feature flag {flag_name} is {state}" + return f"Superset feature flag: {flag_part}" + # Dynamic descriptions for SHA-based labels if " 🚦 " in label_text: sha, status = label_text.replace("🎪 ", "").split(" 🚦 ") diff --git a/showtime/core/pull_request.py b/showtime/core/pull_request.py index 5a41b66..9b48357 100644 --- a/showtime/core/pull_request.py +++ b/showtime/core/pull_request.py @@ -5,7 +5,7 @@ """ from dataclasses import dataclass -from typing import Any, List, Optional +from typing import Any, Dict, List, Optional from .aws import AWSInterface from .github import GitHubInterface @@ -144,6 +144,30 @@ def _get_effective_ttl_display(self) -> str: return DEFAULT_TTL + def get_feature_flags(self) -> Dict[str, bool]: + """Get feature flags from PR-level labels. + + Looks for reusable PR-level feature flag labels like "🎪 🚩 EMBEDDED_SUPERSET=true". + + Returns: + Dict mapping flag name to enabled status. + Example: {"EMBEDDED_SUPERSET": True, "DASHBOARD_NATIVE_FILTERS": False} + """ + from .feature_flags import extract_feature_flags_from_labels + + return extract_feature_flags_from_labels(self.labels) + + def get_feature_flags_as_aws_env(self) -> List[Dict[str, str]]: + """Get feature flags formatted for AWS ECS environment variables. + + Returns: + List of {"name": "SUPERSET_FEATURE_X", "value": "True"/"False"} dicts. + Empty list if no feature flags are set. + """ + from .feature_flags import feature_flags_to_aws_env + + return feature_flags_to_aws_env(self.get_feature_flags()) + def _parse_shows_from_labels(self) -> List[Show]: """Parse all shows from circus tent labels""" # Find all unique SHAs from circus labels @@ -367,6 +391,15 @@ def analyze(self, target_sha: str, pr_state: str = "open") -> SyncState: } action_needed = action_map.get(action_needed_str, ActionNeeded.NO_ACTION) + # Check if feature flags need hot-update even when no deployment action + has_feature_flags = bool(self.get_feature_flags()) + needs_flag_update = ( + action_needed == ActionNeeded.NO_ACTION + and has_feature_flags + and self.current_show is not None + and self.current_show.is_running + ) + # Build sync state return SyncState( action_needed=action_needed, @@ -376,7 +409,8 @@ def analyze(self, target_sha: str, pr_state: str = "open") -> SyncState: ActionNeeded.ROLLING_UPDATE, ActionNeeded.AUTO_SYNC, ], - sync_needed=action_needed not in [ActionNeeded.NO_ACTION, ActionNeeded.BLOCKED], + sync_needed=action_needed not in [ActionNeeded.NO_ACTION, ActionNeeded.BLOCKED] + or needs_flag_update, target_sha=target_sha, github_actor=auth_debug.get("actor", "unknown"), is_github_actions=auth_debug.get("is_github_actions", False), @@ -524,6 +558,13 @@ def sync( ) print("✅ Environment claimed successfully") + # Extract feature flags from PR labels (PR-level, persist across rebuilds) + feature_flags_env = self.get_feature_flags_as_aws_env() + if feature_flags_env: + print(f"🚩 Feature flags: {len(feature_flags_env)} flags set") + for ff in feature_flags_env: + print(f" • {ff['name']}={ff['value']}") + try: # 3. Execute action with error handling if action_needed == "create_environment": @@ -539,7 +580,7 @@ def sync( # Phase 2: AWS deployment print("☁️ Deploying to AWS ECS...") self.set_show_status(show, "deploying") - show.deploy_aws(dry_run_aws) + show.deploy_aws(dry_run_aws, feature_flags=feature_flags_env) self.set_show_status(show, "running") self.set_active_show(show) print(f"✅ Deployment completed - environment running at {show.ip}:8080") @@ -576,7 +617,7 @@ def sync( # Phase 2: Blue-green deployment print("☁️ Deploying updated environment...") self.set_show_status(new_show, "deploying") - new_show.deploy_aws(dry_run_aws) + new_show.deploy_aws(dry_run_aws, feature_flags=feature_flags_env) self.set_show_status(new_show, "running") self.set_active_show(new_show) print(f"✅ Rolling update completed - new environment at {new_show.ip}:8080") @@ -616,6 +657,34 @@ def sync( return SyncResult(success=True, action_taken="destroy_environment") else: + # No deployment action needed - but check for feature flag changes + # on the running environment and hot-update if needed + if feature_flags_env and self.current_show and self.current_show.is_running: + from .feature_flags import feature_flags_to_prefixed_dict + + prefixed_flags = feature_flags_to_prefixed_dict(self.get_feature_flags()) + print( + f"🚩 Hot-updating {len(prefixed_flags)} feature flags on " + f"{self.current_show.ecs_service_name}..." + ) + success = self.current_show.update_feature_flags( + prefixed_flags, dry_run=dry_run_aws + ) + if success: + print("✅ Feature flags updated (container will restart)") + return SyncResult( + success=True, + action_taken="update_feature_flags", + show=self.current_show, + ) + else: + print("⚠️ Feature flag hot-update failed") + return SyncResult( + success=False, + action_taken="update_feature_flags", + error="Feature flag hot-update failed", + ) + return SyncResult(success=True, action_taken="no_action") except Exception as e: @@ -924,7 +993,14 @@ def _post_success_comment(self, show: Show, dry_run: bool = False) -> None: if not dry_run: effective_ttl = self._get_effective_ttl_display() - comment = success_comment(show, ttl=effective_ttl) + feature_flags = self.get_feature_flags() + feature_names = sorted(name for name, enabled in feature_flags.items() if enabled) + comment = success_comment( + show, + feature_count=len(feature_names) if feature_names else None, + feature_names=feature_names if feature_names else None, + ttl=effective_ttl, + ) get_github().post_comment(self.pr_number, comment) def _post_rolling_start_comment( @@ -946,7 +1022,15 @@ def _post_rolling_success_comment( if not dry_run: effective_ttl = self._get_effective_ttl_display() - comment = rolling_success_comment(old_show, new_show, ttl=effective_ttl) + feature_flags = self.get_feature_flags() + feature_names = sorted(name for name, enabled in feature_flags.items() if enabled) + comment = rolling_success_comment( + old_show, + new_show, + feature_count=len(feature_names) if feature_names else None, + feature_names=feature_names if feature_names else None, + ttl=effective_ttl, + ) get_github().post_comment(self.pr_number, comment) def _post_cleanup_comment(self, show: Show, dry_run: bool = False) -> None: diff --git a/showtime/core/show.py b/showtime/core/show.py index c65b63c..2653be1 100644 --- a/showtime/core/show.py +++ b/showtime/core/show.py @@ -6,7 +6,7 @@ from dataclasses import dataclass from datetime import datetime -from typing import List, Optional +from typing import Dict, List, Optional # Import interfaces for singleton access @@ -135,7 +135,11 @@ def build_docker(self, dry_run: bool = False) -> None: if not dry_run: self._build_docker_image() # Raises on failure - def deploy_aws(self, dry_run: bool = False) -> None: + def deploy_aws( + self, + dry_run: bool = False, + feature_flags: Optional[List[Dict[str, str]]] = None, + ) -> None: """Deploy to AWS (atomic operation)""" github, aws = get_interfaces() @@ -144,6 +148,7 @@ def deploy_aws(self, dry_run: bool = False) -> None: pr_number=self.pr_number, sha=self.sha + "0" * (40 - len(self.sha)), # Convert to full SHA github_user=self.requested_by or "unknown", + feature_flags=feature_flags, ) if not result.success: @@ -155,6 +160,30 @@ def deploy_aws(self, dry_run: bool = False) -> None: # Mock successful deployment for dry-run self.ip = "52.1.2.3" + def update_feature_flags( + self, + feature_flags: Dict[str, bool], + dry_run: bool = False, + ) -> bool: + """Hot-update feature flags on a running environment. + + Updates the ECS task definition with new env vars and triggers a rolling + restart. No Docker rebuild needed. + + Args: + feature_flags: Dict with SUPERSET_FEATURE_ prefixed keys and bool values. + dry_run: If True, skip actual AWS call. + + Returns: + True if successful, False otherwise. + """ + if dry_run: + print(f"🚩 [DRY-RUN] Would hot-update {len(feature_flags)} feature flags") + return True + + _, aws = get_interfaces() + return aws.update_feature_flags(self.ecs_service_name, feature_flags) + def stop(self, dry_run_github: bool = False, dry_run_aws: bool = False) -> bool: """Stop this environment (cleanup AWS resources) @@ -170,6 +199,53 @@ def stop(self, dry_run_github: bool = False, dry_run_aws: bool = False) -> bool: return True # Dry run is always "successful" + def _inject_config_overlay(self) -> None: + """Inject superset_config_docker.py into the build context. + + Copies our config overlay into the Superset repo's build context and + appends a COPY instruction to the Dockerfile so the config ends up in + /app/pythonpath/ inside the image. This ensures SUPERSET_FEATURE_* + env vars always override FEATURE_FLAGS, even if a superset_config.py + is present. + """ + import shutil + from pathlib import Path + + # Source: bundled config overlay in our package data + src = Path(__file__).parent.parent / "data" / "superset_config_docker.py" + if not src.exists(): + print("⚠️ superset_config_docker.py not found, skipping config injection") + return + + # Destination: build context (current working directory = Superset repo) + dest = Path("docker") / "pythonpath_dev" / "superset_config_docker.py" + dest.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(src, dest) + print(f"🚩 Injected {dest} into build context") + + # Append a COPY instruction to the Dockerfile so it ends up in the image + dockerfile = Path("Dockerfile") + if not dockerfile.exists(): + print("⚠️ Dockerfile not found, skipping Dockerfile patching") + return + + content = dockerfile.read_text() + if "superset_config_docker.py" in content: + print("🚩 Dockerfile already has config overlay, skipping patch") + return + + # Add a COPY after the showtime stage definition + patched = content.replace( + "FROM lean AS showtime", + "FROM lean AS showtime\n" + "COPY docker/pythonpath_dev/superset_config_docker.py /app/pythonpath/", + ) + if patched != content: + dockerfile.write_text(patched) + print("🚩 Patched Dockerfile to COPY config overlay into /app/pythonpath/") + else: + print("⚠️ Could not find showtime stage in Dockerfile, skipping patch") + def _build_docker_image(self) -> None: """Build Docker image for this environment""" import os @@ -177,6 +253,9 @@ def _build_docker_image(self) -> None: tag = f"apache/superset:pr-{self.pr_number}-{self.sha}-ci" + # Inject config overlay so SUPERSET_FEATURE_* env vars work + self._inject_config_overlay() + # Detect if running in CI environment is_ci = bool(os.getenv("GITHUB_ACTIONS") or os.getenv("CI")) diff --git a/showtime/data/superset_config_docker.py b/showtime/data/superset_config_docker.py new file mode 100644 index 0000000..e408d12 --- /dev/null +++ b/showtime/data/superset_config_docker.py @@ -0,0 +1,20 @@ +# +# Showtime config overlay — injected during ephemeral environment Docker builds. +# +# This file is imported at the end of superset_config.py via: +# from superset_config_docker import * +# +# It reads SUPERSET_FEATURE_* env vars and overrides FEATURE_FLAGS entries, +# giving GitHub PR labels the final say over feature flag values. +# +import os +import re + +# Merge SUPERSET_FEATURE_* env vars into FEATURE_FLAGS (overriding any defaults) +_feature_env_re = re.compile(r"^SUPERSET_FEATURE_(\w+)$") + +for _key, _val in os.environ.items(): + _m = _feature_env_re.match(_key) + if _m: + _flag_name = _m.group(1) + FEATURE_FLAGS[_flag_name] = _val.lower() in ("true", "1", "yes", "on", "t", "y") # noqa: F821 diff --git a/tests/unit/test_feature_flags.py b/tests/unit/test_feature_flags.py new file mode 100644 index 0000000..d968152 --- /dev/null +++ b/tests/unit/test_feature_flags.py @@ -0,0 +1,393 @@ +""" +Tests for feature flag functionality + +Tests cover: +- parse_feature_flag_label() parsing function +- create_feature_flag_label() creation function +- is_feature_flag_label() detection function +- extract_feature_flags_from_labels() extraction +- feature_flags_to_aws_env() conversion +- PullRequest.get_feature_flags() label extraction +- PullRequest.get_feature_flags_as_aws_env() AWS format +- Label color and description generation for feature flag labels +""" + +from showtime.core.feature_flags import ( + create_feature_flag_label, + extract_feature_flags_from_labels, + feature_flags_to_aws_env, + feature_flags_to_prefixed_dict, + is_feature_flag_label, + parse_feature_flag_label, +) +from showtime.core.label_colors import get_label_color, get_label_description +from showtime.core.pull_request import PullRequest + + +class TestParseFeatureFlagLabel: + """Tests for the parse_feature_flag_label helper function""" + + def test_valid_true_flag(self) -> None: + result = parse_feature_flag_label("🎪 🚩 EMBEDDED_SUPERSET=true") + assert result == ("EMBEDDED_SUPERSET", "true") + + def test_valid_false_flag(self) -> None: + result = parse_feature_flag_label("🎪 🚩 DASHBOARD_NATIVE_FILTERS=false") + assert result == ("DASHBOARD_NATIVE_FILTERS", "false") + + def test_non_flag_label_returns_none(self) -> None: + assert parse_feature_flag_label("🎪 ⌛ 48h") is None + assert parse_feature_flag_label("bug") is None + assert parse_feature_flag_label("🎪 abc123f 🚦 running") is None + + def test_invalid_value_returns_none(self) -> None: + assert parse_feature_flag_label("🎪 🚩 FLAG=maybe") is None + assert parse_feature_flag_label("🎪 🚩 FLAG=yes") is None + assert parse_feature_flag_label("🎪 🚩 FLAG=1") is None + + def test_missing_equals_returns_none(self) -> None: + assert parse_feature_flag_label("🎪 🚩 JUST_A_NAME") is None + + def test_case_normalization(self) -> None: + """Flag names are uppercased, values are lowercased""" + result = parse_feature_flag_label("🎪 🚩 embedded_superset=True") + assert result == ("EMBEDDED_SUPERSET", "true") + + result = parse_feature_flag_label("🎪 🚩 Drill_To_Detail=FALSE") + assert result == ("DRILL_TO_DETAIL", "false") + + def test_invalid_flag_name_returns_none(self) -> None: + """Flag names must start with a letter and contain only A-Z, 0-9, _""" + assert parse_feature_flag_label("🎪 🚩 123_BAD=true") is None + assert parse_feature_flag_label("🎪 🚩 =true") is None + assert parse_feature_flag_label("🎪 🚩 FLAG-NAME=true") is None + + def test_empty_value_returns_none(self) -> None: + assert parse_feature_flag_label("🎪 🚩 FLAG=") is None + + +class TestIsFeatureFlagLabel: + """Tests for the is_feature_flag_label function""" + + def test_valid_flag_label(self) -> None: + assert is_feature_flag_label("🎪 🚩 EMBEDDED_SUPERSET=true") is True + + def test_non_flag_labels(self) -> None: + assert is_feature_flag_label("🎪 ⌛ 48h") is False + assert is_feature_flag_label("bug") is False + assert is_feature_flag_label("🎪 abc123f 🚦 running") is False + + +class TestCreateFeatureFlagLabel: + """Tests for the create_feature_flag_label function""" + + def test_create_true_flag(self) -> None: + label = create_feature_flag_label("EMBEDDED_SUPERSET", "true") + assert label == "🎪 🚩 EMBEDDED_SUPERSET=true" + + def test_create_false_flag(self) -> None: + label = create_feature_flag_label("DASHBOARD_NATIVE_FILTERS", "false") + assert label == "🎪 🚩 DASHBOARD_NATIVE_FILTERS=false" + + def test_default_value_is_true(self) -> None: + label = create_feature_flag_label("EMBEDDED_SUPERSET") + assert label == "🎪 🚩 EMBEDDED_SUPERSET=true" + + def test_case_normalization(self) -> None: + label = create_feature_flag_label("embedded_superset", "True") + assert label == "🎪 🚩 EMBEDDED_SUPERSET=true" + + def test_roundtrip(self) -> None: + """Creating and parsing should roundtrip correctly""" + label = create_feature_flag_label("DRILL_TO_DETAIL", "false") + result = parse_feature_flag_label(label) + assert result == ("DRILL_TO_DETAIL", "false") + + +class TestExtractFeatureFlags: + """Tests for extract_feature_flags_from_labels""" + + def test_single_flag(self) -> None: + labels = {"🎪 🚩 EMBEDDED_SUPERSET=true", "bug"} + flags = extract_feature_flags_from_labels(labels) + assert flags == {"EMBEDDED_SUPERSET": True} + + def test_multiple_flags(self) -> None: + labels = { + "🎪 🚩 EMBEDDED_SUPERSET=true", + "🎪 🚩 DRILL_TO_DETAIL=false", + "🎪 abc123f 🚦 running", + } + flags = extract_feature_flags_from_labels(labels) + assert flags == {"EMBEDDED_SUPERSET": True, "DRILL_TO_DETAIL": False} + + def test_no_flags(self) -> None: + labels = {"bug", "🎪 ⌛ 48h", "🎪 abc123f 🚦 running"} + flags = extract_feature_flags_from_labels(labels) + assert flags == {} + + def test_empty_labels(self) -> None: + flags = extract_feature_flags_from_labels(set()) + assert flags == {} + + def test_ignores_invalid_flag_labels(self) -> None: + labels = { + "🎪 🚩 VALID_FLAG=true", + "🎪 🚩 INVALID=maybe", # bad value + "🎪 🚩 JUST_NAME", # no value + } + flags = extract_feature_flags_from_labels(labels) + assert flags == {"VALID_FLAG": True} + + +class TestFeatureFlagsToAwsEnv: + """Tests for feature_flags_to_aws_env conversion""" + + def test_single_true_flag(self) -> None: + flags = {"EMBEDDED_SUPERSET": True} + result = feature_flags_to_aws_env(flags) + assert result == [{"name": "SUPERSET_FEATURE_EMBEDDED_SUPERSET", "value": "True"}] + + def test_single_false_flag(self) -> None: + flags = {"DRILL_TO_DETAIL": False} + result = feature_flags_to_aws_env(flags) + assert result == [{"name": "SUPERSET_FEATURE_DRILL_TO_DETAIL", "value": "False"}] + + def test_multiple_flags_sorted(self) -> None: + flags = {"DRILL_TO_DETAIL": False, "EMBEDDED_SUPERSET": True} + result = feature_flags_to_aws_env(flags) + assert result == [ + {"name": "SUPERSET_FEATURE_DRILL_TO_DETAIL", "value": "False"}, + {"name": "SUPERSET_FEATURE_EMBEDDED_SUPERSET", "value": "True"}, + ] + + def test_empty_flags(self) -> None: + assert feature_flags_to_aws_env({}) == [] + + +class TestPullRequestFeatureFlags: + """Tests for PullRequest feature flag methods""" + + def test_get_feature_flags_single(self) -> None: + pr = PullRequest(1234, ["🎪 🚩 EMBEDDED_SUPERSET=true", "bug"]) + flags = pr.get_feature_flags() + assert flags == {"EMBEDDED_SUPERSET": True} + + def test_get_feature_flags_multiple(self) -> None: + pr = PullRequest( + 1234, + [ + "🎪 🚩 EMBEDDED_SUPERSET=true", + "🎪 🚩 DRILL_TO_DETAIL=false", + "🎪 abc123f 🚦 running", + ], + ) + flags = pr.get_feature_flags() + assert flags == {"EMBEDDED_SUPERSET": True, "DRILL_TO_DETAIL": False} + + def test_get_feature_flags_none(self) -> None: + pr = PullRequest(1234, ["bug", "🎪 ⌛ 48h"]) + assert pr.get_feature_flags() == {} + + def test_get_feature_flags_empty_labels(self) -> None: + pr = PullRequest(1234, []) + assert pr.get_feature_flags() == {} + + def test_get_feature_flags_as_aws_env(self) -> None: + pr = PullRequest(1234, ["🎪 🚩 EMBEDDED_SUPERSET=true"]) + env = pr.get_feature_flags_as_aws_env() + assert env == [{"name": "SUPERSET_FEATURE_EMBEDDED_SUPERSET", "value": "True"}] + + def test_get_feature_flags_as_aws_env_empty(self) -> None: + pr = PullRequest(1234, ["bug"]) + env = pr.get_feature_flags_as_aws_env() + assert env == [] + + def test_feature_flags_persist_across_sha(self) -> None: + """Feature flags are PR-level - they don't change when SHA changes""" + labels_v1 = [ + "🎪 🚩 EMBEDDED_SUPERSET=true", + "🎪 abc123f 🚦 running", + "🎪 🎯 abc123f", + ] + pr1 = PullRequest(1234, labels_v1) + flags_before = pr1.get_feature_flags() + + # Simulate SHA change - flag labels stay, SHA labels change + labels_v2 = [ + "🎪 🚩 EMBEDDED_SUPERSET=true", + "🎪 def456a 🚦 building", + ] + pr2 = PullRequest(1234, labels_v2) + flags_after = pr2.get_feature_flags() + + assert flags_before == flags_after + + def test_feature_flags_with_mixed_labels(self) -> None: + """Feature flags work alongside all other label types""" + labels = [ + "🎪 ⚡ showtime-trigger-start", + "🎪 ⌛ 1w", + "🎪 🚩 EMBEDDED_SUPERSET=true", + "🎪 🚩 DRILL_TO_DETAIL=false", + "🎪 abc123f 🚦 running", + "🎪 🎯 abc123f", + "🎪 abc123f 📅 2024-01-15T14-30", + "🎪 abc123f 🤡 maxime", + "bug", + "enhancement", + ] + pr = PullRequest(1234, labels) + flags = pr.get_feature_flags() + assert flags == {"EMBEDDED_SUPERSET": True, "DRILL_TO_DETAIL": False} + + # TTL should still work independently + assert pr.get_pr_ttl_hours() == 168 + + +class TestFeatureFlagLabelColors: + """Tests for feature flag label color and description generation""" + + def test_predefined_flag_color(self) -> None: + color = get_label_color("🎪 🚩 EMBEDDED_SUPERSET=true") + assert color == "A855F7" # Purple + + def test_dynamic_flag_color(self) -> None: + """Non-predefined flags also get purple""" + color = get_label_color("🎪 🚩 CUSTOM_FLAG=true") + assert color == "A855F7" + + def test_predefined_flag_description(self) -> None: + desc = get_label_description("🎪 🚩 EMBEDDED_SUPERSET=true") + assert "Embedded Superset" in desc + + def test_dynamic_flag_enabled_description(self) -> None: + desc = get_label_description("🎪 🚩 CUSTOM_FLAG=true") + assert "CUSTOM_FLAG" in desc + assert "enabled" in desc + + def test_dynamic_flag_disabled_description(self) -> None: + desc = get_label_description("🎪 🚩 CUSTOM_FLAG=false") + assert "CUSTOM_FLAG" in desc + assert "disabled" in desc + + +class TestFeatureFlagsToPrefixedDict: + """Tests for feature_flags_to_prefixed_dict conversion (used by hot-update)""" + + def test_single_flag(self) -> None: + flags = {"EMBEDDED_SUPERSET": True} + result = feature_flags_to_prefixed_dict(flags) + assert result == {"SUPERSET_FEATURE_EMBEDDED_SUPERSET": True} + + def test_multiple_flags(self) -> None: + flags = {"EMBEDDED_SUPERSET": True, "DRILL_TO_DETAIL": False} + result = feature_flags_to_prefixed_dict(flags) + assert result == { + "SUPERSET_FEATURE_EMBEDDED_SUPERSET": True, + "SUPERSET_FEATURE_DRILL_TO_DETAIL": False, + } + + def test_empty_flags(self) -> None: + assert feature_flags_to_prefixed_dict({}) == {} + + +class TestAnalyzeSyncNeededWithFeatureFlags: + """Tests that analyze() returns sync_needed=True for feature flag hot-updates""" + + def test_analyze_sync_needed_with_flags_and_running_env(self) -> None: + """sync_needed should be True when flags exist on a running environment""" + from unittest.mock import Mock, patch + + from showtime.core.sync_state import ActionNeeded + + labels = [ + "🎪 🚩 EMBEDDED_SUPERSET=true", + "🎪 abc123f 🚦 running", + "🎪 🎯 abc123f", + "🎪 abc123f 📅 2024-01-15T14-30", + ] + + with patch("showtime.core.pull_request.get_github") as mock_get_github: + mock_github = Mock() + mock_get_github.return_value = mock_github + mock_github.get_labels.return_value = labels + + pr = PullRequest(1234, labels) + result = pr.analyze("abc123f", "open") + + assert result.action_needed == ActionNeeded.NO_ACTION + assert result.build_needed is False + assert result.sync_needed is True # Feature flags need hot-update! + + def test_analyze_sync_needed_false_without_flags(self) -> None: + """sync_needed should be False when no flags exist on a running environment""" + from unittest.mock import Mock, patch + + from showtime.core.sync_state import ActionNeeded + + labels = [ + "🎪 abc123f 🚦 running", + "🎪 🎯 abc123f", + ] + + with patch("showtime.core.pull_request.get_github") as mock_get_github: + mock_github = Mock() + mock_get_github.return_value = mock_github + mock_github.get_labels.return_value = labels + + pr = PullRequest(1234, labels) + result = pr.analyze("abc123f", "open") + + assert result.action_needed == ActionNeeded.NO_ACTION + assert result.build_needed is False + assert result.sync_needed is False # No flags, no hot-update needed + + +class TestSyncHotUpdateFeatureFlags: + """Tests for hot-update of feature flags on running environments""" + + def test_sync_no_action_with_flags_triggers_hot_update(self) -> None: + """When sync has no_action but flags exist, it should hot-update""" + from unittest.mock import Mock, patch + + # PR with a running environment and a feature flag label + labels = [ + "🎪 🚩 EMBEDDED_SUPERSET=true", + "🎪 abc123f 🚦 running", + "🎪 🎯 abc123f", + "🎪 abc123f 📅 2024-01-15T14-30", + ] + pr = PullRequest(1234, labels) + + with patch.object(pr, "_determine_action", return_value="no_action"): + with patch.object(pr, "refresh_labels"): + # Mock the current_show's update_feature_flags + assert pr.current_show is not None + pr.current_show.update_feature_flags = Mock(return_value=True) # type: ignore[method-assign] + + result = pr.sync( + "abc123f", dry_run_aws=True, dry_run_github=True, dry_run_docker=True + ) + + assert result.success is True + assert result.action_taken == "update_feature_flags" + + def test_sync_no_action_without_flags_returns_no_action(self) -> None: + """When sync has no_action and no flags, it should return no_action""" + from unittest.mock import patch + + labels = [ + "🎪 abc123f 🚦 running", + "🎪 🎯 abc123f", + ] + pr = PullRequest(1234, labels) + + with patch.object(pr, "_determine_action", return_value="no_action"): + with patch.object(pr, "refresh_labels"): + result = pr.sync( + "abc123f", dry_run_aws=True, dry_run_github=True, dry_run_docker=True + ) + + assert result.success is True + assert result.action_taken == "no_action" diff --git a/tests/unit/test_pull_request.py b/tests/unit/test_pull_request.py index c598f94..36012f4 100644 --- a/tests/unit/test_pull_request.py +++ b/tests/unit/test_pull_request.py @@ -501,7 +501,7 @@ def test_pullrequest_sync_create_environment(mock_get_github: Mock) -> None: # Verify state transitions mock_show.build_docker.assert_called_once_with(True) - mock_show.deploy_aws.assert_called_once_with(True) + mock_show.deploy_aws.assert_called_once_with(True, feature_flags=[]) assert mock_show.status == "running"