Skip to content

Commit 15e650b

Browse files
wasim-buildswasim-builds
authored
Merge branch 'master' into fix/issue-5897-crlf-sm-train
2 parents f28b12c + 8984bca commit 15e650b

38 files changed

Lines changed: 3342 additions & 421 deletions

‎.github/workflows/ai-code-review.yml‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
name: AI Code Review (Claude on Bedrock)
2+
3+
# Automated PR review using Anthropic's Claude Code Action running on Amazon
4+
# Bedrock (inference stays in-account, CloudTrail-audited, no external API key).
5+
#
6+
# Fork safety: this uses pull_request_target (needed to access the Bedrock role
7+
# and post comments), so it reuses the same collaborator gate as
8+
# pr-checks-master.yml. Collaborator PRs -> `auto-approve` env (runs immediately).
9+
# Fork/external PRs -> `manual-approval` env (a maintainer must approve the run
10+
# before any secret or the Bedrock role is exposed).
11+
12+
on:
13+
pull_request_target:
14+
types: [opened, synchronize, ready_for_review, reopened]
15+
branches:
16+
- "master"
17+
# Scope to the same product directories the other PR checks use, so pure
18+
# docs/example PRs don't trigger a model review. Remove this block to
19+
# review every PR.
20+
paths:
21+
- 'sagemaker-train/**'
22+
- 'sagemaker-serve/**'
23+
- 'sagemaker-mlops/**'
24+
- 'sagemaker-core/**'
25+
26+
concurrency:
27+
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.head_ref }}
28+
cancel-in-progress: true
29+
30+
permissions:
31+
id-token: write # OIDC federation to assume the Bedrock role
32+
pull-requests: write # post inline review comments and a summary
33+
contents: read
34+
35+
jobs:
36+
# Identical gate to pr-checks-master.yml: collaborators auto-approve,
37+
# everyone else requires manual approval via the `manual-approval` environment.
38+
collab-check:
39+
runs-on: ubuntu-latest
40+
outputs:
41+
approval-env: ${{ steps.collab-check.outputs.result }}
42+
steps:
43+
- name: Collaborator Check
44+
uses: actions/github-script@v7
45+
id: collab-check
46+
with:
47+
github-token: ${{ secrets.COLLAB_CHECK_TOKEN }}
48+
result-encoding: string
49+
script: |
50+
try {
51+
const res = await github.rest.repos.checkCollaborator({
52+
owner: context.repo.owner,
53+
repo: context.repo.repo,
54+
username: "${{ github.event.pull_request.user.login }}",
55+
});
56+
console.log("Verified ${{ github.event.pull_request.user.login }} is a repo collaborator. Auto approving AI review.")
57+
return res.status == "204" ? "auto-approve" : "manual-approval"
58+
} catch (error) {
59+
console.log("${{ github.event.pull_request.user.login }} is not a collaborator. Requiring manual approval to run AI review.")
60+
return "manual-approval"
61+
}
62+
63+
wait-for-approval:
64+
runs-on: ubuntu-latest
65+
needs: [collab-check]
66+
environment: ${{ needs.collab-check.outputs.approval-env }}
67+
steps:
68+
- run: echo "Approved — starting AI code review."
69+
70+
review:
71+
runs-on: ubuntu-latest
72+
needs: [wait-for-approval]
73+
steps:
74+
# SECURITY: this job runs in the trusted pull_request_target context (it
75+
# holds the Bedrock role + secrets). We therefore check out the BASE repo
76+
# at the PR's base branch — never the fork's head code — so untrusted PR
77+
# code is never executed here ("pwn request" prevention). The PR contents
78+
# are pulled in as a read-only diff below, not as an executable tree.
79+
- uses: actions/checkout@v4
80+
with:
81+
ref: ${{ github.event.pull_request.base.sha }}
82+
fetch-depth: 1
83+
84+
# Fetch the PR diff via the API (does not execute any fork code) and store
85+
# it as a static file. This is the ground truth Claude reviews.
86+
- name: Fetch PR diff
87+
id: diff
88+
env:
89+
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
90+
PR_NUMBER: ${{ github.event.pull_request.number }}
91+
REPO: ${{ github.repository }}
92+
run: |
93+
set -euo pipefail
94+
gh api "repos/$REPO/pulls/$PR_NUMBER" \
95+
-H "Accept: application/vnd.github.v3.diff" > /tmp/pr.diff
96+
BYTES=$(wc -c < /tmp/pr.diff)
97+
echo "bytes=$BYTES" >> "$GITHUB_OUTPUT"
98+
echo "PR diff: $BYTES bytes"
99+
100+
- name: Configure AWS Credentials
101+
if: steps.diff.outputs.bytes != '0'
102+
uses: aws-actions/configure-aws-credentials@v4
103+
with:
104+
role-to-assume: ${{ secrets.CODE_REVIEW_ROLE }}
105+
aws-region: us-west-2
106+
107+
- uses: anthropics/claude-code-action@v1
108+
if: steps.diff.outputs.bytes != '0'
109+
with:
110+
github_token: ${{ secrets.GITHUB_TOKEN }}
111+
use_bedrock: "true"
112+
track_progress: true
113+
# Don't append "Fix this" deep-links (which open Claude Code) to review
114+
# comments — external contributors can't use them and they add noise.
115+
include_fix_links: false
116+
# Bash is intentionally NOT allowed. The PR diff at /tmp/pr.diff is the
117+
# only ground truth; the model reads it and uses Read/Grep/Glob against
118+
# the trusted base checkout for context. It must not execute commands
119+
# (which could run untrusted PR content) nor re-run git.
120+
claude_args: |
121+
--model us.anthropic.claude-opus-4-8
122+
--allowedTools "Read Grep Glob mcp__github_inline_comment__create_inline_comment"
123+
prompt: |
124+
REPO: ${{ github.repository }}
125+
PR NUMBER: ${{ github.event.pull_request.number }}
126+
127+
The complete PR diff is at `/tmp/pr.diff` — read it first with the
128+
Read tool. That file is the ground truth for what this PR changes; do
129+
not run git or any shell commands. For context (callers of changed
130+
functions, existing patterns, project conventions), use Read/Grep/Glob
131+
against the checked-out base repository.
132+
133+
Review this pull request for the SageMaker Python SDK. Focus on:
134+
- Correctness: bugs, incorrect API/argument usage, breaking changes
135+
to public interfaces, backward-incompatibility for SDK consumers
136+
- Python best practices and readability
137+
- Security implications (credential handling, input validation)
138+
- Performance considerations
139+
- Missing or inadequate tests for changed behavior
140+
141+
Post specific issues as inline comments via the
142+
mcp__github_inline_comment__create_inline_comment tool. Skip nits and
143+
style the linters already enforce. If the PR looks clean, say so
144+
briefly.

.github/workflows/ci-health.yml renamed to .github/workflows/ci-health-v2.yml

Lines changed: 11 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
1-
name: CI Health
1+
name: CI Health V2
22
on:
33
schedule:
4-
- cron: "0 */3 * * *"
4+
# Once a day at 4:00 UTC (8pm PDT / 9pm PST).
5+
- cron: "0 4 * * *"
56
workflow_dispatch:
6-
7+
78
permissions:
89
id-token: write # This is required for requesting the JWT
910

1011
jobs:
11-
canaries-v3:
12+
canaries-v2-master:
1213
runs-on: ubuntu-latest
1314
steps:
1415
- name: Configure AWS Credentials
@@ -17,12 +18,12 @@ jobs:
1718
role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
1819
aws-region: us-west-2
1920
role-duration-seconds: 10800
20-
- name: Run Canaries V3
21+
- name: Run Canaries V2 (master)
2122
uses: aws-actions/aws-codebuild-run-build@v1
2223
with:
23-
project-name: sagemaker-python-sdk-ci-health-canaries-v3
24-
source-version-override: refs/heads/master
25-
canaries-v2:
24+
project-name: sagemaker-python-sdk-ci-health-canaries-v2-master
25+
source-version-override: refs/heads/master-v2
26+
canaries-v2-release:
2627
runs-on: ubuntu-latest
2728
steps:
2829
- name: Configure AWS Credentials
@@ -31,10 +32,10 @@ jobs:
3132
role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
3233
aws-region: us-west-2
3334
role-duration-seconds: 10800
34-
- name: Run Canaries V2
35+
- name: Run Canaries V2 (release)
3536
uses: aws-actions/aws-codebuild-run-build@v1
3637
with:
37-
project-name: sagemaker-python-sdk-ci-health-canaries-v2
38+
project-name: sagemaker-python-sdk-ci-health-canaries-v2-release
3839
source-version-override: refs/heads/master-v2
3940
unit-tests-v2:
4041
runs-on: ubuntu-latest
@@ -86,25 +87,3 @@ jobs:
8687
with:
8788
project-name: sagemaker-python-sdk-ci-health-localmode-tests
8889
source-version-override: refs/heads/master-v2
89-
unit-test-v3:
90-
runs-on: ubuntu-latest
91-
strategy:
92-
fail-fast: false
93-
matrix:
94-
submodule: [sagemaker-core, sagemaker-train, sagemaker-serve, sagemaker-mlops]
95-
steps:
96-
- name: Configure AWS Credentials
97-
uses: aws-actions/configure-aws-credentials@v4
98-
with:
99-
role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
100-
aws-region: us-west-2
101-
role-duration-seconds: 10800
102-
- name: Run Unit Tests V3 for ${{ matrix.submodule }}
103-
uses: aws-actions/aws-codebuild-run-build@v1
104-
with:
105-
project-name: sagemaker-python-sdk-ci-health-unit-test-v3
106-
source-version-override: refs/heads/master
107-
env-vars-for-codebuild: |
108-
SUBMODULE
109-
env:
110-
SUBMODULE: ${{ matrix.submodule }}

‎.github/workflows/ci-health-v3-master.yml‎

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
name: CI Health V3 Master
2+
on:
3+
schedule:
4+
# Once a day at 07:00 UTC (11pm PDT / 12pm PST).
5+
- cron: "0 7 * * *"
6+
workflow_dispatch:
7+
8+
permissions:
9+
id-token: write # This is required for requesting the JWT
10+
11+
jobs:
12+
unit-test-v3:
13+
runs-on: ubuntu-latest
14+
strategy:
15+
fail-fast: false
16+
matrix:
17+
submodule: [sagemaker-core, sagemaker-train, sagemaker-serve, sagemaker-mlops]
18+
steps:
19+
- name: Configure AWS Credentials
20+
uses: aws-actions/configure-aws-credentials@v4
21+
with:
22+
role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
23+
aws-region: us-west-2
24+
role-duration-seconds: 10800
25+
- name: Run Unit Tests V3 for ${{ matrix.submodule }}
26+
uses: aws-actions/aws-codebuild-run-build@v1
27+
with:
28+
project-name: sagemaker-python-sdk-ci-health-unit-test-v3
29+
source-version-override: refs/heads/master
30+
env-vars-for-codebuild: |
31+
SUBMODULE
32+
env:
33+
SUBMODULE: ${{ matrix.submodule }}
34+
canaries-v3-master:
35+
runs-on: ubuntu-latest
36+
strategy:
37+
fail-fast: false
38+
matrix:
39+
submodule: [sagemaker-core, sagemaker-train, sagemaker-serve, sagemaker-mlops]
40+
steps:
41+
- name: Configure AWS Credentials
42+
uses: aws-actions/configure-aws-credentials@v4
43+
with:
44+
role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
45+
aws-region: us-west-2
46+
role-duration-seconds: 10800
47+
- name: Run Canaries V3 master for ${{ matrix.submodule }}
48+
uses: aws-actions/aws-codebuild-run-build@v1
49+
with:
50+
project-name: sagemaker-python-sdk-ci-health-canaries-v3-master
51+
source-version-override: refs/heads/master
52+
env-vars-for-codebuild: |
53+
SUBMODULE
54+
env:
55+
SUBMODULE: ${{ matrix.submodule }}
56+
# GPU integ tests against the master source. Two region jobs trigger the same
57+
# CodeBuild project name (deployed in both regions); only the region/creds and
58+
# the region-override buildspec differ. Their results are AND-ed with
59+
# import-model in report-result into a single per-run pass/fail metric.
60+
gpu-integ-tests-master:
61+
runs-on: ubuntu-latest
62+
steps:
63+
- name: Configure AWS Credentials
64+
uses: aws-actions/configure-aws-credentials@v4
65+
with:
66+
role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
67+
aws-region: us-west-2
68+
role-duration-seconds: 10800
69+
- name: Run GPU Integ Tests (master)
70+
uses: aws-actions/aws-codebuild-run-build@v1
71+
with:
72+
project-name: sagemaker-python-sdk-ci-health-gpu-integ-tests-master
73+
source-version: refs/heads/master
74+
gpu-integ-tests-master-us-east-1:
75+
runs-on: ubuntu-latest
76+
steps:
77+
- name: Configure AWS Credentials (us-east-1)
78+
uses: aws-actions/configure-aws-credentials@v4
79+
with:
80+
role-to-assume: ${{ secrets.CI_AWS_ROLE_US_EAST_1_ARN }}
81+
aws-region: us-east-1
82+
role-duration-seconds: 10800
83+
- name: Run GPU Integ Tests (master, us-east-1)
84+
uses: aws-actions/aws-codebuild-run-build@v1
85+
with:
86+
project-name: sagemaker-python-sdk-ci-health-gpu-integ-tests-master
87+
source-version: refs/heads/master
88+
# Bedrock model-import integ tests. Run serially (concurrency 1) in their own
89+
# CodeBuild project because the "Concurrent model import jobs" Bedrock quota is
90+
# fixed at 1 and not raisable; running them in parallel (as PR checks did)
91+
# makes them collide and flake. us-west-2 only (no us_east_1-marked tests).
92+
# Folded into the master run-level pass/fail metric alongside the GPU jobs.
93+
import-model-integ-tests:
94+
runs-on: ubuntu-latest
95+
steps:
96+
- name: Configure AWS Credentials
97+
uses: aws-actions/configure-aws-credentials@v4
98+
with:
99+
role-to-assume: ${{ secrets.CI_AWS_ROLE_ARN }}
100+
aws-region: us-west-2
101+
role-duration-seconds: 10800
102+
- name: Run Bedrock Model-Import Integ Tests
103+
uses: aws-actions/aws-codebuild-run-build@v1
104+
with:
105+
project-name: sagemaker-python-sdk-ci-health-import-model-integ-tests
106+
source-version: refs/heads/master
107+
# Run-level result: the master GPU run is successful only if the us-west-2,
108+
# us-east-1, and import-model jobs all succeeded. Emits GpuIntegMasterRunFailure
109+
# = 1 (failed) / 0 (succeeded) to CloudWatch in us-west-2. The CDK alarm
110+
# (GpuIntegMasterRunAlarm) sums this over a UTC day; it is folded into
111+
# CIHealthCompositeAlarm-V3, which owns ticketing. Only scheduled runs emit the
112+
# metric (manual workflow_dispatch runs are excluded).
113+
report-result:
114+
needs: [gpu-integ-tests-master, gpu-integ-tests-master-us-east-1, import-model-integ-tests]
115+
if: always()
116+
runs-on: ubuntu-latest
117+
steps:
118+
- name: Configure AWS Credentials
119+
uses: aws-actions/configure-aws-credentials@v4
120+
with:
121+
role-to-assume: ${{ secrets.MONITORING_ROLE_ARN }}
122+
aws-region: us-west-2
123+
- name: Emit run-level pass/fail metric
124+
run: |
125+
# Manual (workflow_dispatch) runs must not contribute to the daily
126+
# GpuIntegMasterRunFailure count that drives GpuIntegMasterRunAlarm;
127+
# only scheduled runs count.
128+
if [ "${{ github.event_name }}" != "schedule" ]; then
129+
echo "Not a scheduled run (${{ github.event_name }}); skipping metric emission."
130+
exit 0
131+
fi
132+
if [ "${{ needs.gpu-integ-tests-master.result }}" == "success" ] && \
133+
[ "${{ needs.gpu-integ-tests-master-us-east-1.result }}" == "success" ] && \
134+
[ "${{ needs.import-model-integ-tests.result }}" == "success" ]; then
135+
value=0
136+
echo "All master GPU/import jobs succeeded; emitting GpuIntegMasterRunFailure=0"
137+
else
138+
value=1
139+
echo "At least one master GPU/import job did not succeed; emitting GpuIntegMasterRunFailure=1"
140+
fi
141+
aws cloudwatch put-metric-data \
142+
--namespace GpuIntegRunMetrics \
143+
--metric-name GpuIntegMasterRunFailure \
144+
--value "$value" \
145+
--unit Count

0 commit comments

Comments
 (0)