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.

Sign up for GitHub

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

Jump to bottom

Refactor sidecar command, add configs #241

Merged
merged 4 commits into from
Mar 11, 2019
Merged

Refactor sidecar command, add configs #241

merged 4 commits into from
Mar 11, 2019

Conversation

AMecea
Copy link
Contributor

@AMecea AMecea commented Feb 25, 2019 •
edited
Loading

fixes #240

@AMecea AMecea force-pushed the refactor-sidecar-2 branch 4 times, most recently from 2de1e4d to a5596bc Compare March 4, 2019 10:55
calind
calind requested changes Mar 4, 2019
View reviewed changes
pkg/sidecar/app/configs.go Outdated Show resolved Hide resolved
pkg/sidecar/app/configs.go Outdated Show resolved Hide resolved
pkg/sidecar/app/configs.go Outdated Show resolved Hide resolved
pkg/sidecar/app/configs.go Outdated Show resolved Hide resolved
pkg/sidecar/app/configs.go Outdated Show resolved Hide resolved
@AMecea AMecea force-pushed the refactor-sidecar-2 branch from 60f5de4 to af6f097 Compare March 6, 2019 13:56
calind
calind requested changes Mar 7, 2019
View reviewed changes
pkg/sidecar/appconf.go Outdated Show resolved Hide resolved
pkg/sidecar/appconf.go Outdated Show resolved Hide resolved
pkg/sidecar/apphelper.go
return fmt.Errorf("mysql is not ready, err: %s", err)
}

// deactivate super read only
log.Info("temporary disable SUPER_READ_ONLY")
if err := util.RunQuery("SET GLOBAL READ_ONLY = 1; SET GLOBAL SUPER_READ_ONLY = 0;"); err != nil {
if err := runQuery(cfg, "SET GLOBAL READ_ONLY = 1; SET GLOBAL SUPER_READ_ONLY = 0;"); err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

Can we have the defer pattern here:

enableSuperReadOnly, err := disableSuperReadOnly()
defer enableSuperReadOnly()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR does not introduce disableSuperReadOnly function. Only in #243 this function is added.

pkg/sidecar/appconf.go Outdated Show resolved Hide resolved
pkg/sidecar/apphelper.go Outdated Show resolved Hide resolved
pkg/sidecar/apphelper.go Outdated Show resolved Hide resolved
pkg/sidecar/configs.go Outdated Show resolved Hide resolved
@AMecea AMecea force-pushed the refactor-sidecar-2 branch from af6f097 to 0e47237 Compare March 11, 2019 11:35
@calind calind merged commit 7b13ded into master Mar 11, 2019
@AMecea AMecea added this to the 0.2.6 milestone Mar 12, 2019
@AMecea AMecea deleted the refactor-sidecar-2 branch March 21, 2019 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@calind calind calind requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
0.2.6
Development

Successfully merging this pull request may close these issues.

Refactor sidecar module
2 participants
@AMecea @calind