[ARVADOS] updated: 81352ba328283d7ca2250b90bdfbaa305d5c3355

Git user git at public.curoverse.com
Tue Aug 15 10:39:42 EDT 2017


Summary of changes:
 sdk/python/arvados/commands/put.py | 36 +++---------------------------------
 sdk/python/tests/test_arv_put.py   | 22 ++++++++++++++++------
 2 files changed, 19 insertions(+), 39 deletions(-)

       via  81352ba328283d7ca2250b90bdfbaa305d5c3355 (commit)
      from  7049888ba3001baa7e6428c056f2d5d03789747a (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 81352ba328283d7ca2250b90bdfbaa305d5c3355
Author: Lucas Di Pentima <ldipentima at veritasgenetics.com>
Date:   Tue Aug 15 11:37:53 2017 -0300

    8937: Token expiration is already being checked, removed cache
    validation method and updated integration test.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <ldipentima at veritasgenetics.com>

diff --git a/sdk/python/arvados/commands/put.py b/sdk/python/arvados/commands/put.py
index 4a36ef0..ec4ae8f 100644
--- a/sdk/python/arvados/commands/put.py
+++ b/sdk/python/arvados/commands/put.py
@@ -710,6 +710,7 @@ class ArvPutUploadJob(object):
             elif file_in_local_collection.permission_expired():
                 # Permission token expired, re-upload file. This will change whenever
                 # we have a API for refreshing tokens.
+                self.logger.warning("Uploaded file '{}' access token expired, will re-upload it from scratch".format(filename))
                 should_upload = True
                 self._local_collection.remove(filename)
             elif cached_file_data['size'] == file_in_local_collection.size():
@@ -796,16 +797,8 @@ class ArvPutUploadJob(object):
                 arv_cmd.make_home_conf_dir(self.CACHE_DIR, 0o700, 'raise'),
                 cache_filename)
             if self.resume and os.path.exists(cache_filepath):
-                if self._cache_is_valid(cache_filepath):
-                    self.logger.info(
-                        "Resuming upload from cache file {}".format(
-                            cache_filepath))
-                    self._cache_file = open(cache_filepath, 'a+')
-                else:
-                    self.logger.info(
-                        "Cache file {} is not valid, starting from scratch".format(
-                            cache_filepath))
-                    self._cache_file = open(cache_filepath, 'w+')
+                self.logger.info("Resuming upload from cache file {}".format(cache_filepath))
+                self._cache_file = open(cache_filepath, 'a+')
             else:
                 # --no-resume means start with a empty cache file.
                 self.logger.info("Creating new cache file at {}".format(cache_filepath))
@@ -831,29 +824,6 @@ class ArvPutUploadJob(object):
             # Load the previous manifest so we can check if files were modified remotely.
             self._local_collection = arvados.collection.Collection(self._state['manifest'], replication_desired=self.replication_desired, put_threads=self.put_threads)
 
-    def _cache_is_valid(self, filepath):
-        with open(filepath, 'r') as cache_file:
-            try:
-                manifest = json.load(cache_file)['manifest']
-            except ValueError:
-                # Cache file is empty, all ok.
-                return True
-        if manifest is None:
-            # No cached manifest, all ok.
-            return True
-        # Check that the first block's token (oldest) is valid
-        match = arvados.util.signed_locator_pattern.search(manifest)
-        if match is not None:
-            loc = match.group(0)
-            try:
-                kc = arvados.keep.KeepClient(api_client=api_client)
-                return kc.head(loc, num_retries=self.num_retries)
-            except arvados.errors.KeepRequestError as e:
-                self.logger.info("Cache validation: {}".format(e))
-                return False
-        # No signed locator found, all ok.
-        return True
-
     def collection_file_paths(self, col, path_prefix='.'):
         """Return a list of file paths by recursively go through the entire collection `col`"""
         file_paths = []
diff --git a/sdk/python/tests/test_arv_put.py b/sdk/python/tests/test_arv_put.py
index ae7b7d1..3461678 100644
--- a/sdk/python/tests/test_arv_put.py
+++ b/sdk/python/tests/test_arv_put.py
@@ -9,6 +9,7 @@ standard_library.install_aliases()
 from builtins import str
 from builtins import range
 import apiclient
+import datetime
 import hashlib
 import json
 import mock
@@ -728,6 +729,9 @@ class ArvPutIntegrationTest(run_test_server.TestCaseWithServers,
         cls.ENVIRON = os.environ.copy()
         cls.ENVIRON['PYTHONPATH'] = ':'.join(sys.path)
 
+    def datetime_to_hex(self, dt):
+        return hex(int(time.mktime(dt.timetuple())))[2:]
+
     def setUp(self):
         super(ArvPutIntegrationTest, self).setUp()
         arv_put.api_client = None
@@ -841,7 +845,7 @@ class ArvPutIntegrationTest(run_test_server.TestCaseWithServers,
         self.assertEqual(1, len(collection_list))
         return collection_list[0]
 
-    def test_invalid_token_invalidates_cache(self):
+    def test_expired_token_invalidates_cache(self):
         self.authorize_with('active')
         tmpdir = self.make_tmpdir()
         with open(os.path.join(tmpdir, 'somefile.txt'), 'w') as f:
@@ -858,13 +862,15 @@ class ArvPutIntegrationTest(run_test_server.TestCaseWithServers,
                                    err.decode()).groups()[0]
         self.assertTrue(os.path.isfile(cache_filepath))
         # Load the cache file contents and modify the manifest to simulate
-        # an invalid access token
+        # an expired access token
         with open(cache_filepath, 'r') as c:
             cache = json.load(c)
         self.assertRegex(cache['manifest'], r'\+A\S+\@')
-        cache['manifest'] = re.sub(r'\+A\S+\@',
-                                   '+Adeadbeefdeadbeefdeadbeefdeadbeefdeadbeef@',
-                                   cache['manifest'])
+        a_month_ago = datetime.datetime.now() - datetime.timedelta(days=30)
+        cache['manifest'] = re.sub(
+            r'\@.*? ',
+            "@{} ".format(self.datetime_to_hex(a_month_ago)),
+            cache['manifest'])
         with open(cache_filepath, 'w') as c:
             c.write(json.dumps(cache))
         # Re-run the upload and expect to get an invalid cache message
@@ -875,8 +881,12 @@ class ArvPutIntegrationTest(run_test_server.TestCaseWithServers,
         (out, err) = p.communicate()
         self.assertRegex(
             err.decode(),
-            r'INFO: Cache file (.*) is not valid, starting from scratch')
+            r'WARNING: Uploaded file .* access token expired, will re-upload it from scratch')
         self.assertEqual(p.returncode, 0)
+        # Confirm that the resulting cache is different from the last run.
+        with open(cache_filepath, 'r') as c2:
+            new_cache = json.load(c2)
+        self.assertNotEqual(cache['manifest'], new_cache['manifest'])
 
     def test_put_collection_with_later_update(self):
         tmpdir = self.make_tmpdir()

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list