fix(integ): remove module-level env var pollution from MTRL tests#5929
Open
lucasjia-aws wants to merge 1 commit into
Open
fix(integ): remove module-level env var pollution from MTRL tests#5929lucasjia-aws wants to merge 1 commit into
lucasjia-aws wants to merge 1 commit into
Conversation
The 4 MTRL test files added in aws#5919 set SAGEMAKER_REGION=us-west-2 at module level via os.environ.setdefault(). When pytest collects tests, it imports all modules in the directory—including these files—even when filtering by mark (e.g. -m "gpu_intensive and us_east_1"). This poisons the SageMakerClient singleton's region for the entire process, causing the Nova us-east-1 integ tests (SFT, RLVR, BenchmarkEvaluator) to fail with "ModelPackageGroup does not exist" because API calls land in us-west-2 instead of us-east-1. Changes: - Remove os.environ.setdefault() calls from module level in all 4 files - Move _ACCOUNT_ID resolution from module-level boto3 calls into lazy fixtures/functions (avoids import-time side effects) - Use boto3.Session().client() pattern instead of boto3.client() to follow SDK conventions (wrapper over boto, not direct boto usage) - Align with existing test patterns (DPO, SFT, RLVR) that rely on conftest fixtures for session management Affected files: - test_mtrl_evaluator.py - test_mtrl_evaluator_3p_agent.py - test_mtrl_trainer_integration.py - test_multi_turn_rl_trainer_integration.py
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The 4 MTRL test files added in #5919 set SAGEMAKER_REGION=us-west-2 at module level via os.environ.setdefault(). When pytest collects tests, it imports all modules in the directory—including these files—even when filtering by mark (e.g. -m "gpu_intensive and us_east_1"). This poisons the SageMakerClient singleton's region for the entire process, causing the Nova us-east-1 integ tests (SFT, RLVR, BenchmarkEvaluator) to fail with "ModelPackageGroup does not exist" because API calls land in us-west-2 instead of us-east-1.
Changes:
Affected files:
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.