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

Update PostgreSQL to GA 2017-12-01 API #1190

Merged
merged 9 commits into from
May 23, 2018
Merged

Update PostgreSQL to GA 2017-12-01 API #1190

merged 9 commits into from
May 23, 2018

Conversation

WodansSon
Copy link
Collaborator

Upgrade the PostgreSQL resource the adopt the GA API 2017-12-01. This PR will resolve issue #1004 .

@WodansSon
Copy link
Collaborator Author

WodansSon commented May 2, 2018

DiffSuppressFunc: ignoreCaseDiffSuppressFunc,
},

"family": {
Copy link

Choose a reason for hiding this comment

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

What is the relationship between this and "sku.name"? Because I found that "sku.name" also contains "Gen4" or "Gen5".

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All of the fields in the struct are concatenated together to form the sku.name in the form of tier_family_capacity. This is how the API's schema is defined, I thought about splitting the values out of the name, but then the individual values could not be referenced.

@tombuildsstuff
Copy link
Contributor

@jeffreyCline FYI I'm going to rebase this PR on top of master, since the Diff is showing changes which are already in master (but aren't in this branch)

Copy link
Contributor

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @jeffreyCline

Thanks for pushing the updates to this PR, apologies for the delay in reviewing this whilst we've been out. I've rebased this and pushed a couple of minor updates to match the comments I raised in the MySQL PR and this now LGTM. I'll test the upgrade path shortly (and I've just kicked off the test suite) 👍

Thanks!

@tombuildsstuff
Copy link
Contributor

@jeffreyCline actually in testing this I've found a couple of bugs in this and MySQL regarding SKU's - I'll push a couple of commits to enable SKU changes without forcing a new resource - which is supported for some options:

screen shot 2018-05-22 at 20 08 32

- Switching SKU to a List instead of a Set
- Support for updating the Capacity of a Given SKU
- Tests to update the SKU's for Postgresql/MySql
@tombuildsstuff
Copy link
Contributor

Re-tested the upgrade path from both MySql & Postgresql -> new version and they work as expected. It's also possible to update the Capacity as in the Portal - and there's tests for this.

@tombuildsstuff tombuildsstuff modified the milestones: Soon, 1.6.0 May 22, 2018
@tombuildsstuff
Copy link
Contributor

MySQL tests pass:

screen shot 2018-05-22 at 21 03 31

@tombuildsstuff
Copy link
Contributor

PostgreSQL tests pass:

screen shot 2018-05-23 at 10 55 06

@tombuildsstuff tombuildsstuff merged commit 6868a7c into master May 23, 2018
@tombuildsstuff tombuildsstuff deleted the u-postgresql branch May 23, 2018 09:56
@ghost
Copy link

ghost commented Mar 31, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants