classification
Title: urllib.retrieve's reporthook called with non-helpful value
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: orsenthil Nosy List: ajaksu2, akira, iritkatriel, nelchael, orsenthil, sidnei, zanella
Priority: normal Keywords: easy, patch

Created on 2006-05-18 13:22 by sidnei, last changed 2020-10-26 08:00 by iritkatriel.

Files
File name Uploaded Description Edit
progresshook.diff nelchael, 2008-12-07 21:10 Patch adding progresshook review
test_urllib.patch zanella, 2011-04-24 22:45 review
Messages (7)
msg60914 - (view) Author: Sidnei da Silva (sidnei) Date: 2006-05-18 13:22
Seems like the 'reporthook', when provided to the
'retrieve' function of urllib will be called with
(blocknumber, blocksize, totalsize).

However, the call to read() might return *less* than
'blocksize' bytes (I believe), so it should imho be
called with len(block) as the second argument instead.
Or maybe add a fourth argument 'readbytes'.
msg77257 - (view) Author: Krzysztof Pawlik (nelchael) Date: 2008-12-07 21:10
Attached patch adds new argument: progresshook - it will be passed two
arguments: count of downloaded bytes and total file size (or -1 if it's
not available).

Introducing new argument instead of modifying reporthook maintains
backwards compatibility, also allows removal of reporthook at one point
in the future.

This patch is against r30 SVN tag.
msg81786 - (view) Author: Daniel Diniz (ajaksu2) (Python triager) Date: 2009-02-12 17:44
Patch has docs, tests needed.
msg134357 - (view) Author: Rafael Zanella (zanella) Date: 2011-04-24 22:45
Simple (lazy) test case added.

It just replicates one test case of reporthook to work with progresshook.

The testcases assume the hard-coded value of blocksize on urllib, maybe it should become a public property.

Also commented on diff: http://bugs.python.org/review/1490929/show
msg174870 - (view) Author: Akira Li (akira) * Date: 2012-11-05 07:17
Related issue 16409
msg379618 - (view) Author: Irit Katriel (iritkatriel) * (Python triager) Date: 2020-10-25 23:21
The code in urllib is different now but I see that a test very similar to the one in the patch exists, so was this fixed?

https://github.com/python/cpython/blob/e68c67805e6a4c4ec80bea64be0e8373cc02d322/Lib/test/test_urllib.py#L813
msg379649 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2020-10-26 07:58
> so was this fixed?

Irit, not really. This is adding another hook called "progress hook" in addition to the "report hook". We need to evaluate if this is really required.
History
Date User Action Args
2020-10-26 08:00:19iritkatrielsetstage: test needed ->
versions: + Python 3.8, Python 3.9, Python 3.10, - Python 3.2
2020-10-26 07:58:27orsenthilsetstatus: pending -> open

messages: + msg379649
2020-10-25 23:21:10iritkatrielsetstatus: open -> pending
nosy: + iritkatriel
messages: + msg379618

2012-11-05 07:17:25akirasetnosy: + akira
messages: + msg174870
2011-04-24 22:45:45zanellasetfiles: + test_urllib.patch
nosy: + zanella
messages: + msg134357

2010-08-24 03:18:26orsenthilsetassignee: orsenthil
2010-08-22 10:20:13BreamoreBoysetversions: + Python 3.2, - Python 2.7
2009-04-22 18:48:49ajaksu2setkeywords: + easy
2009-02-12 17:44:18ajaksu2setnosy: + ajaksu2, orsenthil
stage: test needed
messages: + msg81786
versions: + Python 2.7
2008-12-07 21:10:23nelchaelsetfiles: + progresshook.diff
keywords: + patch
messages: + msg77257
nosy: + nelchael
2006-05-18 13:22:13sidneicreate