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

Add support for SageMaker Space Apps and Spaces #157

Closed
npellegrin opened this issue May 3, 2024 · 6 comments · Fixed by #158
Closed

Add support for SageMaker Space Apps and Spaces #157

npellegrin opened this issue May 3, 2024 · 6 comments · Fixed by #158

Comments

@npellegrin
Copy link
Contributor

In rebuy.de/aws-nuke the current SageMakerApp implementation doesn't support deleting apps with a SpaceName. Also, SageMaker Spaces are not currently supported.

This issue duplicates rebuy-de/aws-nuke#1184 to implement the feature in ekristen/aws-nuke

@ekristen
Copy link
Owner

ekristen commented May 3, 2024

🎉 This issue has been resolved in version 3.0.0-beta.47 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@ekristen
Copy link
Owner

ekristen commented May 3, 2024

Thanks for the issue and the PR! I try and see PRs that folks open up over on rebuy-de/aws-nuke, but sometimes I miss them. I have tooling that does automatic conversions for me that gets the code 95% of the way there. (see migrate tool) but it's not documented.

Happy to help get any other PRs converted or added. I'll add tests for the sagemaker stuff later. One goal I have is to get better testing place such that we can run a test suite when adding new resources in.

@npellegrin
Copy link
Contributor Author

Thanks for the migrate tool! Will try it with my next contributions 😃

Agree on unit tests, I will implement some with my other PR because it is hard to test Bedrock stuff on real infrastructure without giving a lot of money to AWS.

@ekristen
Copy link
Owner

ekristen commented May 3, 2024

Indeed, probably why they haven't been added before. Do you occasionally have bedrock resources that need cleaned up or just trying to be proactive? Ideally we should get some real world usage before getting it in, but if we can get some decent mocks in place I'm ok with that too.

Any additional unit tests you can provide is much appreciated. In the near future I'll likely make it a requirement, but not there yet. Goal is to speed up getting bug fixes and resources in, tests help greatly with that.

@npellegrin
Copy link
Contributor Author

I have some bedrock resources to remove occasionally. In particular, last week a provisioned throughput for Bedrock earned AWS $400 in 24 hours before we noticed 😭

I'll be able to test this PR with a real infrastructure, at least once to validate everything is properly deleted, but I'll start with mocks it will be easier.

@ekristen
Copy link
Owner

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants