Idea #2035
closedarv-mount exposes filesystem paths like /tag/tagname/collection-uuid and /folder/foldername/collection-uuid
Updated by Tom Clegg almost 11 years ago
- Target version set to 2014-03-05 Data management
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.
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
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.
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
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
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.
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 Pythonint('')
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.
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
.
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.
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.