Opened at 2010-08-12T20:47:42Z
Closed at 2010-09-02T16:36:07Z
#1172 closed defect (fixed)
active immutable downloads are shown on Recent Operations instead of Active Operations
Reported by: | zooko | Owned by: | francois |
---|---|---|---|
Priority: | major | Milestone: | 1.8.0 |
Component: | code-frontend-web | Version: | 1.8β |
Keywords: | download immutable usability easy review-needed | Cc: | |
Launchpad Bug: |
Description
An immutable download is still in-progress, but it appears in Recent Operations instead of Active Operations. By the way, the source code responsible for this may be close to the source code responsible for #1079 (upload of file into dir doesn't appear on Recent Uploads and Downloads).
Attachments (3)
Change History (18)
Changed at 2010-08-12T20:47:59Z by zooko
comment:1 Changed at 2010-08-14T07:05:58Z by zooko
- Milestone changed from undecided to 1.8.0
comment:2 follow-up: ↓ 6 Changed at 2010-08-14T18:30:56Z by warner
The fix for this will go into source:src/allmydata/immutable/downloader/status.py#L244 ish (DownloadStatus.get_active). It currently is a stub that always returns False. That should be replaced by something which, I dunno, probably does the same "look for read_events that have a non-None finishtime" check that get_progress uses, so "active" means "has outstanding read() calls".
comment:3 Changed at 2010-08-14T18:31:10Z by warner
- Keywords easy added
comment:4 Changed at 2010-08-31T10:43:06Z by francois
- Owner set to francois
- Status changed from new to assigned
Changed at 2010-08-31T11:08:32Z by francois
comment:5 Changed at 2010-08-31T11:10:48Z by francois
- Keywords review-needed added; unfinished-business removed
- Owner changed from francois to somebody
- Status changed from assigned to new
comment:6 in reply to: ↑ 2 Changed at 2010-08-31T11:12:57Z by francois
The attached patch implements what Brian proposed in comment:2 and add an appropriate test.
comment:7 follow-up: ↓ 11 Changed at 2010-08-31T21:15:23Z by warner
I'm ok with this patch. Be aware that if we land the #1170 patches (specifically 1170-p2.diff) then this one will need to be rewritten, since the download-status data structure changed quite a bit (each event is now an object with attributes, rather than a tuple).
comment:8 Changed at 2010-08-31T23:47:27Z by zooko
François, could you rewrite your patch to be based on top of Brian's #1170 patches? :-)
comment:9 Changed at 2010-09-02T05:41:14Z by zooko
- Owner changed from somebody to francois
comment:10 Changed at 2010-09-02T05:51:10Z by zooko
Okay I have applied two out of three of Brian's patches, namely c89a464510394089 and 00e9e4e6760021a1. 00e9e4e6760021a1 is the one that was formerly known as 1170-p2.diff.
comment:11 in reply to: ↑ 7 Changed at 2010-09-02T10:49:03Z by francois
Replying to warner:
I'm ok with this patch. Be aware that if we land the #1170 patches (specifically 1170-p2.diff) then this one will need to be rewritten, since the download-status data structure changed quite a bit (each event is now an object with attributes, rather than a tuple).
Well, the download-status data structure are actually changed by 1170-p3.diff which is not yet commited in trunk. Anyway, here's an up to date version of the patch.
Changed at 2010-09-02T10:49:16Z by francois
comment:12 Changed at 2010-09-02T14:58:08Z by francois@…
- Resolution set to fixed
- Status changed from new to closed
In 485bfc0fd67ef421:
comment:13 Changed at 2010-09-02T15:56:11Z by zooko
- Resolution fixed deleted
- Status changed from closed to reopened
Oh look this failed unit tests:
[ERROR]: allmydata.test.test_download.Status.test_active Traceback (most recent call last): File "/home/buildbot/tahoe-lafs/Kyle OpenBSD-4.6 amd64/build/src/allmydata/test/test_download.py", line 1267, in test_active self.failUnlessEqual(ds.get_active(), True) File "/home/buildbot/tahoe-lafs/Kyle OpenBSD-4.6 amd64/build/src/allmydata/immutable/downloader/status.py", line 210, in get_active if r_ev["finish_time"] is None: exceptions.TypeError: tuple indices must be integers, not str
comment:14 Changed at 2010-09-02T16:20:41Z by zooko
Oh this was my mistake, I should have committed attachment:downloader-status-1172.dpatch, not attachment:downloader-status-1172-2.dpatch . I'll commit a new patch that undoes the effect of attachment:downloader-status-1172-2.dpatch and then another patch that has the same effect as attachment:downloader-status-1172.dpatch. By the way, one thing you can do to make it harder for me to make such mistakes is to use the --ask-deps option to darcs record and specify that a patch depends on another patch. Then when I try to commit your patch, darcs will tell me that I can't do so until I commit that other patch, which will make me realize that I'm trying to commit the wrong patch.
Thanks!
comment:15 Changed at 2010-09-02T16:36:07Z by zooko@…
- Resolution set to fixed
- Status changed from reopened to closed
In 63fb687a440e99a9:
(The changeset message doesn't reference this ticket)
Should we try to fix this for v1.8.0? I would lean toward "yes".