Project

General

Profile

Actions

Idea #2035

closed

arv-mount exposes filesystem paths like /tag/tagname/collection-uuid and /folder/foldername/collection-uuid

Added by Tom Clegg almost 11 years ago. Updated over 10 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
-
Start date:
04/29/2014
Due date:
Story points:
3.0
Release relationship:
Auto

Subtasks 5 (0 open5 closed)

Task #2709: Review 2035-arv-mount-tags-folders at revision b4a9aaaResolvedPeter Amstutz04/29/2014Actions
Task #2742: Review 2035-arv-mount-tags-foldersResolvedPeter Amstutz04/29/2014Actions
Task #2607: arv-mount exposes tags at {mountpoint}/tag/{tag_text}ResolvedPeter Amstutz05/02/2014Actions
Task #2623: Add 'select' and 'distinct' to API server, required to get request a list of distinct tagsResolvedPeter Amstutz04/29/2014Actions
Task #2606: arv-mount exposes different object types at {mountpoint}/{objecttype}/{uuid}ResolvedPeter Amstutz05/06/2014Actions
Actions #1

Updated by Tom Clegg almost 11 years ago

  • Target version set to 2014-03-05 Data management
Actions #2

Updated by Tom Clegg almost 11 years ago

  • Story points set to 2.0
Actions #3

Updated by Tom Clegg over 10 years ago

  • Subject changed from Keep's POSIX interface can give access to collections via names and tags, not just via manifest hashes. to Keep's POSIX interface can give access to collections via names, not just via manifest hashes.
Actions #4

Updated by Tom Clegg over 10 years ago

  • Target version changed from 2014-03-05 Data management to 2014-05-07 Storing and Organizing Data
Actions #5

Updated by Tom Clegg over 10 years ago

  • Subject changed from Keep's POSIX interface can give access to collections via names, not just via manifest hashes. to Keep's POSIX interface can give access to collections via projects, not just manifest hashes.
Actions #6

Updated by Tom Clegg over 10 years ago

  • Subject changed from Keep's POSIX interface can give access to collections via projects, not just manifest hashes. to arv-mount exposes filesystem paths like /tag/tagname/collection-uuid and /folder/foldername/collection-uuid
Actions #7

Updated by Peter Amstutz over 10 years ago

  • Assigned To set to Peter Amstutz
Actions #8

Updated by Peter Amstutz over 10 years ago

  • Story points changed from 2.0 to 3.0
Actions #9

Updated by Tom Clegg over 10 years ago

Looking at b4a9aaa

This looks like it won't play well with "attr asc" and "attr desc" forms (and seems not to have any tests): update, fixed in de54cdc

@orders.select! { |o| @select.include? o } if @select

Add a test with more "select" than "distinct" columns (or no "select" at all), and verify that the "distinct" behavior actually occurs

Mention at doc/api/methods.html

Otherwise, LGTM

Actions #10

Updated by Tom Clegg over 10 years ago

In docs: prefer "false (default)" rather than "default; false:" -- and/or mention the false case first. Currently it's easy to read as if true is the default.

Actions #11

Updated by Brett Smith over 10 years ago

Reviewing e6aa5b4. I love a well-tested branch. Thanks for building all this necessary infrastructure; I'm sure I'll be building on top of it before I know it. My comments are hardly about the code itself; more about glue.

The one thing I'd say about the updated driver is that there seems to be a lot of similar code, especially in the event bus subscriptions and invalidate methods. The merge method does a great job of abstracting away common functionality—I wonder if it'd be possible to use a similar strategy to refactor these too? But this is more along the lines of style and I don't think it needs to hold up the merge before the end of the sprint.

The setup.py templates assume that we launch it from its own directory when they rmtree. Because this is potentially destructive, I think it's worth guarding to make sure we work on the correct build directory. It also causes the script to fail if we haven't built anything yet, or didn't last time. Maybe something like:

import os
import sys
build_dir = os.path.join(os.path.dirname(sys.argv[0]), 'build')
shutil.rmtree(build_dir, ignore_errors=True)

And then a couple small things in run_test_server.py:find_server_pid:

  • Testing server_pid > 0 is extraneous. It's in the Ruby because ''.to_i == 0 (i.e., the pidfile is empty), but in Python int('') is a ValueError. I would expect the outcomes here to always be IOError, OSError, ValueError, or a good pid.
  • Naming those exceptions, or at least catching Exception, would be helpful to avoid catching low-level exceptions like KeyboardInterrupt (since it's conceivable the user might try to ^C during the loop sleep).

Thanks.

Actions #12

Updated by Brett Smith over 10 years ago

One small additional note: setup_fuse.py should be added to the Python SDK .gitignore alongside setup.py.

Actions #13

Updated by Peter Amstutz over 10 years ago

The event bus and invalidation code is more or less just the minimum required to ensure that API server updates are visible to the user. A better implementation would actually look at the event to determine what to do with it.

I applied your suggestion for selecting the correct directory, and that inspired me to refactor the setup scripts to eliminate build.sh and sed.

I applied the changes you suggested to run_test_server.py

The .gitignore comment is no longer applicable because setup.py and setup_fuse.py are no longer generated from setup.py.src and setup_fuse.py.src.

Actions #14

Updated by Brett Smith over 10 years ago

Peter Amstutz wrote:

I applied your suggestion for selecting the correct directory, and that inspired me to refactor the setup scripts to eliminate build.sh and sed.

That's even way better, thank you. This version looks good to merge to me.

Actions #15

Updated by Peter Amstutz over 10 years ago

  • Status changed from New to Resolved
Actions

Also available in: Atom PDF