-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
r/aws_lightsail_database: service/lightsail: new-resource #18663
r/aws_lightsail_database: service/lightsail: new-resource #18663
Conversation
…r max_instance_lifetime is 86400
ASG - correct min value on max_instance_lifetime in the docs
…orp#20701) As shown in hashicorp#20493 the documentation does not accurately reflect the current observed behaviour of this provider. The docs say that metadata_options.http_endpoint is optional, but the provider gives an error if http_endpoint is omitted.
Updated from main in order to resolve new conflicts. Can I get approval to run the workflows? |
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.
Hi @brittandeyoung thanks for this PR! Just dropping a couple additional comments for review
- Amend resource to add CustomizeDiff: verify.SetTagsDiff - Amend database waiter to return lightsail.GetRelationalDatabaseOutput - Amend operation waiter to use lightsail package constants - Amend database resource to remove duplicate logic now that database waiter returns database read output
@anGie44 I have implemented your recommendations for when you get time to look at this PR again. |
Updated Acceptance test run:
|
Bringing up to date with main
- Amend Config names to resolve semgrep errors - Amend function name to remove AWS
- Amend tests to resolve semgrep issues
Here is the latest acceptance test run:
|
- Amend to resolve semgrep function naming issues.
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.
This looks good to me. the only change imo is moving the consts.go content to database.go. convention post-reorg from everything in aws/
to service named directories we now put consts "as close to the code as possible"
Ik some changes have been happening to the semgrep functions also, once you push another commit the errors may resolve themselves. otherwise, you may have to update your test function names
@drewmullen I can make that update, but where would you suggest the I am working on pulling in the latest changes from main and running the semgrep tests locally to see if i get the same results. |
Pull latest from `main`
* Amend Function names to use camelcase
Looks like the function names needed to use camel case. Updated the functions and moved the constants as recommended. |
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 🚀.
% make testacc TESTARGS='-run=TestAccLightsailDatabase_' PKG=lightsail ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/lightsail/... -v -count 1 -parallel 2 -run=TestAccLightsailDatabase_ -timeout 180m
=== RUN TestAccLightsailDatabase_basic
=== PAUSE TestAccLightsailDatabase_basic
=== RUN TestAccLightsailDatabase_RelationalDatabaseName
=== PAUSE TestAccLightsailDatabase_RelationalDatabaseName
=== RUN TestAccLightsailDatabase_MasterDatabaseName
=== PAUSE TestAccLightsailDatabase_MasterDatabaseName
=== RUN TestAccLightsailDatabase_MasterUsername
=== PAUSE TestAccLightsailDatabase_MasterUsername
=== RUN TestAccLightsailDatabase_MasterPassword
=== PAUSE TestAccLightsailDatabase_MasterPassword
=== RUN TestAccLightsailDatabase_PreferredBackupWindow
=== PAUSE TestAccLightsailDatabase_PreferredBackupWindow
=== RUN TestAccLightsailDatabase_PreferredMaintenanceWindow
=== PAUSE TestAccLightsailDatabase_PreferredMaintenanceWindow
=== RUN TestAccLightsailDatabase_PubliclyAccessible
=== PAUSE TestAccLightsailDatabase_PubliclyAccessible
=== RUN TestAccLightsailDatabase_BackupRetentionEnabled
=== PAUSE TestAccLightsailDatabase_BackupRetentionEnabled
=== RUN TestAccLightsailDatabase_FinalSnapshotName
=== PAUSE TestAccLightsailDatabase_FinalSnapshotName
=== RUN TestAccLightsailDatabase_Tags
=== PAUSE TestAccLightsailDatabase_Tags
=== RUN TestAccLightsailDatabase_disappears
=== PAUSE TestAccLightsailDatabase_disappears
=== CONT TestAccLightsailDatabase_basic
=== CONT TestAccLightsailDatabase_PreferredMaintenanceWindow
--- PASS: TestAccLightsailDatabase_basic (968.62s)
=== CONT TestAccLightsailDatabase_FinalSnapshotName
--- PASS: TestAccLightsailDatabase_PreferredMaintenanceWindow (1175.99s)
=== CONT TestAccLightsailDatabase_disappears
--- PASS: TestAccLightsailDatabase_disappears (619.36s)
=== CONT TestAccLightsailDatabase_Tags
--- PASS: TestAccLightsailDatabase_FinalSnapshotName (1207.44s)
=== CONT TestAccLightsailDatabase_MasterUsername
--- PASS: TestAccLightsailDatabase_Tags (1101.79s)
=== CONT TestAccLightsailDatabase_PreferredBackupWindow
--- PASS: TestAccLightsailDatabase_PreferredBackupWindow (950.15s)
=== CONT TestAccLightsailDatabase_MasterPassword
--- PASS: TestAccLightsailDatabase_MasterPassword (2.73s)
=== CONT TestAccLightsailDatabase_BackupRetentionEnabled
--- PASS: TestAccLightsailDatabase_MasterUsername (2112.76s)
=== CONT TestAccLightsailDatabase_MasterDatabaseName
--- PASS: TestAccLightsailDatabase_BackupRetentionEnabled (1107.22s)
=== CONT TestAccLightsailDatabase_RelationalDatabaseName
--- PASS: TestAccLightsailDatabase_RelationalDatabaseName (991.80s)
=== CONT TestAccLightsailDatabase_PubliclyAccessible
--- PASS: TestAccLightsailDatabase_MasterDatabaseName (1783.68s)
--- PASS: TestAccLightsailDatabase_PubliclyAccessible (1033.54s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/lightsail 6986.815s
@brittandeyoung Thanks for the contribution 🎉 👏. |
% make providerlint
==> Checking source code with providerlint... |
This functionality has been released in v4.22.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Community Note
Release note for CHANGELOG:
Closes #8719
Output from acceptance testing:
This PR:
aws_lightsail_database