include full docker progress events in push progress events #727
Conversation
| ) | ||
| last_emit_time = time.time() | ||
| for chunk in client.push(self.output_image_spec, stream=True): | ||
| for line in chunk.splitlines(): |
There was a problem hiding this comment.
You might be wondering why this extra iteration was added. When I tested this locally, each chunk could be multiple JSON lines, split by \r\n, so the json.loads would fail. So I added splitting lines to the iteration. I'm not sure why this hasn't come up on mybinder.org. I'm using docker 3.7.1
There was a problem hiding this comment.
Should we add this comment as a comment to the code? I'd vote yes
Most useful is probably progressDetail.progress which is the docker rendered
Example event:
```json
"layers": {
"25b373f5f7f1": {
"status": "Waiting",
"progressDetail": {},
"id": "25b373f5f7f1"
},
"e9f2b5eca21e": {
"status": "Pushing",
"progressDetail": {
"current": 747589632,
"total": 758965327
},
"progress": "[=================================================> ] 747.6MB/759MB",
"id": "e9f2b5eca21e"
},
"7753d7e0913b": {
"status": "Pushed",
"progressDetail": {},
"id": "7753d7e0913b"
},
"ed03b06eb165": {
"status": "Pushed",
"progressDetail": {},
"id": "ed03b06eb165"
},
"01cb88e6c1af": {
"status": "Pushed",
"progressDetail": {},
"id": "01cb88e6c1af"
},
"adda4de99b3d": {
"status": "Mounted from library/buildpack-deps",
"progressDetail": {},
"id": "adda4de99b3d"
},
"3a034154b7b6": {
"status": "Mounted from library/buildpack-deps",
"progressDetail": {},
"id": "3a034154b7b6"
}
```
|
This looks nice. To display this in BinderHub we will need to add a bit of logic to the JS that displays this to include the right magic/invisible characters to rewind the cursor position before writing the progress info again right? |
Yeah, if we want the progress bars to clear nicely, we'll need to do that. We can also start with a more crude version that prints a static view over and over again (updates are limited to 1.5s, so this wouldn't be a flood) and refine it to update in-place later. |
include full docker progress events in push progress events
for backward-compatibility, add a
layerskey with the full data,preserving the
progressfield, which has only partial data, depending on state of each layer.The most useful field, I think, is
progress, which shows the docker cli-rendered progress bar.Strangely, when I sampled the log data we get from ovh, current and total in progressDetail (which we already publish under 'progress') were on totally different scales,
so rendering our own progress bars with
$current/$totaldoesn't seem trustworthy. Not sure what's up with that.Sample log data:
Related to jupyterhub/binderhub#852 . Passing through the
progressfields might be the easiest implementation.