upload status page should show nicknames

[warner]

drewp suggests that the upload status page (i.e. /status/up-0) should show server nicknames, if available, in addition to the cryptic base32 nodeids.

[akp]

Here's a patch to add server nicknames to all status pages. I also needed to slightly change the history structure otherwise I couldn't get uploads to display in the history. This is my first patch and I'm not a python expert... hope someone can review.

[akp]

patch to add nicknames to status pages (fixed to pass unit tests)

[warner]

Some thoughts:

• beware of script injections when rendering the nicknames: they must be escaped
• creating the Uploader with a history= object (instead of passing one in to each upload() call) seems reasonable, although in the long run I think we'd like to get rid of the Uploader. I don't know what will replace it yet, though, so Uploader(history=) for now seems fine.
• UnlinkedStorageBroker is a bit confusing, it sounds like it involves "unlinked files". Maybe "Assisted Storage Broker"? But actually, clients who are using a Helper may know about those servers anyways, so I think they should probably use their own storagebroker instead of stubbing out the nicknames. If we do stub out the names, the stubs should make that obvious (i.e. "UNKNOWN").
• overall I'd like to find a way to make the patch a bit less.. invasive, I guess. It's unfortunate that so many of the web/status.py constructors have to be touched. Two other approaches that come to mind, not sure if either one is feasible and/or any better:
  • use the self.site.remember(obj, INTERFACE) mechanism to attach an object to each Request. If this works (and I remember having problems with it in the past), then we could pass the Client, the History object, and probably the Storage Broker too, all passed sort of ambiently along with the Request, rather than explicitly down through every single constructor. OTOH, POLA might be achieved better with explicit constructors.
  • move responsibility for nickname-association over to the Uploader: it has the storagebroker already. so it could extract a nickname early and stash it in the record.

In the longer run, I think I might want the History record to point to a StorageClient instance, rather than merely its nodeid, in which case web/status.py would just ask it for an ID or a nickname directly.

[akp]

Appreciate the review, thank you. I hope to take another look at this over the weekend.

Replying to warner:

Some thoughts:

• beware of script injections when rendering the nicknames: they must be escaped

Excellent point, thanks.

• creating the Uploader with a history= object (instead of passing one in to each upload() call) seems reasonable, although in the long run I think we'd like to get rid of the Uploader. I don't know what will replace it yet, though, so Uploader(history=) for now seems fine.
• UnlinkedStorageBroker is a bit confusing, it sounds like it involves "unlinked files". Maybe "Assisted Storage Broker"? But actually, clients who are using a Helper may know about those servers anyways, so I think they should probably use their own storagebroker instead of stubbing out the nicknames. If we do stub out the names, the stubs should make that obvious (i.e. "UNKNOWN").

OK, I completely misunderstood the purpose of the Unlinked* stuff. I thought it was just for unit testing purposes. Now I'm reading it over again, I realize that's not the case. It's still not completely clear to me what it's for, but I will look at that code again.

• overall I'd like to find a way to make the patch a bit less.. invasive, I guess. It's unfortunate that so many of the web/status.py constructors have to be touched. Two other approaches that come to mind, not sure if either one is feasible and/or any better:
  • use the self.site.remember(obj, INTERFACE) mechanism to attach an object to each Request. If this works (and I remember having problems with it in the past), then we could pass the Client, the History object, and probably the Storage Broker too, all passed sort of ambiently along with the Request, rather than explicitly down through every single constructor. OTOH, POLA might be achieved better with explicit constructors.
  • move responsibility for nickname-association over to the Uploader: it has the storagebroker already. so it could extract a nickname early and stash it in the record.

In the longer run, I think I might want the History record to point to a StorageClient instance, rather than merely its nodeid, in which case web/status.py would just ask it for an ID or a nickname directly.

Let me take another look at those approaches. Thanks!

[zooko]

akp: thank you for working on this. Uploading a file unlinked means the file is accessible only from its capability (which is contained in its URL). Uploading it linked is equivalent to uploading it unlinked and then making a link under some name in some directory which points to it.

I'm adding the ostrom tag, about which see the mailing list.