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

chore: Add alternate defaults for mysql & postgresql blueprint #646

Merged
merged 15 commits into from
Oct 18, 2024

Conversation

qz267
Copy link
Contributor

@qz267 qz267 commented Sep 17, 2024

@qz267 qz267 requested review from isaurabhuttam, imrannayer and a team as code owners September 17, 2024 17:45
@imrannayer
Copy link
Collaborator

@qz267 is there any reason for changing defaults instead of sending values and override it in module call?

@qz267 qz267 changed the title Update new defaults [WIP] - Update new defaults Sep 17, 2024
@qz267
Copy link
Contributor Author

qz267 commented Sep 17, 2024

@qz267 is there any reason for changing defaults instead of sending values and override it in module call?

Sorry for the confusion, I was trying to surface some of my WIP changes, to ask for early feedbacks, PR title updated.

b/362377949 is the original request, I don't have strong preference about update in variables or override them in the module call, any suggestions are welcomed, thanks!

@imrannayer
Copy link
Collaborator

@qz267 I think it will be better to take alternate route instead of changing defaults in the module as this change will require all the existing user of the module to adjust code for new defaults.

@bharathkkb can u plz review?

@qz267
Copy link
Contributor Author

qz267 commented Sep 17, 2024

I got @q2w suggested using alt_defaults instead https://github.com/GoogleCloudPlatform/cloud-foundation-toolkit/blob/master/cli/bpmetadata/proto/bpmetadata_ui.proto#L149, WDYT?

@qz267 qz267 changed the title [WIP] - Update new defaults chore: Update alt defaults for mysql blueprint Oct 9, 2024
@q2w
Copy link
Collaborator

q2w commented Oct 14, 2024

@qz267 We should also update for modules/postgresql.

@qz267 qz267 changed the title chore: Update alt defaults for mysql blueprint chore: Update alt defaults for mysql & postgres blueprint Oct 15, 2024
modules/postgresql/metadata.display.yaml Show resolved Hide resolved
modules/postgresql/metadata.display.yaml Outdated Show resolved Hide resolved
modules/postgresql/metadata.display.yaml Show resolved Hide resolved
modules/mysql/metadata.display.yaml Outdated Show resolved Hide resolved
@qz267 qz267 requested a review from q2w October 17, 2024 16:38
@q2w
Copy link
Collaborator

q2w commented Oct 18, 2024

/gcbrun

@q2w q2w enabled auto-merge (squash) October 18, 2024 06:05
@q2w q2w changed the title chore: Update alt defaults for mysql & postgres blueprint chore: Add alternate defaults for mysql & postgresql blueprint Oct 18, 2024
@q2w q2w merged commit 72bb2f5 into terraform-google-modules:master Oct 18, 2024
4 checks passed
q2w added a commit that referenced this pull request Oct 18, 2024
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.

4 participants