Code review comment for lp:~stub/launchpad/replication

merge-conditional, so that you can get ahead with that when you get up ... ;-)

Thank you for improving this. I just have two improvements that I'd like to have in there.

> + # Get the connection string for masters.
> + lpmain_connection_string = get_master_connection_string(
> + source_connection, parser, 1) or source_connection_string
> + authdb_connection_string = get_master_connection_string(
> + source_connection, parser, 2) or source_connection_string

Please define constants for "1" and "2", e.g. "LPMAIN_SET_ID = 1".

> + # Duplicate the authdb schema.
> + log.info("Duplicating authdb schema from '%s' to '%s'" % (
> + authdb_connection_string, target_connection_string))
> + table_args = ["--table=%s" % table for table in _auth_store_tables]
> + # We need to restore the two cross-replication-set views that where
> + # dropped as a side effect of dropping the auth store tables.
> + table_args.append("--table=ValidPersonCache")
> + table_args.append("--table=ValidPersonOrTeamCache")

Please list these two tables in a list, close to where you define _auth_store_tables. This keeps this information from being scattered throughout the code. You can then create table_args directly from the two lists, like this:

table_args = ["--table=%s" % table for table in _auth_store_tables + whatever_you_call_them_tables]

BTW, since _auth_store_tables is imported from somewhere else, the leading _ is missleading, isn't it?

Cheers,
Henning