diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 6a786be4..3cf9e4cf 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -21,7 +21,6 @@ A single workflow handles all phases of the submission lifecycle: - **`brainscore_language/submission/actions_helpers.py`** — Python CLI used by the orchestrator for: - `validate_pr` — Polls GitHub commit statuses API to check if tests pass - - `trigger_update_existing_metadata` — Triggers the Jenkins `update_existing_metadata` job - `trigger_layer_mapping` — Triggers the Jenkins layer mapping job (non-language domains only) - `extract_email` — Resolves submitter email from GitHub username or Brain-Score user ID - `send_failure_email` — Sends failure notification emails via Gmail SMTP @@ -40,7 +39,7 @@ The orchestrator has 7 numbered jobs: | 3 | **Handle Metadata-Only PR** | Only when PR contains only `metadata.yml` changes | | 4 | **Generate Mutations and Commit** | When metadata generation or layer mapping is needed and tests pass | | 5 | **Auto-merge** | Web submissions only, when tests pass and `submission_prepared` label exists | -| 6 | **Post-Merge Kickoff** | After PR is merged (`pull_request_target` with `merged == true`) | +| 6 | **Post-Merge Kickoff** | After a scoring-eligible plugin PR is merged | | 7 | **Notify on Failure** | When any upstream job fails or tests don't pass | ### Job Details @@ -70,7 +69,7 @@ If metadata already exists and tests pass, adds the `submission_prepared` label. **5. Auto-merge** — Only merges **web submissions** (PR title contains `(user:)`). Non-web submissions are never auto-merged. For plugin PRs, requires `submission_prepared` label + tests passing. For metadata-only PRs, checks tests directly. Uses `hmarr/auto-approve-action` for approval and `plm9606/automerge_actions` for squash merge. -**6. Post-Merge Kickoff** — Triggered by `pull_request_target` (merged). For plugin PRs: extracts submitter email (encrypted/decrypted with `EMAIL_ENCRYPTION_KEY`), builds plugin info JSON, and calls `call_jenkins_language()` to trigger the `core/job/score_plugins` Jenkins job. For metadata-only PRs: triggers `update_existing_metadata` Jenkins job. +**6. Post-Merge Kickoff** — Triggered by `pull_request_target` (merged) for plugin PRs that need scoring. Extracts submitter email (encrypted/decrypted with `EMAIL_ENCRYPTION_KEY`), builds plugin info JSON, and calls `call_jenkins_language()` to trigger the `core/job/score_plugins` Jenkins job. Metadata-only merges do not run this job or mutate Jenkins/database state. **7. Notify on Failure** — Sends an email to the submitter when any job fails or tests don't pass. Extracts email using the same web/non-web logic as post-merge. Falls back to `mferg@mit.edu` if email lookup fails. @@ -177,8 +176,7 @@ Plugin Submission Orchestrator ├─ 3. Handle Metadata-Only PR ✓ (label already exists, exits) ├─ 5. Auto-merge ✓ (web submission, checks tests directly for metadata-only) │ └─ Squash merges -├─ 6. Post-Merge Kickoff ✓ -│ └─ Triggers update_existing_metadata Jenkins job +├─ 6. Post-Merge Kickoff (skipped — no scoring or metadata mutation) └─ 7. Notify on Failure (skipped) ``` @@ -270,9 +268,9 @@ Triggered by `call_jenkins_language()` in `endpoints.py`. Sends parameters: The function uses CSRF crumb handling (fetches crumb from `/crumbIssuer/api/json`, includes in POST headers) and basic auth. -### Metadata Update (`update_existing_metadata`) +### Metadata-only merges -Triggered by `actions_helpers.py trigger_update_existing_metadata` for metadata-only PRs. Sends `domain`, `plugin_dirs`, `plugin_type`, and serialized metadata JSON. +Metadata files are versioned in Git. A metadata-only merge does not trigger a Jenkins job or perform a separate database mutation. ### Layer Mapping (non-language domains only) diff --git a/.github/workflows/plugin_submission_orchestrator.yml b/.github/workflows/plugin_submission_orchestrator.yml index 85040232..58cf93fb 100644 --- a/.github/workflows/plugin_submission_orchestrator.yml +++ b/.github/workflows/plugin_submission_orchestrator.yml @@ -789,10 +789,8 @@ jobs: github.event_name == 'pull_request_target' && github.event.pull_request.merged == true && needs.detect_changes.result == 'success' && - ( - (needs.detect_changes.outputs.needs_scoring == 'true' && needs.detect_changes.outputs.metadata_only == 'false') || - (needs.detect_changes.outputs.metadata_only == 'true') - ) + needs.detect_changes.outputs.needs_scoring == 'true' && + needs.detect_changes.outputs.metadata_only == 'false' runs-on: ubuntu-latest env: BSC_DATABASESECRET: ${{ secrets.BSC_DATABASESECRET }} @@ -955,86 +953,6 @@ jobs: } echo "Jenkins scoring job triggered successfully" - - name: Read metadata and layer mapping files (metadata-only PRs) - if: needs.detect_changes.outputs.metadata_only == 'true' - id: read_metadata_metadata_only - run: | - PLUGIN_DIRS="${{ needs.detect_changes.outputs.plugin_dirs }}" - - # Write Python script to read metadata - cat > /tmp/read_metadata.py << 'PYTHON_SCRIPT' - import yaml - import json - import os - import sys - - plugin_dirs_str = os.environ.get('PLUGIN_DIRS', '') - metadata_dict = {} - - if plugin_dirs_str: - plugin_dirs = [d.strip() for d in plugin_dirs_str.split(',') if d.strip()] - - for plugin_dir in plugin_dirs: - if not plugin_dir: - continue - - plugin_name = os.path.basename(plugin_dir.rstrip('/')) - metadata_file = None - - # Check for metadata.yml or metadata.yaml - if os.path.isfile(os.path.join(plugin_dir, 'metadata.yml')): - metadata_file = os.path.join(plugin_dir, 'metadata.yml') - elif os.path.isfile(os.path.join(plugin_dir, 'metadata.yaml')): - metadata_file = os.path.join(plugin_dir, 'metadata.yaml') - - if metadata_file: - try: - with open(metadata_file, 'r') as f: - metadata_content = yaml.safe_load(f) - if metadata_content: - metadata_dict[plugin_name] = metadata_content - except Exception as e: - print(f'Error reading {metadata_file}: {e}', file=sys.stderr) - - # Build the final structure - result = { - 'metadata': metadata_dict, - 'layer_mapping': None # Language domain doesn't have layer mapping - } - - print(json.dumps(result)) - PYTHON_SCRIPT - - # Execute the script - export PLUGIN_DIRS - METADATA_AND_LAYER_MAP=$(python /tmp/read_metadata.py 2>/dev/null || echo '{"metadata": {}, "layer_mapping": null}') - - # Base64 encode for safe storage - METADATA_AND_LAYER_MAP_B64=$(echo "$METADATA_AND_LAYER_MAP" | base64 | tr -d '\n') - echo "metadata_and_layer_map_b64=${METADATA_AND_LAYER_MAP_B64}" >> $GITHUB_OUTPUT - echo "Read metadata for plugins: $PLUGIN_DIRS" - - - name: Trigger update_existing_metadata Jenkins job (metadata-only PRs) - if: needs.detect_changes.outputs.metadata_only == 'true' - env: - JENKINS_USER: ${{ secrets.JENKINS_USER }} - JENKINS_TOKEN: ${{ secrets.JENKINS_TOKEN }} - JENKINS_TRIGGER: ${{ secrets.JENKINS_TRIGGER }} - run: | - # Decode metadata_and_layer_map - METADATA_AND_LAYER_MAP_B64='${{ steps.read_metadata_metadata_only.outputs.metadata_and_layer_map_b64 }}' - METADATA_AND_LAYER_MAP=$(echo "$METADATA_AND_LAYER_MAP_B64" | base64 -d) - - # Base64 encode metadata_and_layer_map for passing to Python script - METADATA_B64=$(echo "$METADATA_AND_LAYER_MAP" | base64 | tr -d '\n') - - python brainscore_language/submission/actions_helpers.py trigger_update_existing_metadata \ - --plugin-dirs "${{ needs.detect_changes.outputs.plugin_dirs }}" \ - --plugin-type "${{ needs.detect_changes.outputs.plugin_type }}" \ - --domain "${{ env.DOMAIN }}" \ - --metadata-and-layer-map-b64 "$METADATA_B64" - echo "Jenkins update_existing_metadata job triggered successfully" - # ============================================================================ # JOB 7: Failure Notification (Runs if any job fails) # ============================================================================ diff --git a/SCORING_FLOW.md b/SCORING_FLOW.md index f7652105..134dec46 100644 --- a/SCORING_FLOW.md +++ b/SCORING_FLOW.md @@ -206,9 +206,9 @@ PR Created/Updated **When:** Only runs if: - PR was merged to `main` (`pull_request_target` event, `merged == true`) - For plugin PRs: `needs_scoring == true` and `metadata_only == false` -- For metadata-only PRs: `metadata_only == true` +- Metadata-only PRs skip this job -**Purpose:** Trigger Jenkins scoring (plugin PRs) or update metadata (metadata-only PRs) +**Purpose:** Trigger Jenkins scoring for plugin PRs that need it **Process:** @@ -248,10 +248,9 @@ PR Created/Updated - Jenkins job URL: `http://www.brain-score-jenkins.com:8080/job/dev_score_plugins/buildWithParameters` **For Metadata-Only PRs:** -1. **Trigger Jenkins Update Metadata:** - - Calls `actions_helpers.py trigger_update_existing_metadata` - - Sends plugin directories, plugin type, and domain to Jenkins - - Jenkins job updates existing metadata in database +1. **Preserve Repository Metadata:** + - Metadata changes remain versioned in Git + - No Jenkins job or separate database mutation is requested **Jenkins Scoring Job:** @@ -329,7 +328,6 @@ brainscore_language/submission/ **Functions:** - `validate_pr()` - Validates PR for automerge eligibility -- `trigger_update_existing_metadata()` - Triggers Jenkins update_existing_metadata job - `trigger_layer_mapping()` - Triggers Jenkins layer mapping job - `extract_email()` - Extracts user email from PR or database - `send_failure_email()` - Sends failure notification emails @@ -463,10 +461,8 @@ brainscore_language/submission/ 3. **Auto-merge job** runs: - Checks test status directly - If tests pass → auto-approves → auto-merges -4. **Post-Merge Kickoff job** runs: - - Triggers Jenkins job "update_existing_metadata" - - Jenkins updates database with new metadata -5. Post-merge scoring skipped (`metadata_only == true`) +4. **Post-Merge Kickoff job** is skipped +5. No Jenkins job or separate database mutation is requested ### Scenario 4: Web Submission @@ -606,8 +602,7 @@ Plugin Submission Orchestrator ├─ 3. Handle Metadata-Only PR (success, label exists, exits) ├─ 5. Auto-merge (success) │ └─→ Checks tests directly, merges if pass -├─ 6. Post-Merge Kickoff (success) -│ └─→ Triggers update_existing_metadata Jenkins job +├─ 6. Post-Merge Kickoff (skipped - no scoring or metadata mutation) └─ 7. Notify on Failure (skipped - no failures) ``` diff --git a/brainscore_language/submission/README.md b/brainscore_language/submission/README.md index 3418d206..aefd95a2 100644 --- a/brainscore_language/submission/README.md +++ b/brainscore_language/submission/README.md @@ -79,17 +79,6 @@ python actions_helpers.py validate_pr \ } ``` -#### `trigger_update_existing_metadata` -Triggers the Jenkins `update_existing_metadata` job for metadata-only PRs. - -```bash -python actions_helpers.py trigger_update_existing_metadata \ - --plugin-dirs "brainscore_language/models/mymodel" \ - --plugin-type "models" \ - --domain "language" \ - --metadata-and-layer-map-b64 "" -``` - #### `trigger_layer_mapping` Triggers Jenkins layer mapping job. Only used for non-language domains (language always has `needs_mapping=false`). @@ -127,7 +116,7 @@ The submission module is used at various stages of the orchestrator pipeline: 1. **Step 2 (Validate PR)** → `actions_helpers.py validate_pr` polls test statuses 2. **Step 4 (Generate Mutations)** → `hardcoded_metadata.py` generates missing metadata 3. **Step 5 (Auto-merge)** → `actions_helpers.py validate_pr` re-checks tests for metadata-only PRs -4. **Step 6 (Post-Merge)** → `endpoints.py call_jenkins_language` triggers scoring; `actions_helpers.py trigger_update_existing_metadata` for metadata-only PRs +4. **Step 6 (Post-Merge)** → `endpoints.py call_jenkins_language` triggers scoring for plugin PRs that need it; metadata-only merges perform no Jenkins or database mutation 5. **Step 7 (Notify)** → `actions_helpers.py send_failure_email` sends failure notifications ## Environment Variables diff --git a/brainscore_language/submission/actions_helpers.py b/brainscore_language/submission/actions_helpers.py index 0ad3ac64..3bdb9b8a 100644 --- a/brainscore_language/submission/actions_helpers.py +++ b/brainscore_language/submission/actions_helpers.py @@ -186,41 +186,6 @@ def validate_pr(pr_number: int, pr_head: str, is_automerge_web: bool, token: str } -def trigger_update_existing_metadata(plugin_dirs: str, plugin_type: str, domain: str, - jenkins_user: str, jenkins_token: str, jenkins_trigger: str, - metadata_and_layer_map: dict = None): - """Trigger Jenkins update_existing_metadata job""" - import json - - # Build Jenkins trigger URL - jenkins_base = "http://www.brain-score-jenkins.com:8080" - url = f"{jenkins_base}/job/update_existing_metadata/buildWithParameters?token={jenkins_trigger}" - - # Prepare payload - payload = { - "domain": domain, - "plugin_dirs": plugin_dirs, - "plugin_type": plugin_type, - "update_metadata_only": "true" - } - - # Add metadata_and_layer_map if provided (JSON-serialize nested dict) - if metadata_and_layer_map: - payload["metadata_and_layer_map"] = json.dumps(metadata_and_layer_map) - - # Trigger Jenkins - from requests.auth import HTTPBasicAuth - auth = HTTPBasicAuth(username=jenkins_user, password=jenkins_token) - - try: - response = requests.get(url, params=payload, auth=auth) - response.raise_for_status() - print(f"Successfully triggered update_existing_metadata for {plugin_type}: {plugin_dirs}") - except Exception as e: - print(f"Failed to trigger Jenkins update_existing_metadata: {e}") - raise - - def trigger_layer_mapping(new_models: str, pr_number: int, source_repo: str, source_branch: str, jenkins_user: str, jenkins_user_api: str, jenkins_token: str, jenkins_trigger: str): @@ -303,13 +268,6 @@ def main(): validate_parser.add_argument('--is-automerge-web', type=str, default='false') validate_parser.add_argument('--token', type=str, default=os.getenv('GITHUB_TOKEN')) - # Trigger update existing metadata command - update_metadata_parser = subparsers.add_parser('trigger_update_existing_metadata', help='Trigger update existing metadata') - update_metadata_parser.add_argument('--plugin-dirs', type=str, required=True) - update_metadata_parser.add_argument('--plugin-type', type=str, required=True) - update_metadata_parser.add_argument('--domain', type=str, default='language') - update_metadata_parser.add_argument('--metadata-and-layer-map-b64', type=str, default='') - # Trigger layer mapping command mapping_parser = subparsers.add_parser('trigger_layer_mapping', help='Trigger layer mapping') mapping_parser.add_argument('--new-models', type=str, required=True) @@ -386,27 +344,6 @@ def main(): result = validate_pr(args.pr_number, args.pr_head, is_automerge_web, args.token) print(json.dumps(result)) - elif args.command == 'trigger_update_existing_metadata': - # Decode metadata_and_layer_map if provided - metadata_and_layer_map = None - if args.metadata_and_layer_map_b64: - import base64 - try: - metadata_json = base64.b64decode(args.metadata_and_layer_map_b64).decode('utf-8') - metadata_and_layer_map = json.loads(metadata_json) - except Exception as e: - print(f"Warning: Failed to decode metadata_and_layer_map: {e}", file=sys.stderr) - - trigger_update_existing_metadata( - plugin_dirs=args.plugin_dirs, - plugin_type=args.plugin_type, - domain=args.domain, - metadata_and_layer_map=metadata_and_layer_map, - jenkins_user=os.getenv('JENKINS_USER'), - jenkins_token=os.getenv('JENKINS_TOKEN'), - jenkins_trigger=os.getenv('JENKINS_TRIGGER') - ) - elif args.command == 'trigger_layer_mapping': trigger_layer_mapping( new_models=args.new_models, diff --git a/tests/test_submission/test_metadata_only_workflow.py b/tests/test_submission/test_metadata_only_workflow.py new file mode 100644 index 00000000..6b18d518 --- /dev/null +++ b/tests/test_submission/test_metadata_only_workflow.py @@ -0,0 +1,34 @@ +from pathlib import Path + +REPOSITORY_ROOT = Path(__file__).resolve().parents[2] +WORKFLOW_PATH = REPOSITORY_ROOT / ".github/workflows/plugin_submission_orchestrator.yml" +HELPER_PATH = REPOSITORY_ROOT / "brainscore_language/submission/actions_helpers.py" + + +def test_metadata_update_jenkins_trigger_is_not_exposed(): + helper = HELPER_PATH.read_text() + + assert "def trigger_update_existing_metadata" not in helper + assert "'trigger_update_existing_metadata'" not in helper + + +def test_metadata_only_merges_do_not_request_jenkins_updates(): + workflow = WORKFLOW_PATH.read_text() + post_merge_condition = workflow.split(" post_merge_scoring:\n", 1)[1].split( + " runs-on:", 1 + )[0] + + assert "trigger_update_existing_metadata" not in workflow + assert "/job/update_existing_metadata" not in workflow + assert "needs.detect_changes.outputs.needs_scoring == 'true'" in post_merge_condition + assert "needs.detect_changes.outputs.metadata_only == 'false'" in post_merge_condition + assert "needs.detect_changes.outputs.metadata_only == 'true'" not in post_merge_condition + + +def test_post_merge_job_has_no_metadata_only_steps(): + workflow = WORKFLOW_PATH.read_text() + post_merge_job = workflow.split(" post_merge_scoring:\n", 1)[1].split( + " notify_on_failure:\n", 1 + )[0] + + assert "metadata_only == 'true'" not in post_merge_job