D4813: narrow: the first version of narrow_widen wireprotocol command

indygreg (Gregory Szorc) phabricator at mercurial-scm.org
Wed Oct 3 14:52:00 EDT 2018


indygreg added inline comments.

INLINE COMMENTS

> narrowbundle2.py:329
>      getbundleargs['narrow'] = 'boolean'
> -    getbundleargs['widen'] = 'boolean'
>      getbundleargs['depth'] = 'plain'

Because of how `wireprotov1server.getbundle()` handles arguments, removing this argument from the definition of arguments to the `getbundle` wire protocol command means that existing clients attempting to send the argument to the `getbundle` wire protocol command will cause a server-side exception.

We need to keep this key around for BC unless we're fine breaking old clients or unless there is something that changes server capabilities in a way that prevents old clients from calling `getbundle` with this argument. We could ignore the argument or nerf the server to error if it is received.

> martinvonz wrote in narrowwirepeer.py:79-82
> That code was added in https://www.mercurial-scm.org/repo/hg/rev/3e7f675628ad. Maybe @indygreg can tell us if he thinks we should support the same values for a new wireprotocol command.

We probably want to extract the argument "parsing" from `wireprotov1server.getbundle()` into a standalone function so we can use it for this command.

Arguments in wire protocol version 1 are wonky :/

> narrowwirepeer.py:64
> +    """
> +
> +    oldincludes = wireprototypes.decodelist(args.get('oldincludes'))

I'm pretty sure we'll want a try..except around this entire command so that we'll send a bundle2 error payload on failure. See `wireprotov1server.getbundle()` for inspiration. Search for use of the `error:abort` bundle2 part. We'll want to do something like that in the except block.

Keep in mind exceptions can occur mid stream. In that case, proper error handling is a bit harder. But a good start is to put a `try..except` around all the code before we `return` the output.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D4813

To: pulkit, durin42, #hg-reviewers, martinvonz
Cc: indygreg, mercurial-devel


More information about the Mercurial-devel mailing list