Fix: Bug fixes for s3 path validation, mlflow app creation#5402
Merged
Conversation
rsareddy0329
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
jam-jee
reviewed
Dec 10, 2025
| error_msg = f"MLflow app creation failed. Current status: {new_app.status}" | ||
| if hasattr(new_app, 'failure_reason') and new_app.failure_reason: | ||
| error_msg += f". Reason: {new_app.failure_reason}" | ||
| raise RuntimeError(error_msg) |
Collaborator
There was a problem hiding this comment.
Do we need to raise error in case of timeout exception or should just display error message ? If MLFlow app is required parameter for CTJ API then it's fine to throw error.
jam-jee
reviewed
Dec 10, 2025
| if "NoSuchBucket" in str(e) or "Not Found" in str(e): | ||
| # Create bucket | ||
| region = sagemaker_session.boto_region_name | ||
| if region == 'us-east-1': |
Collaborator
There was a problem hiding this comment.
why this region specific check here ?
Member
There was a problem hiding this comment.
This is expected for s3
rsareddy0329
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
rsareddy0329
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
…ianers to maintain consistency
rsareddy0329
temporarily deployed
to
auto-approve
GitHub Actions
Inactive
mufaddal-rohawala
approved these changes
Dec 10, 2025
| if "NoSuchBucket" in str(e) or "Not Found" in str(e): | ||
| # Create bucket | ||
| region = sagemaker_session.boto_region_name | ||
| if region == 'us-east-1': |
Member
There was a problem hiding this comment.
This is expected for s3
Evan-W-ang
added a commit
to Evan-W-ang/sagemaker-python-sdk
that referenced
this pull request
Jun 8, 2026
* fix: Fix the recipe selection for multiple recipe scenario * fix: Fix the recipe selection for multiple recipe scenario * fix: Hyperparameter issue fixes, validate s3 output path,additional unit tests * Fix: Add validation to bedrock reward models * Fix: Add validation to bedrock reward models * Fix: Add allow list for bedrock eval models * Fix: Add allow list for bedrock eval models * Fix: Bug fixes for s3 path validation, mlflow app creation * Fix: Update Legal verbiage, and allowed reward model ids based on region * Fix: Update model_package_group_name to model_package_group in all trianers to maintain consistency --------- Co-authored-by: Roja Reddy Sareddy <rsareddy@amazon.com>
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
3 participants
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.
Issue #, if available:
Description of changes:
Testing:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.