-
Notifications
You must be signed in to change notification settings - Fork 9.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
F/Add model_package_name for aws_sagemaker_model #31755
F/Add model_package_name for aws_sagemaker_model #31755
Conversation
Community NoteVoting for Prioritization
For Submitters
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Welcome @holly-evans 👋
It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTOR guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.
Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.
Thanks again, and welcome to the community! 😃
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting in the work so far @holly-evans, if you could kindly update the test we are close to getting this done :)
I'll make sure to run the tests for you, if you seem to bump into anything I'm happy to help with updating the test.
Co-authored-by: Bruno Schaatsbergen <[email protected]>
…t we have a subscription for.
…orkIsolation must be set to true for using a product from AWS Marketplace'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccSageMakerModel_' PKG=sagemaker ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/sagemaker/... -v -count 1 -parallel 2 -run=TestAccSageMakerModel_ -timeout 180m
=== RUN TestAccSageMakerModel_basic
=== PAUSE TestAccSageMakerModel_basic
=== RUN TestAccSageMakerModel_inferenceExecution
=== PAUSE TestAccSageMakerModel_inferenceExecution
=== RUN TestAccSageMakerModel_tags
=== PAUSE TestAccSageMakerModel_tags
=== RUN TestAccSageMakerModel_primaryContainerModelDataURL
=== PAUSE TestAccSageMakerModel_primaryContainerModelDataURL
=== RUN TestAccSageMakerModel_primaryContainerHostname
=== PAUSE TestAccSageMakerModel_primaryContainerHostname
=== RUN TestAccSageMakerModel_primaryContainerImage
=== PAUSE TestAccSageMakerModel_primaryContainerImage
=== RUN TestAccSageMakerModel_primaryContainerEnvironment
=== PAUSE TestAccSageMakerModel_primaryContainerEnvironment
=== RUN TestAccSageMakerModel_primaryContainerModeSingle
=== PAUSE TestAccSageMakerModel_primaryContainerModeSingle
=== RUN TestAccSageMakerModel_primaryContainerModelPackageName
=== PAUSE TestAccSageMakerModel_primaryContainerModelPackageName
=== RUN TestAccSageMakerModel_containers
=== PAUSE TestAccSageMakerModel_containers
=== RUN TestAccSageMakerModel_vpc
=== PAUSE TestAccSageMakerModel_vpc
=== RUN TestAccSageMakerModel_primaryContainerPrivateDockerRegistry
=== PAUSE TestAccSageMakerModel_primaryContainerPrivateDockerRegistry
=== RUN TestAccSageMakerModel_networkIsolation
=== PAUSE TestAccSageMakerModel_networkIsolation
=== RUN TestAccSageMakerModel_disappears
=== PAUSE TestAccSageMakerModel_disappears
=== CONT TestAccSageMakerModel_basic
=== CONT TestAccSageMakerModel_primaryContainerModeSingle
--- PASS: TestAccSageMakerModel_primaryContainerModeSingle (36.49s)
=== CONT TestAccSageMakerModel_primaryContainerHostname
--- PASS: TestAccSageMakerModel_basic (37.86s)
=== CONT TestAccSageMakerModel_primaryContainerEnvironment
--- PASS: TestAccSageMakerModel_primaryContainerHostname (38.34s)
=== CONT TestAccSageMakerModel_primaryContainerImage
--- PASS: TestAccSageMakerModel_primaryContainerEnvironment (44.24s)
=== CONT TestAccSageMakerModel_tags
--- PASS: TestAccSageMakerModel_primaryContainerImage (45.37s)
=== CONT TestAccSageMakerModel_primaryContainerModelDataURL
--- PASS: TestAccSageMakerModel_tags (70.62s)
=== CONT TestAccSageMakerModel_primaryContainerPrivateDockerRegistry
--- PASS: TestAccSageMakerModel_primaryContainerModelDataURL (40.63s)
=== CONT TestAccSageMakerModel_disappears
--- PASS: TestAccSageMakerModel_disappears (30.61s)
=== CONT TestAccSageMakerModel_networkIsolation
--- PASS: TestAccSageMakerModel_primaryContainerPrivateDockerRegistry (41.08s)
=== CONT TestAccSageMakerModel_containers
--- PASS: TestAccSageMakerModel_networkIsolation (34.56s)
=== CONT TestAccSageMakerModel_vpc
--- PASS: TestAccSageMakerModel_containers (35.87s)
=== CONT TestAccSageMakerModel_primaryContainerModelPackageName
--- PASS: TestAccSageMakerModel_primaryContainerModelPackageName (33.49s)
=== CONT TestAccSageMakerModel_inferenceExecution
--- PASS: TestAccSageMakerModel_vpc (40.42s)
--- PASS: TestAccSageMakerModel_inferenceExecution (35.45s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/sagemaker 273.878s
@holly-evans Thanks for the contribution 🎉 👏. |
This functionality has been released in v5.2.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Description
SageMaker
Model
object has support for passing aModelPackageName
to create models from model packages but this functionality was not implemented in the provider.This PR adds this missing functionality.
Additionally,
image
is an optional argument for theContainers
andPrimaryContainer
structures.Relations
Closes #23968.
Closes #31533.
Closes #31532.
Closes #24004.
^ This PR combines these PRs and addresses the contributor feedback. @bschaatsbergen
References
https://docs.aws.amazon.com/sagemaker/latest/APIReference/API_CreateModel.html
https://docs.aws.amazon.com/sagemaker/latest/APIReference/API_ContainerDefinition.html
The AWS provider does not have a
aws_sagemaker_model_package
resource to create a new model package, so I used a free public model package in the test. I used a list from here - the models are region-specific.https://github.com/aws/amazon-sagemaker-examples/blob/main/aws_marketplace/using_model_packages/creative-writing-using-gpt-2-text-generation/src/model_package_arns.py
Output from Acceptance Testing
I am unable to run the tests on my AWS account.