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