D6267: incoming: detect if server send partial replies

[indygreg (Gregory Szorc)]

indygreg requested changes to this revision.
indygreg added a comment.
This revision now requires changes to proceed.


  +1 for feature. Needs some minor work to get review.

INLINE COMMENTS

> bundlerepo.py:639
>  
> +        if rheads and any(x not in localrepo for x in rheads):
> +            ui.warn(_("warning: partial reply from server\n"))

`localrepo.__contains__` can be slow in tight loops because it has to resolve `self.changelog`. Also, `localrepo.__getitem__` (which `__contains__` calls) accepts a myriad of different values (integers, hex and binary hashes, etc). We want to user a lower-level API to perform lookups. I think `localrepo.changelog.hasnode()` is what we want. Please alias that to a local to avoid the `localrepo.changelog` property resolution in a loop.

> bundlerepo.py:640
> +        if rheads and any(x not in localrepo for x in rheads):
> +            ui.warn(_("warning: partial reply from server\n"))
> +            rheads = [x for x in rheads if x in localrepo]

+1 for this feature. But the warning message is a bit ambiguous to me. How about something like `warning: server sent partial data; not all remote changesets are available`. This still isn't great. But I'm struggling to find a way to better state the issue here.

> bundlerepo.py:641
> +            ui.warn(_("warning: partial reply from server\n"))
> +            rheads = [x for x in rheads if x in localrepo]
>      csets = localrepo.changelog.findmissing(common, rheads)

Same comment as above regarding the `x in localrepo` issues.

REPOSITORY
  rHG Mercurial

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

To: joerg.sonnenberger, #hg-reviewers, indygreg
Cc: indygreg, mercurial-devel