-
Notifications
You must be signed in to change notification settings - Fork 39
chore: Updates cleanup-test-env workflow for MongoDB Atlas provider and cleanup script #1281
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
Conversation
…leanup-prod-data-federation job
| runs-on: ubuntu-latest | ||
| steps: | ||
| - name: Checkout | ||
| - name: terraform-provider-mongodbatlas-checkout |
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.
i had to see the new file entirely instead of diff to understand this checkout name :-)
| scripts | ||
| - uses: mongodb/atlas-github-action@15663d068c40a8582d881560961fce9d45e0df9a | ||
| repository: mongodb/terraform-provider-mongodbatlas | ||
| ref: 133ee57735e0c8f2b22e74bbd9f8c3fdd8da2f9c |
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.
as we control both repos, how about using master directly?
In current approach I can see the benefit that for instance if we change workflow path this won't break. As a con we need to remember to update CFN if we do some improvements in clean GHA in TF
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.
A pro for using the SHA is caching. Not a strong opinion, but prefer keeping as is.
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.
I think I'd prefer to use master as well. If we change the workflow path or something things will simply fail right? and at least we'll get notified that ways to take an action. With the current approach it's easy to miss out on updating the CFN repo.
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.
Changed in 455c7d5 ✅
| shell: bash | ||
| uses: ./.github/templates/clean-atlas-org | ||
| env: | ||
| MONGODB_ATLAS_PUBLIC_KEY: ${{ secrets.CLOUD_DEV_PUBLIC_KEY }} |
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.
what projects are we effectively cleaning with this PR before unifying names?
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.
Not many, although the CFNTest org had 98 projects removed.
maastha
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.
LGTM
Proposed changes
Link to any related issue(s): CLOUDP-288937
Successful run
Type of change:
expected)
Manual QA performed:
Required Checklist:
make fmtand formatted my codeworks in Atlas
Further comments