Skip to content
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

New Resource: aws_sagemaker_training_job #2955

Closed
wants to merge 5 commits into from

Conversation

ddcprg
Copy link
Contributor

@ddcprg ddcprg commented Jan 12, 2018

Helps resolving #2493

@ddcprg ddcprg mentioned this pull request Jan 12, 2018
@@ -0,0 +1,119 @@
package aws
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file is a copy of the file in #2478

@@ -664,6 +664,24 @@ func validateS3BucketLifecycleRuleId(v interface{}, k string) (ws []string, erro
return
}

func validateSagemakerName(v interface{}, k string) (ws []string, errors []error) {
Copy link
Contributor Author

@ddcprg ddcprg Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function has been copied from the same file in #2478

@Ninir Ninir changed the title [issue-2493] SageMaker Training Job resource new resource: SageMaker Training Job Jan 12, 2018
@Ninir Ninir changed the title new resource: SageMaker Training Job New resource: SageMaker Training Job Jan 12, 2018
@Ninir Ninir added enhancement Requests to existing resources that expand the functionality or scope. new-resource Introduces a new resource. labels Jan 12, 2018
@ddcprg ddcprg force-pushed the feature/sagemaker-vendor branch 2 times, most recently from 2e30876 to d8969b3 Compare January 15, 2018 10:18
@radeksimko radeksimko added the service/sagemaker Issues and PRs that pertain to the sagemaker service. label Jan 16, 2018
@radeksimko radeksimko changed the title New resource: SageMaker Training Job New Resource: aws_sagemaker_training_job Jan 16, 2018
@radeksimko radeksimko removed the enhancement Requests to existing resources that expand the functionality or scope. label Jan 16, 2018
@ddcprg ddcprg force-pushed the feature/sagemaker-vendor branch 2 times, most recently from c23ff51 to 87ead61 Compare January 30, 2018 10:40
@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Jan 30, 2018
@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Jan 30, 2018
@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Jan 30, 2018
@ddcprg
Copy link
Contributor Author

ddcprg commented Mar 22, 2018

The build has failed due to S3 errors in aws/validators_test.go

@ddcprg
Copy link
Contributor Author

ddcprg commented May 16, 2018

The build error is unrelated to this PR

@bcornils
Copy link
Contributor

looks like we have a check failing. Has anyone had time to assess or review criticality of the failure?

@ddcprg
Copy link
Contributor Author

ddcprg commented Jun 12, 2018

@bcornils I'll merge master and build again, the error was unrelated to this change last time I checked

@ghost ghost added the size/XXL Managed by automation to categorize the size of a PR. label Jun 12, 2018
@ibash
Copy link

ibash commented Jul 17, 2018

🙌

@ghost ghost added the documentation Introduces or discusses updates to documentation. label Nov 1, 2018
@ghost ghost added provider Pertains to the provider itself, rather than any interaction with AWS. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Nov 1, 2018
@bflad
Copy link
Contributor

bflad commented Apr 25, 2019

Hi @ddcprg 👋 Thanks so much for submitting this and sorry for the delayed review process. We have recently merged in a bunch of the Sagemaker resources surrounding model deployment and now we are ready to review/work through other parts of Sagemaker. Could you please rebase this PR? The shared files currently conflicting should be stable now for this pull request. 🤞

@bflad bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Apr 25, 2019
@ddcprg
Copy link
Contributor Author

ddcprg commented Apr 30, 2019

hi @bflad no problem at all, I thought this one was going to be left out for being a static non-deletable resource in AWS. I'll try to take a look at this today and leave an update here

@ghost ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Apr 30, 2019
@ddcprg
Copy link
Contributor Author

ddcprg commented May 9, 2019

@bflad I've brought this branch up to date, feel free to take a look any time, cheers

@aeschright aeschright requested a review from a team June 25, 2019 18:51
@nicholasgcoles
Copy link

@ddcprg do you have any idea if this will get merged anytime soon?

@ddcprg
Copy link
Contributor Author

ddcprg commented May 21, 2020

hey @nicholasgcoles I'm not able to tell, I was expecting some feedback from @bflad

@teamterraform
Copy link

Notification of Recent and Upcoming Changes to Contributions

Thank you for this contribution! There have been a few recent development changes that affect this pull request. We apologize for the inconvenience, especially if there have been long review delays up until now. Please note that this is automated message from an unmonitored account. See the FAQ for additional information on the maintainer team and review prioritization.

If you are unable to complete these updates, please leave a comment for the community and maintainers so someone can potentially continue the work. The maintainers will encourage other contributors to use the existing contribution as the base for additional changes as appropriate. Otherwise, contributions that do not receive updated code or comments from the original contributor may be closed in the future so the maintainers can focus on active items.

For the most up to date information about Terraform AWS Provider development, see the Contributing Guide. Additional technical debt changes can be tracked with the technical-debt label on issues.

As part of updating a pull request with these changes, the most current unit testing and linting will run. These may report issues that were not previously reported.

Action Required: Terraform 0.12 Syntax

Reference: #8950
Reference: #14417

Version 3 and later of the Terraform AWS Provider, which all existing contributions would potentially be added, only supports Terraform 0.12 and later. Certain syntax elements of Terraform 0.11 and earlier show deprecation warnings during runs with Terraform 0.12. Documentation and test configurations, such as those including deprecated string interpolations (some_attribute = "${aws_service_thing.example.id}") should be updated to the newer syntax (some_attribute = aws_service_thing.example.id). Contribution testing will automatically fail on older syntax in the near future. Please see the referenced issues for additional information.

Action Required: Terraform Plugin SDK Version 2

Reference: #14551

The Terraform AWS Provider has been upgraded to the latest version of the Terraform Plugin SDK. Generally, most changes to contributions should only involve updating Go import paths in source code files. Please see the referenced issue for additional information.

Action Required: Removal of website/aws.erb File

Reference: #14712

Any changes to the website/aws.erb file are no longer necessary and should be removed from this contribution to prevent merge issues in the near future when the file is removed from the repository. Please see the referenced issue for additional information.

Upcoming Change of Git Branch Naming

Reference: #14292

Development environments will need their upstream Git branch updated from master to main in the near future. Please see the referenced issue for additional information and scheduling.

Upcoming Change of GitHub Organization

Reference: #14715

This repository will be migrating from https://github.com/terraform-providers/terraform-provider-aws to https://github.com/hashicorp/terraform-provider-aws. No practitioner or developer action is anticipated and most GitHub functionality will automatically redirect to the new location. Go import paths including terraform-providers can remain for now. Please see the referenced issue for additional information and scheduling.

@DrFaust92
Copy link
Collaborator

Hey @ddcprg, can you rebase and address V2 changes mentioned above and then ill take a look and review this PR.

@ddcprg
Copy link
Contributor Author

ddcprg commented Sep 30, 2020

@DrFaust92 I've left the company where this code is and they removed it as collaborator no long ago so I cannot longer make changes to this PR. I'll have to close and open a new one in my own fork - AFAIK there is no way to change the PR to another fork. I'd like to confirm whether this is actually going to be merged as I don't have a lot of spare time at the moment, last feedback I got is this was a static resource and should not be part of terraform - I think this was before the AWS provider was refactored into its own project.

@DrFaust92
Copy link
Collaborator

Let me look at the API and I'll get back to you. I can fork the original branch an continue of you would like.

@ddcprg
Copy link
Contributor Author

ddcprg commented Oct 27, 2020

any updates on this one @DrFaust92 ? if we can continue where we left that'd be great

@DrFaust92
Copy link
Collaborator

DrFaust92 commented Nov 22, 2020

Hey @ddcprg if you are willing to continue with this PR ill help with the review.

Edit: after looking at the API and docs im not so sure this fits in terraform (as previously mentioned) if you can provide a use case for this we can discuss how to go about implementing this.

Base automatically changed from master to main January 23, 2021 00:55
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:55
@DrFaust92
Copy link
Collaborator

Closing, If this is still relevant please open a new issue with the relevant information on why this is resource is needed to be supported by the provider.

@DrFaust92 DrFaust92 closed this Jan 29, 2021
@ghost
Copy link

ghost commented Feb 28, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators Feb 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. new-resource Introduces a new resource. provider Pertains to the provider itself, rather than any interaction with AWS. service/sagemaker Issues and PRs that pertain to the sagemaker service. size/XXL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants