r/sagemaker_device_fleet - new resource #20058

DrFaust92 · 2021-07-02T14:44:32Z

Community Note

Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates OR Closes #0000

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSSagemakerDeviceFleet_'
--- PASS: TestAccAWSSagemakerDeviceFleet_disappears (66.25s)
--- PASS: TestAccAWSSagemakerDeviceFleet_basic (73.51s)
--- PASS: TestAccAWSSagemakerDeviceFleet_description (139.28s)
--- PASS: TestAccAWSSagemakerDeviceFleet_tags (219.07s)

gdavison

I have a few comments so far

gdavison · 2021-07-15T18:23:01Z

aws/internal/service/sagemaker/finder/finder.go

+		DeviceFleetName: aws.String(id),
+	}
+
+	output, err := conn.DescribeDeviceFleet(input)


Looking at https://docs.aws.amazon.com/sagemaker/latest/APIReference/API_DescribeDeviceFleet.html, it looks like the API can return ResourceNotFound if the Device Fleet doesn't exist

wasnt able to produce this error from tests

moved error check to finder

gdavison · 2021-07-15T18:35:24Z

aws/resource_aws_sagemaker_device_fleet.go

+
+	deviceFleet, err := finder.DeviceFleetByName(conn, d.Id())
+	if err != nil {
+		if isAWSErr(err, "ValidationException", "No devicefleet with name") {


Does the ValidationException have a nested error that has a specific code? We'd like to reduce the number of places we check the error message, since AWS recommends not relying on it.

anyway i can verify this? (some common function the provider supports to check it)

from a few tests, doesnt seem like it.

aws/resource_aws_sagemaker_device_fleet.go

website/docs/r/sagemaker_device_fleet.html.markdown

ewbankkit

LGTM 🚀.

Commercial

 % make testacc TESTARGS='-run=TestAccAWSSagemakerDeviceFleet_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSagemakerDeviceFleet_ -timeout 180m
=== RUN   TestAccAWSSagemakerDeviceFleet_basic
=== PAUSE TestAccAWSSagemakerDeviceFleet_basic
=== RUN   TestAccAWSSagemakerDeviceFleet_description
=== PAUSE TestAccAWSSagemakerDeviceFleet_description
=== RUN   TestAccAWSSagemakerDeviceFleet_tags
=== PAUSE TestAccAWSSagemakerDeviceFleet_tags
=== RUN   TestAccAWSSagemakerDeviceFleet_disappears
=== PAUSE TestAccAWSSagemakerDeviceFleet_disappears
=== CONT  TestAccAWSSagemakerDeviceFleet_basic
=== CONT  TestAccAWSSagemakerDeviceFleet_disappears
=== CONT  TestAccAWSSagemakerDeviceFleet_description
=== CONT  TestAccAWSSagemakerDeviceFleet_tags
--- PASS: TestAccAWSSagemakerDeviceFleet_basic (38.30s)
--- PASS: TestAccAWSSagemakerDeviceFleet_description (69.04s)
--- PASS: TestAccAWSSagemakerDeviceFleet_disappears (77.98s)
--- PASS: TestAccAWSSagemakerDeviceFleet_tags (171.97s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       176.062s

GovCloud

% make testacc TESTARGS='-run=TestAccAWSSagemakerDeviceFleet_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSSagemakerDeviceFleet_ -timeout 180m
=== RUN   TestAccAWSSagemakerDeviceFleet_basic
=== PAUSE TestAccAWSSagemakerDeviceFleet_basic
=== RUN   TestAccAWSSagemakerDeviceFleet_description
=== PAUSE TestAccAWSSagemakerDeviceFleet_description
=== RUN   TestAccAWSSagemakerDeviceFleet_tags
=== PAUSE TestAccAWSSagemakerDeviceFleet_tags
=== RUN   TestAccAWSSagemakerDeviceFleet_disappears
=== PAUSE TestAccAWSSagemakerDeviceFleet_disappears
=== CONT  TestAccAWSSagemakerDeviceFleet_basic
=== CONT  TestAccAWSSagemakerDeviceFleet_disappears
=== CONT  TestAccAWSSagemakerDeviceFleet_description
=== CONT  TestAccAWSSagemakerDeviceFleet_tags
=== CONT  TestAccAWSSagemakerDeviceFleet_basic
    provider_test.go:1166: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: Error attaching policy arn:aws-us-gov:iam::aws:policy/service-role/AmazonSageMakerEdgeDeviceFleetPolicy to IAM Role tf-acc-test-2090968910314501706: NoSuchEntity: Policy arn:aws-us-gov:iam::aws:policy/service-role/AmazonSageMakerEdgeDeviceFleetPolicy does not exist or is not attachable.
        	status code: 404, request id: e6327bde-ff83-4406-9993-c26085edc747
        
          with aws_iam_role_policy_attachment.test,
          on terraform_plugin_test.tf line 57, in resource "aws_iam_role_policy_attachment" "test":
          57: resource "aws_iam_role_policy_attachment" "test" {
        
        
        Error: error creating SageMaker Device Fleet tf-acc-test-2090968910314501706: UnknownOperationException: The requested operation is not supported in the called region.
        	status code: 400, request id: 815bca17-94f9-4cce-9f02-25d5fc140db5
        
          with aws_sagemaker_device_fleet.test,
          on terraform_plugin_test.tf line 62, in resource "aws_sagemaker_device_fleet" "test":
          62: resource "aws_sagemaker_device_fleet" "test" {
        
=== CONT  TestAccAWSSagemakerDeviceFleet_disappears
    provider_test.go:1166: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: Error attaching policy arn:aws-us-gov:iam::aws:policy/service-role/AmazonSageMakerEdgeDeviceFleetPolicy to IAM Role tf-acc-test-62678035077255900: NoSuchEntity: Policy arn:aws-us-gov:iam::aws:policy/service-role/AmazonSageMakerEdgeDeviceFleetPolicy does not exist or is not attachable.
        	status code: 404, request id: 953788f7-fd80-4fce-8baa-2f6613d19df7
        
          with aws_iam_role_policy_attachment.test,
          on terraform_plugin_test.tf line 57, in resource "aws_iam_role_policy_attachment" "test":
          57: resource "aws_iam_role_policy_attachment" "test" {
        
        
        Error: error creating SageMaker Device Fleet tf-acc-test-62678035077255900: UnknownOperationException: The requested operation is not supported in the called region.
        	status code: 400, request id: 8c8295f1-cbeb-4ca1-a805-da9ce148462a
        
          with aws_sagemaker_device_fleet.test,
          on terraform_plugin_test.tf line 62, in resource "aws_sagemaker_device_fleet" "test":
          62: resource "aws_sagemaker_device_fleet" "test" {
        
=== CONT  TestAccAWSSagemakerDeviceFleet_tags
    provider_test.go:1166: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: Error attaching policy arn:aws-us-gov:iam::aws:policy/service-role/AmazonSageMakerEdgeDeviceFleetPolicy to IAM Role tf-acc-test-3903314069976798545: NoSuchEntity: Policy arn:aws-us-gov:iam::aws:policy/service-role/AmazonSageMakerEdgeDeviceFleetPolicy does not exist or is not attachable.
        	status code: 404, request id: 47b7ec0d-87ad-4d90-abac-6d1c6a390079
        
          with aws_iam_role_policy_attachment.test,
          on terraform_plugin_test.tf line 57, in resource "aws_iam_role_policy_attachment" "test":
          57: resource "aws_iam_role_policy_attachment" "test" {
        
        
        Error: error creating SageMaker Device Fleet tf-acc-test-3903314069976798545: UnknownOperationException: The requested operation is not supported in the called region.
        	status code: 400, request id: 000e30fd-de9d-4ce1-b9de-4e95efc69d50
        
          with aws_sagemaker_device_fleet.test,
          on terraform_plugin_test.tf line 62, in resource "aws_sagemaker_device_fleet" "test":
          62: resource "aws_sagemaker_device_fleet" "test" {
        
--- SKIP: TestAccAWSSagemakerDeviceFleet_basic (19.51s)
--- SKIP: TestAccAWSSagemakerDeviceFleet_tags (19.52s)
--- SKIP: TestAccAWSSagemakerDeviceFleet_disappears (19.54s)
=== CONT  TestAccAWSSagemakerDeviceFleet_description
    provider_test.go:1166: skipping test for aws-us-gov/us-gov-west-1: Error running apply: exit status 1
        
        Error: Error attaching policy arn:aws-us-gov:iam::aws:policy/service-role/AmazonSageMakerEdgeDeviceFleetPolicy to IAM Role tf-acc-test-6807235469790963240: NoSuchEntity: Policy arn:aws-us-gov:iam::aws:policy/service-role/AmazonSageMakerEdgeDeviceFleetPolicy does not exist or is not attachable.
        	status code: 404, request id: 39dd7028-c851-457e-bae4-13824e016587
        
          with aws_iam_role_policy_attachment.test,
          on terraform_plugin_test.tf line 57, in resource "aws_iam_role_policy_attachment" "test":
          57: resource "aws_iam_role_policy_attachment" "test" {
        
        
        Error: error creating SageMaker Device Fleet tf-acc-test-6807235469790963240: UnknownOperationException: The requested operation is not supported in the called region.
        	status code: 400, request id: 231cf23d-bd35-4462-be8e-408cad8fc9f5
        
          with aws_sagemaker_device_fleet.test,
          on terraform_plugin_test.tf line 62, in resource "aws_sagemaker_device_fleet" "test":
          62: resource "aws_sagemaker_device_fleet" "test" {
        
--- SKIP: TestAccAWSSagemakerDeviceFleet_description (25.27s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	28.316s

ewbankkit · 2021-08-17T14:13:46Z

@DrFaust92 Thanks for the contribution 🎉 👏.

github-actions · 2021-08-19T22:40:21Z

This functionality has been released in v3.55.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!

github-actions · 2021-09-19T02:08:36Z

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.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

DrFaust92 marked this pull request as draft July 2, 2021 14:44

DrFaust92 added the new-resource Introduces a new resource. label Jul 2, 2021

github-actions bot added the size/XL Managed by automation to categorize the size of a PR. label Jul 2, 2021

DrFaust92 marked this pull request as ready for review July 3, 2021 21:53

gdavison reviewed Jul 15, 2021

View reviewed changes

DrFaust92 force-pushed the r/sagemaker_device_fleet branch from c6d939e to d883267 Compare July 15, 2021 19:04

DrFaust92 added 11 commits August 14, 2021 14:22

initial commit

bdf6583

add sagemaker device fleet resouce

24ac3f2

docs

04c535a

adjust tests

a03a609

add iam role check

08aa5b3

fmt

73f4a8a

diff for tags

3d8fede

changelog

d300324

less permissions

8e099ee

CR comment

baad5ce

use finder better

5266ccb

DrFaust92 force-pushed the r/sagemaker_device_fleet branch from d883267 to 5266ccb Compare August 14, 2021 11:42

ewbankkit self-assigned this Aug 17, 2021

ewbankkit approved these changes Aug 17, 2021

View reviewed changes

ewbankkit removed their assignment Aug 17, 2021

ewbankkit merged commit 1c69c66 into hashicorp:main Aug 17, 2021

github-actions bot added this to the v3.55.0 milestone Aug 17, 2021

DrFaust92 deleted the r/sagemaker_device_fleet branch August 17, 2021 16:44

github-actions bot locked as resolved and limited conversation to collaborators Sep 19, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

r/sagemaker_device_fleet - new resource #20058

r/sagemaker_device_fleet - new resource #20058

DrFaust92 commented Jul 2, 2021 •

edited

Loading

gdavison left a comment

gdavison Jul 15, 2021

DrFaust92 Jul 15, 2021

DrFaust92 Aug 14, 2021

gdavison Jul 15, 2021

DrFaust92 Jul 15, 2021

DrFaust92 Aug 14, 2021

ewbankkit left a comment

ewbankkit commented Aug 17, 2021

github-actions bot commented Aug 19, 2021

github-actions bot commented Sep 19, 2021

r/sagemaker_device_fleet - new resource #20058

r/sagemaker_device_fleet - new resource #20058

Conversation

DrFaust92 commented Jul 2, 2021 • edited Loading

Community Note

gdavison left a comment

Choose a reason for hiding this comment

gdavison Jul 15, 2021

Choose a reason for hiding this comment

DrFaust92 Jul 15, 2021

Choose a reason for hiding this comment

DrFaust92 Aug 14, 2021

Choose a reason for hiding this comment

gdavison Jul 15, 2021

Choose a reason for hiding this comment

DrFaust92 Jul 15, 2021

Choose a reason for hiding this comment

DrFaust92 Aug 14, 2021

Choose a reason for hiding this comment

ewbankkit left a comment

Choose a reason for hiding this comment

Commercial

GovCloud

ewbankkit commented Aug 17, 2021

github-actions bot commented Aug 19, 2021

github-actions bot commented Sep 19, 2021

DrFaust92 commented Jul 2, 2021 •

edited

Loading