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

feat: MySQL binary logs on replica #466

Merged

Conversation

zculek-fb
Copy link
Contributor

Hello everyone,

As stated in https://cloud.google.com/sql/docs/mysql/replication#bin-log-replica, binary logging is supported on MySQL read replica instances (MySQL 5.7 and 8.0 only). Since having binary logs on read replicas could be handy (e.g. for Fivetran, etc.), this PR adds an option to enable binary logs on replicas and to set retention.

@zculek-fb zculek-fb requested a review from a team as a code owner May 2, 2023 12:05
@zculek-fb
Copy link
Contributor Author

Hello @g-awmalik @apeabody

Sorry to bug you, but can one of you please do a /gcbrun for this?

Thanks in advance!

@apeabody
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

Hello @g-awmalik @apeabody

Sorry to bug you, but can one of you please do a /gcbrun for this?

Thanks in advance!

Apologies for the delay. Looks good for the most part. Please review one minor suggestion.

Thanks,

@g-awmalik
Copy link
Contributor

/gcbrun

@zculek-fb
Copy link
Contributor Author

Thank you both!

@g-awmalik
Please review one minor suggestion.

Sure, but I'm not seeing any. Can you please point me to it?

@g-awmalik
Copy link
Contributor

Thank you both!

@g-awmalik
Please review one minor suggestion.

Sure, but I'm not seeing any. Can you please point me to it?

Apologies. I forgot to submit it.

g-awmalik and others added 2 commits June 2, 2023 09:24
No need to use lookup function, since there's no default defined.

Co-authored-by: Awais Malik <awmalik@google.com>
@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

/gcbrun

@zculek-fb
Copy link
Contributor Author

Hello

Just to check if anything else is needed for this to be merged and feature added to the module?

Thanks!

@zculek-fb
Copy link
Contributor Author

Hello @g-awmalik

Sorry to bother you, but just want to know if there is anything else needed here or it's good to be merged?

Thank you for your time and help!

@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

/gcbrun

@zculek-fb
Copy link
Contributor Author

@g-awmalik @isaurabhuttam
I've readded the README.md change as it seems it got lost with the merge from main, I think all should be good again.

@g-awmalik
Copy link
Contributor

/gcbrun

@zculek-fb
Copy link
Contributor Author

@g-awmalik

@g-awmalik
Copy link
Contributor

/gcbrun

1 similar comment
@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik
Copy link
Contributor

/gcbrun

@g-awmalik g-awmalik merged commit 0e0c196 into terraform-google-modules:master Sep 3, 2023
4 checks passed
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.

None yet

3 participants