Skip to content

fix: apply user-defined aws_batch_tags regardless of BATCH_EMIT_TAGS#3210

Open
odncode wants to merge 4 commits into
Netflix:masterfrom
odncode:fix-batch-tags-always-emit-user-tags
Open

fix: apply user-defined aws_batch_tags regardless of BATCH_EMIT_TAGS#3210
odncode wants to merge 4 commits into
Netflix:masterfrom
odncode:fix-batch-tags-always-emit-user-tags

Conversation

@odncode

@odncode odncode commented May 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #3209

User-defined aws_batch_tags and METAFLOW_BATCH_DEFAULT_TAGS are currently dropped silently when BATCH_EMIT_TAGS is not set. This PR ensures that user-defined tags are always respected and applied to AWS Batch jobs, regardless of internal telemetry settings.


Problem

The aws_batch_tags block in batch.py (L401-403) sits inside the if BATCH_EMIT_TAGS: guard (L380).

While BATCH_EMIT_TAGS is intended to toggle Metaflow's internal observability tags (such as app=metaflow or metaflow.flow_name), it inadvertently gates user-defined tags meant for cost attribution or compliance.

In enterprise environments with mandatory AWS Service Control Policies (SCPs) or AWS Config rules requiring specific tags, this causes Batch job submissions to fail with access-denied errors unless BATCH_EMIT_TAGS is enabled.


Fix

  • Decoupled User Tags: Moved the aws_batch_tags processing block outside of the if BATCH_EMIT_TAGS: guard.
  • Preserved Telemetry Rules: Internal Metaflow tags remain gated; user-defined tags are now always applied.

Testing

  • Added test/unit/test_batch_user_tags.py — an AST-based structural test that parses create_job and verifies aws_batch_tags is not referenced inside any if BATCH_EMIT_TAGS: block.
  • Results: Verified that the test PASSES with this fix, and FAILS without it.

PR Checklist

  • Link tracking issue (Fixes #3209)
  • Add/update unit tests
  • Update documentation (N/A)

Closes #3209

User-defined tags (aws_batch_tags and METAFLOW_BATCH_DEFAULT_TAGS) were
trapped inside the 'if BATCH_EMIT_TAGS:' guard, which controls Metaflow's
internal observability tags. This caused user tags to be silently dropped
when BATCH_EMIT_TAGS was not set.

In enterprise deployments with mandatory AWS tagging policies (SCPs),
this results in Batch job submission failures with no indication that
the root cause is a missing Metaflow config flag.

Fix: move the aws_batch_tags block outside the BATCH_EMIT_TAGS guard.
Internal tags remain gated; user-defined tags are always applied.

Includes an AST-based regression test that verifies aws_batch_tags
is not referenced inside any BATCH_EMIT_TAGS conditional block.

Closes Netflix#3209
@greptile-apps

greptile-apps Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR decouples user-defined aws_batch_tags from the BATCH_EMIT_TAGS guard so that cost-attribution and compliance tags are always submitted to AWS Batch, fixing silent tag drops that caused SCP/Config-rule-enforced job rejections.

  • batch.py: The aws_batch_tags loop is moved four indentation levels out, placing it after the if BATCH_EMIT_TAGS: block. Internal Metaflow observability tags (app=metaflow, metaflow.flow_name, etc.) remain gated as before.
  • test_batch_user_tags.py: Two new AST-based regression tests verify that aws_batch_tags is referenced in create_job and is not nested inside any if BATCH_EMIT_TAGS: block.

Confidence Score: 5/5

Safe to merge — the change is a minimal, targeted indentation shift with a clear mechanical justification and a new regression test.

The production change is a single block moved out of a conditional; the logic for building and iterating over aws_batch_tags is unchanged. The decorator already normalises the value to a dict before create_job is called, so the guard if aws_batch_tags is not None: still behaves correctly. No new code paths, data mutations, or API calls are introduced.

No files require special attention.

Important Files Changed

Filename Overview
metaflow/plugins/aws/batch/batch.py Moves aws_batch_tags application outside the if BATCH_EMIT_TAGS: guard so user-defined tags are always sent to AWS Batch, regardless of the internal telemetry toggle. Also removes a now-orphaned inline comment that preceded the sanitise loop.
test/unit/test_batch_user_tags.py Adds two AST-based regression tests: one confirming aws_batch_tags is not nested inside any if BATCH_EMIT_TAGS: block, and one confirming it is referenced somewhere in create_job. Has an unused import pytest.

Reviews (4): Last reviewed commit: "style: add missing EOF newline (pre-comm..." | Re-trigger Greptile

Comment thread test/unit/test_batch_user_tags.py
Comment thread test/unit/test_batch_user_tags.py Outdated
Addresses review feedback: previous test only checked the negative
(tags not inside guard) without verifying they exist at all. New test
ensures aws_batch_tags is referenced in create_job, guarding against
silent removal of the feature.
@codecov

codecov Bot commented Jun 5, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@6cc3431). Learn more about missing BASE report.

Files with missing lines Patch % Lines
metaflow/plugins/aws/batch/batch.py 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##             master    #3210   +/-   ##
=========================================
  Coverage          ?   28.85%           
=========================================
  Files             ?      381           
  Lines             ?    52467           
  Branches          ?     9260           
=========================================
  Hits              ?    15138           
  Misses            ?    36315           
  Partials          ?     1014           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: odncode <nnajiodera2@gmail.com>
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.

aws_batch_tags / METAFLOW_BATCH_DEFAULT_TAGS silently ignored unless METAFLOW_BATCH_EMIT_TAGS=true

1 participant