Skip to content

Conversation

@rpmcginty
Copy link
Collaborator

@rpmcginty rpmcginty commented Oct 23, 2025

  • update deps on core
  • upgrade aws-utils to avoid py3.9 typing issue
  • Add job_role_arn to batch job definitions and related handlers

What's in this Change?

This change utilizes the new batch job role field in demand execution's aws batch execution platform config and passes to the necessary code functions

Testing

@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.93%. Comparing base (c974350) to head (c857447).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #30      +/-   ##
==========================================
- Coverage   90.96%   90.93%   -0.04%     
==========================================
  Files          26       26              
  Lines        1428     1423       -5     
  Branches      122      122              
==========================================
- Hits         1299     1294       -5     
  Misses         98       98              
  Partials       31       31              
Files with missing lines Coverage Δ
...bs_informatics_aws_lambda/handlers/batch/create.py 83.78% <ø> (ø)
...ibs_informatics_aws_lambda/handlers/batch/model.py 100.00% <100.00%> (ø)
...tics_aws_lambda/handlers/demand/context_manager.py 96.61% <100.00%> (+0.01%) ⬆️
...bs_informatics_aws_lambda/handlers/demand/model.py 100.00% <100.00%> (ø)
...ormatics_aws_lambda/handlers/demand/scaffolding.py 93.33% <ø> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +661 to +663
job_role_arn=demand_execution.execution_platform.aws_batch.job_role
if demand_execution.execution_platform.aws_batch
else None,

Choose a reason for hiding this comment

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

P1 Badge Pass job_role_arn from demand execution to BatchJobBuilder

The new plumbing for job roles still reads the field as aws_batch.job_role. The rest of the API uses job_role_arn (e.g., CreateDefinitionAndPrepareArgsRequest.job_role_arn and the new handler signatures), so this lookup will either raise an AttributeError or silently return None, causing the Batch job definition to be registered without the caller’s role even when one was supplied. When a demand execution contains a role ARN, this code path will drop it and the job will execute with the default role instead.

Useful? React with 👍 / 👎.

@rpmcginty rpmcginty requested a review from njmei October 23, 2025 21:09
@rpmcginty rpmcginty merged commit 0b0b502 into main Oct 27, 2025
8 checks passed
@rpmcginty rpmcginty deleted the feature/DT-8749-add-support-for-job-def-role branch October 27, 2025 22:28
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