https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422016-04-12T17:16:28ZArvadosArvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=377492016-04-12T17:16:28ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/37749/diff?detail_id=36833">diff</a>)</li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=377582016-04-12T18:01:35ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Target version</strong> set to <i>Arvados Future Sprints</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=377632016-04-12T18:24:53ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Description</strong> updated (<a title="View differences" href="/journals/37763/diff?detail_id=36847">diff</a>)</li><li><strong>Category</strong> set to <i>SDKs</i></li><li><strong>Story points</strong> set to <i>2.0</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=378532016-04-13T19:13:01ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Radhika Chippada</i></li><li><strong>Target version</strong> changed from <i>Arvados Future Sprints</i> to <i>2016-04-27 sprint</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=380162016-04-19T15:46:41ZRadhika Chippadaradhika@curoverse.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=381322016-04-20T20:14:14ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Reviewing <a class="changeset" title="8937: add head request to python keep client." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/20fc2f783478fb438fb0a6eee193909f899f139b">20fc2f7</a></p>
<p>HEAD requests should ignore the cache. Revert the changes to <code>KeepBlockCache</code> and don't make any calls to <code>self.block_cache</code> when <code>method == "HEAD"</code></p>
<p>In <code>KeepService.get()</code> it should be <code>self._result.get('content-length'))</code> (the response) not <code>self._headers.get('content-length'))</code> (the request)</p>
<p><code>KeepService.get()</code> should return <code>True</code> on a successful <code>HEAD</code> per the story description, not the content-length.</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=381572016-04-21T15:24:07ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p><code>ResumeCache</code> already validates that the files being uploaded have probably not changed on disk, so there is nothing to do there.</p>
<p>ResumeCache.__init__ should do the following:</p>
<ul>
<li>Call <code>state = self.load()</code></li>
<li>If state["_finished_streams"] is not empty, then get the first locator from <code>locator = state["_finished_streams"][0].split(' ')[1]</code></li>
<li>Otherwise, if state["_finished_streams"] is empty, get the first locator from <code>locator = state["_current_stream_locators"].split(' ')[0]</code></li>
<li>Create an instance of <code>keepclient = KeepClient()</code></li>
<li>Call <code>keepclient.head(locator)</code></li>
</ul>
<p>Add KeepRequestError to the exception handler around the ResumeCache to reset the cache:</p>
<pre>
try:
cachepath = ResumeCache.make_path(args)
resume_cache = ResumeCache(cachepath)
except (IOError, OSError, ValueError):
pass # Couldn't open cache directory/file. Continue without it.
except KeepRequestError:
# delete the cache and create a new one
shutil.rmtree(cachepath)
resume_cache = ResumeCache(cachepath)
except ResumeCacheConflict:
print >>stderr, "\n".join([
"arv-put: Another process is already uploading this data.",
" Use --no-resume if this is really what you want."])
sys.exit(1)
</pre> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=382262016-04-25T13:44:37ZRadhika Chippadaradhika@curoverse.com
<ul></ul><ul>
<li>Addressed comments from note 6</li>
</ul>
<ul>
<li>Branch 8937-arv-put-cache-check handles the updates to arv-put where a head request is made on the first locator and the cache is recreated on errors. <a class="changeset" title="8937: invalidate cache and create new one if there are errors on head request during ResumeCache." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/3fc755d673dbdb29e467e26d8124f3c7d6eaab6a">3fc755d67</a></li>
</ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=383162016-04-27T15:03:43ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>On review, I think we want to refactor a little bit, I apologize.</p>
<p>I suggest that the block in ResumeCache.__init__ goes into its own method, <code>check_cache()</code>:</p>
<pre>
def check_cache(self):
try:
state = self.load()
locator = None
try:
if "_finished_streams" in state and len(state["_finished_streams"]) > 0:
locator = state["_finished_streams"][0][1][0]
elif "_current_stream_locators" in state and len(state["_current_stream_locators"]) > 0:
locator = state["_current_stream_locators"][0]
if locator is not None:
kc = arvados.keep.KeepClient(api_client=api_client)
kc.head(locator, num_retries=num_retries)
except Exception as e:
self.restart()
except (ValueError):
pass
</pre>
<p>Then, the block in main changes a little bit:</p>
<pre>
if args.resume:
try:
cachepath = ResumeCache.make_path(args)
resume_cache = ResumeCache(cachepath, api_client=api_client, num_retries=args.retries)
resume_cache.check_cache()
except (IOError, OSError, ValueError):
pass # Couldn't open cache directory/file. Continue without it.
except ResumeCacheConflict:
print >>stderr, "\n".join([
"arv-put: Another process is already uploading this data.",
" Use --no-resume if this is really what you want."])
sys.exit(1)
</pre>
<p>The ResumeCache.restart() method takes care of managing the cache file correctly, which I overlooked in the original suggestion for implementation.</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=383252016-04-27T16:03:17ZRadhika Chippadaradhika@curoverse.com
<ul></ul><p>Commit <a class="changeset" title="8937: refactor cache check logic into a check_cache method and update all references." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/8820ca5276510fd4ffad4040603508f2dd6ef3fa">8820ca52</a> incorporates those suggestions. Thanks.</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=383262016-04-27T16:17:22ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><pre>
======================================================================
FAIL: test_ArvPutSignedManifest (tests.test_arv_put.ArvPutIntegrationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/src/arvados/sdk/python/tests/test_arv_put.py", line 575, in test_ArvPutSignedManifest
self.assertEqual(p.returncode, 0)
AssertionError: 1 != 0
======================================================================
FAIL: test_put_collection_with_name_and_no_project (tests.test_arv_put.ArvPutIntegrationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/src/arvados/sdk/python/tests/test_arv_put.py", line 625, in test_put_collection_with_name_and_no_project
['--portable-data-hash', '--name', link_name])
File "/usr/src/arvados/sdk/python/tests/test_arv_put.py", line 598, in run_and_find_collection
self.assertEqual(1, len(collection_list))
AssertionError: 1 != 0
======================================================================
FAIL: test_put_collection_with_named_project_link (tests.test_arv_put.ArvPutIntegrationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/src/arvados/sdk/python/tests/test_arv_put.py", line 635, in test_put_collection_with_named_project_link
'--project-uuid', self.PROJECT_UUID])
File "/usr/src/arvados/sdk/python/tests/test_arv_put.py", line 598, in run_and_find_collection
self.assertEqual(1, len(collection_list))
AssertionError: 1 != 0
======================================================================
FAIL: test_put_collection_with_unnamed_project_link (tests.test_arv_put.ArvPutIntegrationTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/usr/src/arvados/sdk/python/tests/test_arv_put.py", line 615, in test_put_collection_with_unnamed_project_link
['--portable-data-hash', '--project-uuid', self.PROJECT_UUID])
File "/usr/src/arvados/sdk/python/tests/test_arv_put.py", line 598, in run_and_find_collection
self.assertEqual(1, len(collection_list))
AssertionError: 1 != 0
</pre> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=383282016-04-27T16:35:06ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><pre>
$ arvbox restart test --only sdk/python sdk/python_test=--test-suite=tests.test_arv_put.ArvPutIntegrationTest.test_ArvPutSignedManifest
...
[keep0] 2016/04/27 16:33:33 keepstore starting, pid 1034
[keep0] 2016/04/27 16:33:33 Using volume [UnixVolume /tmp/tmpUuhWG8] (writable=true)
[keep0] 2016/04/27 16:33:33 never-delete is not set. Warning: the relevant features in keepstore and data manager have not been extensively tested. You should leave this option alone unless you can afford to lose data.
[keep0] 2016/04/27 16:33:33 listening at :52737
[keep1] 2016/04/27 16:33:34 keepstore starting, pid 1042
[keep1] 2016/04/27 16:33:34 Using volume [UnixVolume /tmp/tmpxnkxp3] (writable=true)
[keep1] 2016/04/27 16:33:34 never-delete is not set. Warning: the relevant features in keepstore and data manager have not been extensively tested. You should leave this option alone unless you can afford to lose data.
[keep1] 2016/04/27 16:33:34 listening at :42993
0M / 0M 0.0% [keep1] 2016/04/27 16:33:34 [[::1]:55496] PUT 08a008a01d498c404b0c30852b39d3b8 44 0.000962s 0.000956s 0.000006s 200 87 "OK"
[keep0] 2016/04/27 16:33:34 [[::1]:40156] PUT 08a008a01d498c404b0c30852b39d3b8 44 0.000863s 0.000857s 0.000006s 200 87 "OK"
0M / 0M 100.0%
arv-put: Error creating Collection on project: <HttpError 403 when requesting https://0.0.0.0:43382/arvados/v1/collections?ensure_unique_name=true&alt=json returned "#<ArvadosModel::PermissionDeniedError: ArvadosModel::PermissionDeniedError>">.
Traceback (most recent call last):
File "/usr/src/arvados/sdk/python/arvados/commands/put.py", line 567, in <module>
main()
File "/usr/src/arvados/sdk/python/arvados/commands/put.py", line 551, in main
stdout.write(output)
UnboundLocalError: local variable 'output' referenced before assignment
FAIL
</pre>
<p>Not sure why it's getting permission denied error, but that might be what's going on here.</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=385282016-05-02T18:46:26ZRadhika Chippadaradhika@curoverse.com
<ul><li><strong>Target version</strong> changed from <i>2016-04-27 sprint</i> to <i>2016-05-11 sprint</i></li><li><strong>Story points</strong> changed from <i>2.0</i> to <i>0.5</i></li></ul><p>Merged code into master in <a class="changeset" title="refs #8937 Merge branch '8937-arv-put-cache-check'" href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/e62a18f3786d0f3c12865f865294a0f4d39ff548">e62a18f3</a> and moving to next sprint to add the integration test with 0.5 story points.</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=388772016-05-10T17:39:32ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Here's an empty example of what the cache file looks like:</p>
<pre>
{
"_current_stream_files": [],
"_current_stream_length": 0,
"_current_stream_locators": [],
"_current_stream_name": ".",
"_current_file": None,
"_current_file_name": null,
"_current_file_pos": 0,
"_close_file": null,
"_data_buffer": "",
"_dependencies": {},
"_finished_streams": [],
"_queued_dirents": [],
"_queued_trees": []
}
</pre> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=389682016-05-11T18:04:06ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Subject</strong> changed from <i>[SDKs] When arv-put resumes, it does a HEAD request on the first block, and invalidates the cache if that request fails</i> to <i>[SDKs] Write integration test for when arv-put resumes from a cache with expired access tokens</i></li><li><strong>Status</strong> changed from <i>In Progress</i> to <i>New</i></li><li><strong>Assigned To</strong> deleted (<del><i>Radhika Chippada</i></del>)</li><li><strong>Target version</strong> changed from <i>2016-05-11 sprint</i> to <i>Arvados Future Sprints</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=532612017-07-05T19:57:28ZTom Morristfmorris@veritasgenetics.com
<ul><li><strong>Assigned To</strong> set to <i>Lucas Di Pentima</i></li><li><strong>Target version</strong> changed from <i>Arvados Future Sprints</i> to <i>2017-07-19 sprint</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=535692017-07-19T18:46:13ZLucas Di Pentimalucas.dipentima@curii.com
<ul><li><strong>Target version</strong> changed from <i>2017-07-19 sprint</i> to <i>2017-08-02 sprint</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=538972017-08-02T18:53:14ZLucas Di Pentimalucas.dipentima@curii.com
<ul><li><strong>Target version</strong> changed from <i>2017-08-02 sprint</i> to <i>2017-08-16 sprint</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=540102017-08-09T14:19:23ZLucas Di Pentimalucas.dipentima@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=540322017-08-09T23:47:56ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Updates at <a class="changeset" title="8937: Added cache validation method to arv-put. For now it only checks that the first signed bloc..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/7b93dc26eb285261a6a431daa511edfb7392a3a1">7b93dc26e</a><br />Test run: <a class="external" href="https://ci.curoverse.com/job/developer-run-tests/409/">https://ci.curoverse.com/job/developer-run-tests/409/</a></p>
<ul>
<li>Updated <code>arv-put</code> code to do a cache file validation before loading. It checks the cached manifest's first signed block locator against keep using the <code>HEAD</code> method.</li>
<li>Added related integration test.</li>
</ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=540472017-08-10T20:01:32ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Looking at _cache_is_valid</p>
<ul>
<li>Instead of splitting the manifest into lines, could you use re.MULTILINE to find the first token?</li>
<li>KeepClient.head() will raise an exception if the block is inaccessible. The log message "Something wrong happened" is a little vague and misleading since this is an expected error. Consider catching KeepRequestError and printing a more precise error message.</li>
</ul> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=540482017-08-10T21:31:33ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Updates at <a class="changeset" title="8937: Refactored _cache_is_valid() to catch specific exceptions. Fixed related test. Arvados-DCO..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/7049888ba3001baa7e6428c056f2d5d03789747a">7049888ba</a><br />Test run: <a class="external" href="https://ci.curoverse.com/job/developer-run-tests/413/">https://ci.curoverse.com/job/developer-run-tests/413/</a></p>
<p>Made suggested changes.<br />Catching the specific <code>KeepRequestError</code> exception revealed some additional cases where other exceptions don't mean an invalid cache file.</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=540642017-08-11T15:30:20ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>Lucas Di Pentima wrote:</p>
<blockquote>
<p>Updates at <a class="changeset" title="8937: Refactored _cache_is_valid() to catch specific exceptions. Fixed related test. Arvados-DCO..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/7049888ba3001baa7e6428c056f2d5d03789747a">7049888ba</a><br />Test run: <a class="external" href="https://ci.curoverse.com/job/developer-run-tests/413/">https://ci.curoverse.com/job/developer-run-tests/413/</a></p>
<p>Made suggested changes.<br />Catching the specific <code>KeepRequestError</code> exception revealed some additional cases where other exceptions don't mean an invalid cache file.</p>
</blockquote>
<p>It seems weird that if the manifest is empty or invalid (lacks any signed blocks) it would still treat that as "valid cache" ? Seems like the logic should be flipped around, the only case where you want to use the cache is when it is both well formed and the signatures are valid.</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=541122017-08-15T14:44:01ZLucas Di Pentimalucas.dipentima@curii.com
<ul></ul><p>Updates at <a class="changeset" title="8937: Token expiration is already being checked, removed cache validation method and updated inte..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/81352ba328283d7ca2250b90bdfbaa305d5c3355">81352ba32</a><br />Test run: <a class="external" href="https://ci.curoverse.com/job/developer-run-tests/414/">https://ci.curoverse.com/job/developer-run-tests/414/</a></p>
<p>Realized that as part of <a class="issue tracker-6 status-3 priority-4 priority-default closed parent" title="Idea: Support incremental upload (rsync style) for arv-put (Resolved)" href="https://dev.arvados.org/issues/10383">#10383</a> we already added a token expiration check (see <a class="external" href="https://github.com/curoverse/arvados/blob/master/sdk/python/arvados/commands/put.py#L710">https://github.com/curoverse/arvados/blob/master/sdk/python/arvados/commands/put.py#L710</a>), so I removed the <code>_cache_is_valid()</code> method, and updated the integration test to work with the already existing code.</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=541382017-08-16T15:10:26ZPeter Amstutzpeter.amstutz@curii.com
<ul></ul><p>LGTM</p> Arvados - Idea #8937: [SDKs] Write integration test for when arv-put resumes from a cache with expired access tokenshttps://dev.arvados.org/issues/8937?journal_id=541512017-08-16T16:35:05ZAnonymous
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li></ul><p>Applied in changeset arvados|commit:51fe3482a5253d054e838fb1dcf3982f79650cee.</p>