#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)

Tahoe-LAFS - Current Uploads_Downloads.html (2.3 KB) - added by zooko at 2010-08-12T20:47:59Z.
downloader-status-1172.dpatch (2.8 KB) - added by francois at 2010-08-31T11:08:32Z.
downloader-status-1172-2.dpatch (4.5 KB) - added by francois at 2010-09-02T10:49:16Z.

Download all attachments as: .zip

Change History (18)

comment:1 Changed at 2010-08-14T07:05:58Z by zooko

  • Milestone changed from undecided to 1.8.0

Should we try to fix this for v1.8.0? I would lean toward "yes".

comment:2 follow-up: 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: 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.

Last edited at 2010-09-02T10:49:51Z by francois (previous) (diff)

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:

DownloadStatus?: show active immutable downloads in Active Operations, Fix #1172

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:

http://tahoe-lafs.org/buildbot/builders/Kyle%20OpenBSD-4.6%20amd64/builds/390/steps/test-coverage/logs/stdio

[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!

Last edited at 2010-09-02T16:31:09Z by zooko (previous) (diff)

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)

Note: See TracTickets for help on using tickets.