Skip to content

Add Docker images for Percona Server.#1547

Merged
enisoc merged 1 commit into
vitessio:masterfrom
enisoc:percona
Mar 5, 2016
Merged

Add Docker images for Percona Server.#1547
enisoc merged 1 commit into
vitessio:masterfrom
enisoc:percona

Conversation

@enisoc
Copy link
Copy Markdown
Contributor

@enisoc enisoc commented Mar 3, 2016

Review on Reviewable

@alainjobart
Copy link
Copy Markdown
Contributor

Is it really a different GTID implementation than MySQL 5.6?

@enisoc
Copy link
Copy Markdown
Contributor Author

enisoc commented Mar 3, 2016

I don't think so. This was contributed as a patch file by another Googler, and I turned it into a PR so I can take a look and clean it up. I asked him if there are any changes in the copied GTID files, and he said he just copied them because he's not familiar with how our flavor plugin system works. So I should be able to dedup those Go files.

@michael-berlin
Copy link
Copy Markdown
Contributor

I reviewed the whole change because I was interested in the GTID implementation ;)

So I left some comments there. I didn't know that you didn't write this, so excuse me when I meant you :)


Reviewed 17 of 17 files at r1.
Review status: all files reviewed at latest revision, 10 unresolved discussions, some commit checks broke.


bootstrap.sh, line 143 [r1] (raw file):
Why do you want to remove the default? I liked that :) Less things to configure and worry about.


dev.env, line 88 [r1] (raw file):
Fix indentation please. (Should be the same as the case before.)


docker/bootstrap/build.sh, line 18 [r1] (raw file):
Why that?


docker/bootstrap/Dockerfile.percona, line 8 [r1] (raw file):
Mix of tabs and spaces. Convert all to spaces?


docker/lite/Dockerfile.percona, line 5 [r1] (raw file):
RUN?


docker/lite/Dockerfile.percona, line 9 [r1] (raw file):
Mixed tabs and spaces.


docker/lite/Dockerfile.percona, line 18 [r1] (raw file):
Could we source these from a common file?


go/vt/mysqlctl/mysql_flavor_percona.go, line 167 [r1] (raw file):
missing period


go/vt/mysqlctl/replication/percona_gtid.go, line 73 [r1] (raw file):
Missing period.


go/vt/mysqlctl/replication/percona_gtid_set.go, line 36 [r1] (raw file):
It would be nice to have here a comment with an example for "s".


Comments from the review on Reviewable.io

@alainjobart
Copy link
Copy Markdown
Contributor

If the GTID code is the same as MySQL 5.6, it just shouldn't be duplicated... The rest I think is just using the right docker image for Percona in our scripts.

@enisoc enisoc changed the title [WIP] Support Percona Server Add Docker images for Percona Server. Mar 4, 2016
@enisoc
Copy link
Copy Markdown
Contributor Author

enisoc commented Mar 4, 2016

I've cleaned it up to re-use the MySQL56 flavor since it's supposed to be compatible. PTAL.

$ go run test.go -flavor=percona
35 PASSED, 0 FLAKY, 0 FAILED, 0 SKIPPED

@michael-berlin
Copy link
Copy Markdown
Contributor

LGTM


Review status: 2 of 6 files reviewed at latest revision, 10 unresolved discussions.


Comments from the review on Reviewable.io

Approved with PullApprove

@michael-berlin
Copy link
Copy Markdown
Contributor

Reviewed 15 of 15 files at r2.
Review status: all files reviewed at latest revision, 10 unresolved discussions.


Comments from the review on Reviewable.io

@alainjobart
Copy link
Copy Markdown
Contributor

LGTM cool!

Approved with PullApprove

enisoc pushed a commit that referenced this pull request Mar 5, 2016
Add Docker images for Percona Server.
@enisoc enisoc merged commit cde25b0 into vitessio:master Mar 5, 2016
@enisoc enisoc deleted the percona branch March 5, 2016 00:26
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