Merge lp:~percona-toolkit-dev/percona-toolkit/fix-950294-ptc-if-not-exists into lp:percona-toolkit/2.1
Proposed by
Brian Fraser
Status: | Merged |
---|---|
Approved by: | Daniel Nichter |
Approved revision: | 333 |
Merged at revision: | 335 |
Proposed branch: | lp:~percona-toolkit-dev/percona-toolkit/fix-950294-ptc-if-not-exists |
Merge into: | lp:percona-toolkit/2.1 |
Diff against target: |
242 lines (+143/-18) (has conflicts) 2 files modified
bin/pt-table-checksum (+63/-15) t/pt-table-checksum/privs.t (+80/-3) Text conflict in bin/pt-table-checksum Text conflict in t/pt-table-checksum/privs.t |
To merge this branch: | bzr merge lp:~percona-toolkit-dev/percona-toolkit/fix-950294-ptc-if-not-exists |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Daniel Nichter | Approve | ||
Review via email: mp+117303@code.launchpad.net |
To post a comment you must log in.
1) "--no-create- replicate- table was passed in." -> ""--no- create- replicate- table was specified."
2)
29 + print STDERR "CREATE DATABASE IF NOT EXISTS $db failed, but the "
30 + . "db already exists. This is harmless unless the "
31 + . "slaves don't have the database. $EVAL_ERROR"; # kb link?
I think we need to make this fatal because we must avoid breaking replication. So:
die "Error executing $sql: $EVAL_ERROR\nThe database exists on the master, but replication will break if it does not also exist on all replicas. If the --replicate database and table already exist on the master and all replicas, or if you create them manually, then specify --no-create- replicate- table to avoid this error."
How does that ^ sound? Better to force the user to think/do a little manual work than break their replication.
3) 49 +<<<<<<< TREE -- why is there a merge conflict?
4) 63 + . "--no-create- replicate- table was passed in. " -- "passed id" -> "specified"
5) 73 + print STDERR "CREATE TABLE IF NOT EXISTS $args{repl_table} " -- Same as above with the db: make fatal, change error message, etc.
6) 142 +isnt($exit_status, 0, "");
i) Let's never use isnt() because 1) it amounts to "yes we have no bananas" ("yes, $got is not $bar") and 2) it's dangerously close to isn't() which is just crazy.
ii) Test has no name.