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 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
