Skip to content

Conversation

swhite-oreilly
Copy link

@swhite-oreilly swhite-oreilly commented Aug 17, 2023

Description

Adding Completed to the job status types that are ignored upon cleanup for all Comprehend job types.

Adding support for the following job types: ComprehendEventsDetectionJob, ComprehendPiiEntititesDetectionJob, and ComprehendTargetedSentimentDetectionJob.

Note

document-classification-job and topics-detection-job do not have stop functions in the CLI nor Boto3. As any personal identifying data is deleted on S3 cleanup, and these jobs technically don't store anything but metadata links to the files in S3 and naming/tags, the team has decided support for these can be left in and it isn't a hard requirement to add support for these at this time.

dependabot bot and others added 19 commits July 25, 2023 10:26
Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.44.295 to 1.44.307.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.44.295...v1.44.307)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.44.307 to 1.44.313.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.44.307...v1.44.313)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Support for removing TGW Attachment Peering resources, as only VPC attachments
where supported before.

Co-authored-by: Björn Häuser <b.haeuser@rebuy.com>
…de#1046)

* kms-keys: Skip keys already in pending replica deletion state

Multi-region KMS keys enter state KeyStatePendingReplicaDeletion when
deleted, they should be filtered out in the list operation.

* Filter out PendingReplicaDeletion in the filter state rather than list state
Co-authored-by: Björn Häuser <b.haeuser@rebuy.com>
* Add formatting check to GitHub Actions

* Fix pre-existing formatting issues
Signed-off-by: Taylor Barrella <tbarrella@gmail.com>
Co-authored-by: Björn Häuser <b.haeuser@rebuy.com>
Signed-off-by: Taylor Barrella <tbarrella@gmail.com>
* Adding CloudFrontOriginRequestPolicy resources.

---------

Co-authored-by: Philipp Trulson <der-eismann@users.noreply.github.com>
Bumps [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) from 1.44.313 to 1.44.318.
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.44.313...v1.44.318)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
> This is an automatically generated PR.

@rebuy-de/ FYI
* Filter main route table
* use `Filter` instead of custom method
Bumps the golang group with 1 update: [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go).

- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.44.318...v1.44.323)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: golang
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: der-eismann <der-eismann@users.noreply.github.com>
Co-authored-by: der-eismann <der-eismann@users.noreply.github.com>
Adding `Completed` to the job status types that are ignored upon cleanup.
@swhite-oreilly swhite-oreilly changed the base branch from main to oreilly-main August 18, 2023 19:25
Copy link

@gsoria gsoria left a comment

Choose a reason for hiding this comment

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

These changes make sense to me. I've tried it locally and I see that the completed SentimentDetectionJob and EntitiesDetectionJob jobs were skipped.

main

us-east-1 - ComprehendEntityRecognizer - arn:aws:comprehend:us-east-1:318480998194:entity-recognizer/test-3241623422e062a1c5f9b9199432bff80d4109be - [EntityRecognizerArn: "arn:aws:comprehend:us-east-1:318480998194:entity-recognizer/test-3241623422e062a1c5f9b9199432bff80d4109be", LanguageCode: "en"] - would remove
us-east-1 - ComprehendDocumentClassifier - arn:aws:comprehend:us-east-1:318480998194:document-classifier/testDelete-3241623422e062a1c5f9b9199432bff80d4109be - [DocumentClassifierArn: "arn:aws:comprehend:us-east-1:318480998194:document-classifier/testDelete-3241623422e062a1c5f9b9199432bff80d4109be", LanguageCode: "en"] - would remove
us-east-1 - ComprehendSentimentDetectionJob - sentiment-analysis-3241623422e062a1c5f9b9199432bff80d4109be - [JobId: "c08aabeafd134afef71126260046ada6", JobName: "sentiment-analysis-3241623422e062a1c5f9b9199432bff80d4109be"] - would remove
us-east-1 - ComprehendEntitiesDetectionJob - entities-analysis-3241623422e062a1c5f9b9199432bff80d4109be - [JobId: "d7f4050116539b3ac90baf530c22807e", JobName: "entities-analysis-3241623422e062a1c5f9b9199432bff80d4109be"] - would remove
Scan complete: 4 total, 4 nukeable, 0 filtered.

this branch

us-east-1 - ComprehendEntityRecognizer - arn:aws:comprehend:us-east-1:318480998194:entity-recognizer/test-3241623422e062a1c5f9b9199432bff80d4109be - [EntityRecognizerArn: "arn:aws:comprehend:us-east-1:318480998194:entity-recognizer/test-3241623422e062a1c5f9b9199432bff80d4109be", LanguageCode: "en"] - would remove
us-east-1 - ComprehendDocumentClassifier - arn:aws:comprehend:us-east-1:318480998194:document-classifier/testDelete-3241623422e062a1c5f9b9199432bff80d4109be - [DocumentClassifierArn: "arn:aws:comprehend:us-east-1:318480998194:document-classifier/testDelete-3241623422e062a1c5f9b9199432bff80d4109be", LanguageCode: "en"] - would remove
Scan complete: 2 total, 2 nukeable, 0 filtered.

I see there are other resources not supported by aws-nuke nor cloud control API:

  • document-classification-job
  • document-classifier-endpoint
  • entity-recognizer-endpoint
  • events-detection-job
  • pii-entities-detection-job
  • targeted-sentiment-detection-job
  • topics-detection-job

Are these going to be covered as part of your changes?

Copy link

@corybekk corybekk left a comment

Choose a reason for hiding this comment

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

Just to make sure I am understanding the change, aws-nuke was trying to delete resources that were in the "completed" state? i.e resources that have already been deleted?

@swhite-oreilly
Copy link
Author

Just to make sure I am understanding the change, aws-nuke was trying to delete resources that were in the "completed" state? i.e resources that have already been deleted?

@corybekk These don't actually get deleted as they are just metadata pointing to the trained files in S3. The original function goal was to stop in progress jobs, but they missed filtering out those in the Completed status so they were causing errors because they were in the wrong state.

@swhite-oreilly
Copy link
Author

swhite-oreilly commented Aug 21, 2023

These changes make sense to me. I've tried it locally and I see that the completed SentimentDetectionJob and EntitiesDetectionJob jobs were skipped.

main

us-east-1 - ComprehendEntityRecognizer - arn:aws:comprehend:us-east-1:318480998194:entity-recognizer/test-3241623422e062a1c5f9b9199432bff80d4109be - [EntityRecognizerArn: "arn:aws:comprehend:us-east-1:318480998194:entity-recognizer/test-3241623422e062a1c5f9b9199432bff80d4109be", LanguageCode: "en"] - would remove
us-east-1 - ComprehendDocumentClassifier - arn:aws:comprehend:us-east-1:318480998194:document-classifier/testDelete-3241623422e062a1c5f9b9199432bff80d4109be - [DocumentClassifierArn: "arn:aws:comprehend:us-east-1:318480998194:document-classifier/testDelete-3241623422e062a1c5f9b9199432bff80d4109be", LanguageCode: "en"] - would remove
us-east-1 - ComprehendSentimentDetectionJob - sentiment-analysis-3241623422e062a1c5f9b9199432bff80d4109be - [JobId: "c08aabeafd134afef71126260046ada6", JobName: "sentiment-analysis-3241623422e062a1c5f9b9199432bff80d4109be"] - would remove
us-east-1 - ComprehendEntitiesDetectionJob - entities-analysis-3241623422e062a1c5f9b9199432bff80d4109be - [JobId: "d7f4050116539b3ac90baf530c22807e", JobName: "entities-analysis-3241623422e062a1c5f9b9199432bff80d4109be"] - would remove
Scan complete: 4 total, 4 nukeable, 0 filtered.

this branch

us-east-1 - ComprehendEntityRecognizer - arn:aws:comprehend:us-east-1:318480998194:entity-recognizer/test-3241623422e062a1c5f9b9199432bff80d4109be - [EntityRecognizerArn: "arn:aws:comprehend:us-east-1:318480998194:entity-recognizer/test-3241623422e062a1c5f9b9199432bff80d4109be", LanguageCode: "en"] - would remove
us-east-1 - ComprehendDocumentClassifier - arn:aws:comprehend:us-east-1:318480998194:document-classifier/testDelete-3241623422e062a1c5f9b9199432bff80d4109be - [DocumentClassifierArn: "arn:aws:comprehend:us-east-1:318480998194:document-classifier/testDelete-3241623422e062a1c5f9b9199432bff80d4109be", LanguageCode: "en"] - would remove
Scan complete: 2 total, 2 nukeable, 0 filtered.

I see there are other resources not supported by aws-nuke nor cloud control API:

* document-classification-job

* document-classifier-endpoint

* entity-recognizer-endpoint

* events-detection-job

* pii-entities-detection-job

* targeted-sentiment-detection-job

* topics-detection-job

Are these going to be covered as part of your changes?

@gsoria added support for all but document-classification-job or topics-detection-job. Waiting on team response in slack.

@corybekk
Copy link

Just to make sure I am understanding the change, aws-nuke was trying to delete resources that were in the "completed" state? i.e resources that have already been deleted?

@corybekk These don't actually get deleted as they are just metadata pointing to the trained files in S3. The original function goal was to stop in progress jobs, but they missed filtering out those in the Completed status so they were causing errors because they were in the wrong state.

thanks for the clarification!

dependabot bot and others added 5 commits August 22, 2023 10:56
Bumps the golang group with 2 updates: [github.com/aws/aws-sdk-go](https://github.com/aws/aws-sdk-go) and [github.com/google/uuid](https://github.com/google/uuid).


Updates `github.com/aws/aws-sdk-go` from 1.44.323 to 1.44.328
- [Release notes](https://github.com/aws/aws-sdk-go/releases)
- [Commits](aws/aws-sdk-go@v1.44.323...v1.44.328)

Updates `github.com/google/uuid` from 1.3.0 to 1.3.1
- [Release notes](https://github.com/google/uuid/releases)
- [Changelog](https://github.com/google/uuid/blob/master/CHANGELOG.md)
- [Commits](google/uuid@v1.3.0...v1.3.1)

---
updated-dependencies:
- dependency-name: github.com/aws/aws-sdk-go
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: golang
- dependency-name: github.com/google/uuid
  dependency-type: direct:production
  update-type: version-update:semver-patch
  dependency-group: golang
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Adding filters for key-phrases and dominant-language
der-eismann and others added 10 commits August 23, 2023 17:39
Co-authored-by: der-eismann <der-eismann@users.noreply.github.com>
* Adding Elasticache User and UserGroup Support

Adding go modules for elasticache users and groups.  Adding filtering for subnet groups to ignore the default elasticache subnet group.

* Create opensearchservice-packages.go

Adding working code for packages cleanup.

* Delete opensearchservice-packages.go

Moving opensearch changes to separate branch.

* Updating elasticache user/group list calls with pagination.

* Reverting versions to match oreilly-main

Reverting versions to match oreilly-main

* Updating go version to match upstream.

* Updating to more closely match style of other resource types.

* Adding properties to EC user/usergroups.
feat: MemoryDBCluster resource support
feat: MemoryDBParameterGroup resource support
feat: MemoryDBSubnetGroup resource support
feat: MemoryDBUser resource support
…uy-de#1053)

* Added Deletion protection disable feature in cognito user-pool

* Minor Changes

* nit

* spaces

* spaces 2

* stopping executions

* Stopped executiins

* nit

* Formatted

---------

Co-authored-by: Suleman Sohail <Suleman@Suleman-Sohail-SWE.local>
Co-authored-by: Philipp Trulson <der-eismann@users.noreply.github.com>
* Add RedshiftScheduledAction resource

---------

Co-authored-by: Björn Häuser <b.haeuser@rebuy.com>
Copy link

@gsoria gsoria left a comment

Choose a reason for hiding this comment

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

Nice updates! I've tried it locally and it correctly cleanup the resources. Initially the DocumentClassifier failed, but eventually was cleanup up as well.

us-east-1 - ComprehendDocumentClassifier - arn:aws:comprehend:us-east-1:430037344291:document-classifier/my-doc-classifier-21ef490ae3601ac28f88 - [DocumentClassifierArn: "arn:aws:comprehend:us-east-1:430037344291:document-classifier/my-doc-classifier-21ef490ae3601ac28f88", LanguageCode: "en"] - failed
...

us-east-1 - ComprehendDocumentClassifier - arn:aws:comprehend:us-east-1:430037344291:document-classifier/my-doc-classifier-21ef490ae3601ac28f88 - [DocumentClassifierArn: "arn:aws:comprehend:us-east-1:430037344291:document-classifier/my-doc-classifier-21ef490ae3601ac28f88", LanguageCode: "en"] - removed

Removal requested: 0 waiting, 0 failed, 0 skipped, 9 finished

Don't forget to sync with the upstream repo before merging this PR 🙂

Screenshot 2023-08-24 at 8 19 44 AM

@swhite-oreilly swhite-oreilly merged commit ea07a10 into oreilly-main Aug 24, 2023
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.