Skip to content

fix: add role parameter to BaseEvaluator#5671

Merged
mollyheamazon merged 1 commit intoaws:masterfrom
mollyheamazon:fix/evaluator
Mar 25, 2026
Merged

fix: add role parameter to BaseEvaluator#5671
mollyheamazon merged 1 commit intoaws:masterfrom
mollyheamazon:fix/evaluator

Conversation

@mollyheamazon
Copy link
Copy Markdown
Contributor

Issue #, if available:
Ticket: https://tiny.amazon.com/9v6vezon/tcorpamazP403over

Description of changes:
BaseEvaluator (and all subclasses — LLMAsJudgeEvaluator, BenchMarkEvaluator, CustomScorerEvaluator) had no way to accept an explicit IAM execution role. The role was always derived from sagemaker_session.get_caller_identity_arn(), which returns the caller's identity rather than a SageMaker-assumable service role. This caused pipeline execution failures when running outside SageMaker-managed environments (local notebooks, CI/CD, Kiro IDE).

Every other job-submitting class in the SDK (ModelTrainer, SFTTrainer, RLVRTrainer) accepts role=BaseEvaluator was the outlier.

Changes:

  • Added role: Optional[str] = None to BaseEvaluator
  • _get_aws_execution_context() uses self.role when provided, falls back to existing session-derived behavior
  • Added unit test verifying explicit role overrides session identity

Backward compatible — role=None preserves existing behavior.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@mollyheamazon mollyheamazon marked this pull request as ready for review March 24, 2026 22:21
mattliu-mygit

This comment was marked as outdated.

Copy link
Copy Markdown

@mattliu-mygit mattliu-mygit left a comment

Choose a reason for hiding this comment

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

Flake8 seems to be failing

@mollyheamazon mollyheamazon merged commit 6a0ef97 into aws:master Mar 25, 2026
14 of 20 checks passed
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.

3 participants