Arvados

commit 64027668d7b5bd47a863c86277c83961d8a29914

Implemented the items listed in the description. I chose option three and added a by_pdh optional argument to MagicDirectory since method overloading seemed to be the most straightforward and non-intrusive.

I spent a couple hours trying to see how I may write a test to pass by_pdh instead of by_id to the MagicDirectory based tests. I could not figure out where to improve existing test implementation to do this. So, did not add any tests for this.

In MagicDirectory, suggest calling it pdh_only instead of by_pdh.

This statement:

        if self.by_pdh and uuid_pattern.match(k):
            raise llfuse.FUSEError(errno.ENOENT)

should be folded into the existing conditional. Also it should return False, not raise an exception:

         if not portable_data_hash_pattern.match(k) and (self.pdh_only or not uuid_pattern.match(k)):
             return False

I don't think it's necessary or desirable to modify the behavior of arv-mount --all based on --by-pdh. This means you can revert the changes to the text of --all (line 35) and behavior of e._entries['by_id'] (line 135).

For testing, suggest starting from FuseMagicTest

1. make_mount takes keyword arguments, which are passed through, so you can write tests for each of make_mount(fuse.MagicDirectory, by_pdh=True) and make_mount(fuse.MagicDirectory, by_pdh=False)
2. self.api.collections().create()... should return an API object with a UUID of the collection
3. When by_pdh is False, test that the collection is accessible by both the PDH and the UUID
4. When by_pdh is True, perform the same test except check that the collection is only accessable by PDH and fails when accessing by UUID

• I figured out why FuseMagicTestPDHOnly is failing.

Line 514 of fusedir.py:

                self._entries['by_id'] = self.inodes.add_entry(MagicDirectory(
                        self.inode, self.inodes, self.api, self.num_retries))

You need to add self.pdh_only:

                self._entries['by_id'] = self.inodes.add_entry(MagicDirectory(
                        self.inode, self.inodes, self.api, self.num_retries, self.pdh_only))

• Can you add the same access-by-UUID test to FuseMagicTest? Except it should succeed, of course.

• tests.test_mount.MagicDirApiError is failing for me:
  

  ERROR: runTest (tests.test_mount.MagicDirApiError)
  ----------------------------------------------------------------------
  Traceback (most recent call last):
    File "/home/peter/work/arvados/services/fuse/tests/test_mount.py", line 1030, in runTest
      llfuse.listdir(os.path.join(self.mounttmp, self.testcollection))
    File "llfuse/fuse_api.pxi", line 43, in llfuse.capi.listdir (src/llfuse/capi_linux.c:22621)
  OSError: [Errno 2] No such file or directory: '/tmp/tmpBpoluv/97d180c4f916faf61fb3d64aa2263961+52'
  

tests.test_mount.MagicDirApiError seems to be failing in master as well for me, so it is likely unrelated to this change.

Thanks for the help with line 514 of fusedir.py. The test is now working.

I also added the other test variations (by_id, pdh_only=False) etc that I was holding back until the main test passed.

All fuse tests are passing for me.

768f114b1746dddec0f0224dee92a58c8ddc5499 Looks good to me. Please merge.

This change appears to be causing a problem for me - I appear to have a crunch-job that passes `--by-pdh` to arv-mount, but my arv-mount does not support that option so it exits with an error. I guess this change requires that crunch-job and arv-mount be upgraded in sync. I think in my case arv-mount is coming from the Ubuntu apt packages, and I can confirm that now that I've updated python-arvados-fuse on all nodes, this issue appears to have been resolved.

Sorry for the trouble, Josh. Getting cross-component dependencies declared correctly is always a challenge. I should've given you a heads-up about that when we discussed upgrading yesterday.