Skip to content

Conversation

@ptelang
Copy link
Collaborator

@ptelang ptelang commented Nov 14, 2025

This change will allow user to specify skipped-workloads in an environment variable.

@claude
Copy link

claude bot commented Nov 14, 2025

Code Review

✅ Approval

This PR correctly adds the SKIPPED_WORKLOADS environment variable mapping to _populate_config_from_env().

Analysis

What's good:

  • Follows existing pattern exactly - maps SKIPPED_WORKLOADS env var to skipped_workloads config field
  • Placement maintains alphabetical order in env_mappings dictionary
  • The Pydantic field already exists with proper validator (parse_skipped_workloads at line 256-291)
  • Field is actively used in polling_manager.py and ingestion.py

Observation:

  • No test coverage exists for environment variable mappings in _populate_config_from_env(). While this change is correct, it's untested like all other env mappings.

Recommendation

Approve and merge - Change is correct and maintains consistency.

Optional future improvement: Add integration tests for environment variable loading to prevent regressions.

Copilot finished reviewing on behalf of ptelang November 14, 2025 17:55
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for configuring skipped workloads through the SKIPPED_WORKLOADS environment variable, allowing users to specify workload names to skip during ingestion without requiring a configuration file.

Key Changes:

  • Adds SKIPPED_WORKLOADS to the environment variable mapping in config.py, enabling users to set skipped workloads via environment variable (e.g., SKIPPED_WORKLOADS="inspector,mcp-optimizer,custom-workload")

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

JAORMX
JAORMX previously approved these changes Nov 14, 2025
@ptelang ptelang force-pushed the add-skipped-workload-env-mapping branch from c72e1f4 to 4b472d1 Compare November 14, 2025 18:17
@ptelang ptelang merged commit 027ecee into main Nov 17, 2025
7 checks passed
@ptelang ptelang deleted the add-skipped-workload-env-mapping branch November 17, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants