test: add integ tests for show_metrics and model reuse (reuse_resourc… - #6158
Merged
mujtaba1747 merged 1 commit intoAug 5, 2026
Conversation
amazeAmazing
marked this pull request as ready for review
August 5, 2026 19:44
mujtaba1747
reviewed
Aug 5, 2026
| endpoint2 = builder2.deploy(reuse_resources=True) | ||
|
|
||
| assert endpoint2 is not None | ||
| assert endpoint2.endpoint_arn == endpoint.endpoint_arn, ( |
mujtaba1747
reviewed
Aug 5, 2026
| role=config["role"], | ||
| accept_eula=True, | ||
| ) | ||
| trainer._latest_job = job |
Collaborator
There was a problem hiding this comment.
Can we also call stream_logs() here. Not sure how we can validation stream_logs() output. But at least calling it in the test will ensure no exception was thrown.
mujtaba1747
reviewed
Aug 5, 2026
| assert training_job.output_model_package_arn is not None | ||
|
|
||
| # Verify show_metrics() works via MLflow path for OSS models | ||
| result = sft_trainer.show_metrics() |
Collaborator
There was a problem hiding this comment.
Same, can we also add stream_logs() below show_metrics()
mujtaba1747
reviewed
Aug 5, 2026
| import matplotlib | ||
| matplotlib.use("Agg") | ||
|
|
||
| df = sft_trainer.show_metrics() |
Collaborator
There was a problem hiding this comment.
Can we add stream logs here too?
mujtaba1747
reviewed
Aug 5, 2026
| First build creates a Model, second build with reuse finds it. | ||
| """ | ||
| training_job = TrainingJob.get(training_job_name=training_job_name, region=AWS_REGION) | ||
| unique_id = f"{int(time.time())}-{random.randint(100, 10000)}" |
Collaborator
There was a problem hiding this comment.
There is a super small chance of race condition here. If 2 codebuild test runs end up having the same unique_id. Might want to call this out as a code comment.
cmahima
reviewed
Aug 5, 2026
| # Verify only one model exists with our unique prefix | ||
| sm_client = boto3.client("sagemaker", region_name=AWS_REGION) | ||
| models = sm_client.list_models(NameContains=f"reuse-build-{unique_id}", MaxResults=10).get("Models", []) | ||
| assert len(models) == 1, ( |
Contributor
There was a problem hiding this comment.
Should we assert model2.model_arn == model1.model_arn instead?
| NameContains=f"nova-reuse-build-{unique_id}", | ||
| MaxResults=10, | ||
| ).get("Models", []) | ||
| assert len(models) == 1, ( |
Contributor
There was a problem hiding this comment.
Should we assert model2.model_arn == model1.model_arn instead?
amazeAmazing
force-pushed
the
integ-tests-metrics-notifications-reuse
branch
from
August 5, 2026 21:09
1abebcf to
e0ac4e3
Compare
…es=True) - Add show_metrics() assertions to existing Nova SFT tests (serverless + serverful) - Add show_metrics() via MLflow to OSS Llama SFT test - Add show_metrics() to MTRL trainer integration test - Add reuse round-trip tests to test_model_customization_deployment.py (OSS, us-west-2) - Add reuse round-trip tests to test_nova_model_customization_deployment.py (Nova, us-east-1) - Add build(reuse_resources=True) tests to both deployment test files
amazeAmazing
force-pushed
the
integ-tests-metrics-notifications-reuse
branch
from
August 5, 2026 21:31
e0ac4e3 to
c4f3737
Compare
mujtaba1747
approved these changes
Aug 5, 2026
cmahima
approved these changes
Aug 5, 2026
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.
…es=True)
Issue #, if available:
Description of changes:
Adds integration test coverage for:
show_metrics()
global_step, metric filter, and step range filter — added totest_sft_trainer_integration.py(serverless) andtest_sft_trainer_serverful_smtj.py(serverful)show_metrics()executes without error after training completes — added totest_sft_trainer_integration.pyshow_metrics()on completed MTRL job — added totest_mtrl_trainer_integration.pyreuse_resources=True
reuse_resources=Truereturns the same endpoint ARN — added totest_nova_model_customization_deployment.pyandtest_model_customization_deployment.pybuild(reuse_resources=True)doesn't create a duplicate Model resource — validated inline with the deploy testdeploy(reuse_resources=True)finds existing tagged custom model — added totest_nova_model_customization_deployment.pyCleanup:
import timestatementsTested in: a different account (us-east-1) via equivalent tests for OSS and Nova models.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.