indygreg accepted this revision.
indygreg added a comment.
This revision is now accepted and ready to land.
OK. I applied this series locally and looked at the final code.
There are some minor issues with the current series. But I can fix those in
flight.
I've also captured a number of other issues in the review of this commit. IMO
they can be addressed as follow-ups.
A larger issue that I think may be worth pursuing is dropping the code to
support pre-bundle2. I think it is reasonable for a server operator to say that
a repo using this feature requires bundle2. Because attempting to handle e.g.
bookmark or phases updates pre-bundle2 is a recipe for disaster since these
updates are performed on a separate wire protocol command, after changegroup
data is exchanged. Clients pushing without bundle2 may see errors on `pushkey`
operations because nodes aren't readily available. And, having a server update
bookmarks, phases, etc in multiple bundles isn't reasonable.
I would make it so enabling this extension also implies
`server.bundle1=false`.
INLINE COMMENTS
> README:1
> +## What is it?
> +
I think we can delete this file since it is redundant with info in the
docstring of the extension.
> __init__.py:1409-1428
> +def _asyncsavemetadata(root, nodes):
> + '''starts a separate process that fills metadata for the nodes
> +
> + This function creates a separate process and doesn't wait for it's
> + completion. This was done to avoid slowing down pushes
> + '''
> +
This code can be deleted since `hg debugfilleinfinitepushmetadata` was deleted.
> indexapi.py:10
> +
> +class indexapi(object):
> + """Class that manages access to infinitepush index.
We'll want to port this to `zope.interface`.
> sqlindexapi.py:15
> +import warnings
> +import mysql.connector
> +
I think we should rename this module since it is MySQL specific.
Various database packages do conform to a similar API. So we could potentially
make the SQL layer implementation agnostic by coding to an interface.
> sqlindexapi.py:64
> + self.sqlconn = mysql.connector.connect(
> + force_ipv6=True, **self.sqlargs)
> +
`force_ipv6=True`. Kudos to Facebook for getting away with that in their
network. But this won't fly in the real world.
> sqlindexapi.py:84-85
> + self.sqlcursor = self.sqlconn.cursor()
> + self.sqlcursor.execute("SET wait_timeout=%s" % waittimeout)
> + self.sqlcursor.execute("SET innodb_lock_wait_timeout=%s" %
> + self._locktimeout)
And here we have some MySQL specific functionality :/
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D2096
To: pulkit, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
