Skip to content

fix(train): correct Networking field names in ModelTrainer intelligen…#6064

Open
mohamedzeidan2021 wants to merge 2 commits into
aws:masterfrom
mohamedzeidan2021:fix/networking-intelligent-defaults-with-tests
Open

fix(train): correct Networking field names in ModelTrainer intelligen…#6064
mohamedzeidan2021 wants to merge 2 commits into
aws:masterfrom
mohamedzeidan2021:fix/networking-intelligent-defaults-with-tests

Conversation

@mohamedzeidan2021

Copy link
Copy Markdown
Collaborator

…t defaults

Fixes two bugs in _populate_intelligent_defaults_from_training_job_space() in model_trainer.py that prevent ModelTrainer from working when sagemaker_config has a VpcConfig set.

Bug 1: Networking() was passed default_enable_network_isolation, which is not a field in sagemaker-core 2.x. The correct field is enable_network_isolation. This raised a pydantic ValidationError on every train() call when VPC config was present.

Bug 2: When updating an existing Networking object, security_group_ids was mistakenly assigned the subnets value from TRAINING_JOB_SUBNETS_PATH instead of security_group_ids from TRAINING_JOB_SECURITY_GROUP_IDS_PATH.

Adds unit tests covering both bug paths (fresh Networking construction and updating an existing Networking object).

Fix originally from #5866; unit tests added here.

Issue #, if available:

Description of changes:

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

…t defaults

Fixes two bugs in _populate_intelligent_defaults_from_training_job_space()
in model_trainer.py that prevent ModelTrainer from working when
sagemaker_config has a VpcConfig set.

Bug 1: Networking() was passed default_enable_network_isolation, which is
not a field in sagemaker-core 2.x. The correct field is
enable_network_isolation. This raised a pydantic ValidationError on every
train() call when VPC config was present.

Bug 2: When updating an existing Networking object, security_group_ids was
mistakenly assigned the subnets value from TRAINING_JOB_SUBNETS_PATH instead
of security_group_ids from TRAINING_JOB_SECURITY_GROUP_IDS_PATH.

Adds unit tests covering both bug paths (fresh Networking construction and
updating an existing Networking object).

Fix originally from aws#5866; unit tests added here.
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