Page MenuHomePhabricator

httpconnection: allow `httpsendfile` subclasses to suppress the progressbar
ClosedPublic

Authored by mharbison72 on Jan 21 2020, 1:37 PM.

Details

Summary

This will be neccessary for LFS, which manages the progressbar outside of the
file.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

mharbison72 created this revision.Jan 21 2020, 1:37 PM
marmoute added a subscriber: marmoute.EditedFeb 5 2020, 6:13 PM

This is very unappealling ☹ could we update the total value in case authentication is needed instead ?

(also, the double sending is very sad, could we have a small wireprotocol command dedicated at testing authentication first instead ?)

This is very unappealling ☹ could we update the total value in case authentication is needed instead ?

Sorry, I'm not sure what you mean. The progressbar is per-file in the existing uses. With LFS, the worker pool drives the bus and just keeps bumping the completed amount as workers finish.

This is also a bit different for LFS because LFS sends a small text volley to the server first. That triggers the authentication before sending everything, and then the auth hash is carried in the headers of each blob. (It might seem a bit weird that the bug is fixed by reusing code that works around the auth issue. I can't explain it either; that's why I'm hoping there's somebody with a deep knowledge of how all of this URL handling and keepalive works.)

(also, the double sending is very sad, could we have a small wireprotocol command dedicated at testing authentication first instead ?)

Presumably, but IDK anything about the wire protocol, and the existing code would still be needed for the case where the server doesn't know about the command.

(also, the double sending is very sad, could we have a small wireprotocol command dedicated at testing authentication first instead ?)

Presumably, but IDK anything about the wire protocol, and the existing code would still be needed for the case where the server doesn't know about the command.

I looked into that for the core wireproto years ago. The summary is basically "no, httplib is a very simple http client and not readily extended." If you want to do better, you're in the bad place of doing something like using libcurl in hg, which will be very complicated. :(

This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.

I forgot to reply to this last week- I was concerned when the subsequent part of the stack went in without suppressing the per file progress bars here, that the displayed progress bar would be bouncing around randomly. But it wasn't- it was ever increasing like only the main bar was being changed. Any ideas why that would be?

I forgot to reply to this last week- I was concerned when the subsequent part of the stack went in without suppressing the per file progress bars here, that the displayed progress bar would be bouncing around randomly. But it wasn't- it was ever increasing like only the main bar was being changed. Any ideas why that would be?

Progress bars have some stickiness and won't show brief sub-topics. It's all pretty configurable, but I think the default is we don't show a nested topic if it exists for less than 2 seconds of ticks of the upper level progress bar.