-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add folder sweeper #16095
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
base: main
Are you sure you want to change the base?
Add folder sweeper #16095
Conversation
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Errors
|
Tests analyticsTotal tests: 0 Click here to see the affected service packages
🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR. View the build log |
550ff33 to
99247df
Compare
Tests analyticsTotal tests: 168 Click here to see the affected service packages
🟢 All tests passed! View the build log |
19acdd0 to
51a496d
Compare
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Errors
|
Tests analyticsTotal tests: 0 Click here to see the affected service packages
🔴 Errors occurred during REPLAYING mode. Please fix them to complete your PR. View the build log |
51a496d to
f734aa9
Compare
Tests analyticsTotal tests: 168 Click here to see the affected service packages
🟢 All tests passed! View the build log |
shuyama1
left a comment
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.
Looks good overall. However, should this follow the same logic as the project sweeper? Like, introducing a env var to be able to skip this? mmv1/third_party/terraform/services/resourcemanager/resource_google_project_sweeper.go
We likely need to skip this in service sweeper runs in provider tests and instead run that after all nightly tests are done, alongside the project sweeper for the organization
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
9344161 to
216d406
Compare
Tests analyticsTotal tests: 167 Click here to see the affected service packages
🟢 All tests passed! View the build log |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
|
Tests analyticsTotal tests: 168 Click here to see the affected service packages
🟢 All tests passed! View the build log |
Good catch. Thanks for pointing it out. I added the env var for folder sweeper and env var works locally. |
Added the folder sweeper to sweep the folders with the prefix
tf-test.Sometimes deleting folders failed because the folders are not empty.
The PR only cover the case that the folders have configured capability. Other non-empty cases could be handled in the future.
Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.