Story #8784
closed
[Workbench] Use keep-web to generate directory listings
100%
Description
Human- and machine-readable directory listings would allow browsing collections without involving Workbench.
TBD: Bookmarks to non-public collections in keep-web come with a bit of a snag, which auto-indexing could turn into a bigger practical problem. If you click a bookmark/permalink instead of going through Workbench's links/redirector, and you don't already have an active cookie from previously clicking the Workbench links, there's no option to log in -- you just get an error. You have to recognize the error yourself, find the collection in Workbench, and follow a link from there.
(Features copied from #5824's "nice to have" section)- Very basic formatting (bootstrap would be nice but can wait)
- OK if we show a directory listing only at the top level, similar to the way Workbench does it.
- Explicitly no index.html in place of directory listings.
- This means "wget -r" can always be expected to work, but it also ensures this can't be used to host entire web sites along the lines of github pages. If we add this capability in the future we could add a -render-index flag. This would make more sense if it came with a bunch of other features anyway, like dynamically mapping vhosts to collections.
A complementary piece to this is having Workbench generate sharing links which point directly to keep-web rather than redirecting through Workbench.
This is in support of a request to be able to share data without exposing the Workbench host. https://support.curoverse.com/rt/Ticket/Display.html?id=364
Files
Updated by Brett Smith over 8 years ago
- Target version set to Arvados Future Sprints
Updated by Peter Amstutz over 7 years ago
Related to this, workbench sharing links should link directly to the keep-web index page.
Updated by Tom Morris over 7 years ago
- Target version changed from Arvados Future Sprints to 2017-06-21 sprint
Updated by Tom Morris over 7 years ago
- Description updated (diff)
- Story points set to 2.0
Updated by Tom Clegg over 7 years ago
- File index page 0e3369b.png added
- File index page 2dc0e00.png added
Updated by Tom Clegg over 7 years ago
- File index.page.2dc0e00.png index.page.2dc0e00.png added
- File index.page.0e3369b.png index.page.0e3369b.png added
Existing Workbench index page
¶
new keep-web index page @ 2dc0e00
¶
TODO:
- use new SDK to load into collection record and cache that, instead of loading/caching a map[string]interface and copying to a Collection in each request
- (separate branch) redirect Workbench index page URL to keep-web, and use keep-web directly when generating new links
- monospace font change a good idea?
- dropping the hierarchy a good idea? if so, maybe only the name should be hotlinked, not the whole path?
- rename (*Collection)FileSystem() to ROFileSystem() or HTTPFileSystem() or Snapshot()?
Updated by Tom Clegg over 7 years ago
- File index.page.a42cb73.png index.page.a42cb73.png added
Updated by Tom Clegg over 7 years ago
8784-dir-listings @ abf007273ba68c2eb541763e40b19d1703132685
Updated by Tom Clegg over 7 years ago
- use new SDK to load into collection record and cache that, instead of loading/caching a map[string]interface and copying to a Collection in each request
Updated by Lucas Di Pentima over 7 years ago
Sorry for the delay, here are some questions:
- File
sdk/go/arvados/collection_fs.go- Line 60, 68: Shouldn’t
retbe checked if it’s nil before returning? As I read on File’s Readdir doc, if returning an empty slice, it should return a non-nil error. - Line 189: I suppose that empty conditional should be removed, right?
- Line 60, 68: Shouldn’t
- File
services/keep-web/handler.go- Line 168: Shouldn't that comment line be updated to something like “
/c=ID[/PATH…]” to stay in sync with the code? - Lines 323-335: It’s a little difficult (at least for me) to follow that conditionals chain, could you add some comments? I’m specially interested on L329, but in general I feel that for an Open() operation, those are lots of checks, it would be nice to have comments.
- Line 402: Couldn’t
colllectionbe retrieved fromfsdoing type assertion? (to avoid passing it as an argument)
- Line 168: Shouldn't that comment line be updated to something like “
- Don’t understand why, but running local tests failed with this:
# git.curoverse.com/arvados.git/services/keep-web tmp/GOPATH/src/git.curoverse.com/arvados.git/services/keep-web/handler.go:451: undefined: sort.Slice
- If a collection is empty, is it possible to show some legend stating that fact instead of using the title “File Listing”?
Updated by Tom Clegg over 7 years ago
Lucas Di Pentima wrote:
Sorry for the delay, here are some questions:
- File
sdk/go/arvados/collection_fs.go
- Line 60, 68: Shouldn’t
retbe checked if it’s nil before returning? As I read on File’s Readdir doc, if returning an empty slice, it should return a non-nil error.
Yes, updated. If count>0 then an empty slice has to come with an error.
- Line 189: I suppose that empty conditional should be removed, right?
Whoops. That can't happen: we would have returned it as a file earlier. Removed.
- File
services/keep-web/handler.go
- Line 168: Shouldn't that comment line be updated to something like “
/c=ID[/PATH…]” to stay in sync with the code?
Updated.
- Lines 323-335: It’s a little difficult (at least for me) to follow that conditionals chain, could you add some comments? I’m specially interested on L329, but in general I feel that for an Open() operation, those are lots of checks, it would be nice to have comments.
Added comments to the error cases.
- Line 402: Couldn’t
colllectionbe retrieved fromfsdoing type assertion? (to avoid passing it as an argument)
I suppose... turns out the only thing we need from the collection is its name anyway, so I changed that arg to "collectionName string".
- Don’t understand why, but running local tests failed with this:
[...]
Ah, it seems that was introduced in Go 1.8 -- I thought sorting seemed easier than last time! Updated run-tests.sh and packaging scripts accordingly.
- If a collection is empty, is it possible to show some legend stating that fact instead of using the title “File Listing”?
Done.
8784-dir-listings @ f77d63e6cfaf7278c1cb0fb05e5a4e3f45320e3a
Updated by Tom Clegg over 7 years ago
- Workbench redirects to keep-web (if available) instead of serving its own directory listing page.
- passed https://ci.curoverse.com/job/developer-run-tests/342/
Updated by Tom Clegg over 7 years ago
- Subject changed from [Keep-web] Generate directory listings to [Workbench] Use keep-web to generate directory listings
- Story points changed from 2.0 to 0.5
Updated by Tom Clegg over 7 years ago
- Target version changed from 2017-06-21 sprint to 2017-07-05 sprint
Updated by Tom Clegg over 7 years ago
8784-dir-listings @ ff723f5f08e286df4c4c58a27a9db574ca604a6f -- passed https://ci.curoverse.com/job/developer-run-tests/356/
Updated by Peter Amstutz over 7 years ago
What's the rationale for using query_token instead of path_token?
Updated by Tom Clegg over 7 years ago
Peter Amstutz wrote:
What's the rationale for using query_token instead of path_token?
None -- changed to path_token.
Also added a fix to make the "upload / network error" integration test pass with the latest Firefox.
8784-dir-listings @ e59c1d365d9b6e1eff9b5cb030a8b1a3aaf14353
Updated by Peter Amstutz over 7 years ago
Updated by Tom Clegg over 7 years ago
- Status changed from In Progress to Resolved