https://dev.arvados.org/https://dev.arvados.org/favicon.ico?15576888422015-04-30T15:30:11ZArvadosArvados - Bug #5856: gzip.open(f) sometimes throws struct.error (unpack requires a string argument of length 4) on File handle from CollectionReaderhttps://dev.arvados.org/issues/5856?journal_id=240252015-04-30T15:30:11ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset arvados|commit:c807aef7059b253512ba5da5d535e89e800b9c11.</p> Arvados - Bug #5856: gzip.open(f) sometimes throws struct.error (unpack requires a string argument of length 4) on File handle from CollectionReaderhttps://dev.arvados.org/issues/5856?journal_id=240282015-04-30T16:42:31ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>In Progress</i></li></ul> Arvados - Bug #5856: gzip.open(f) sometimes throws struct.error (unpack requires a string argument of length 4) on File handle from CollectionReaderhttps://dev.arvados.org/issues/5856?journal_id=240292015-04-30T16:43:54ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Target version</strong> changed from <i>Bug Triage</i> to <i>2015-05-20 sprint</i></li></ul> Arvados - Bug #5856: gzip.open(f) sometimes throws struct.error (unpack requires a string argument of length 4) on File handle from CollectionReaderhttps://dev.arvados.org/issues/5856?journal_id=240302015-04-30T16:45:09ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset arvados|commit:4570521e9d639806b1f25c93822d5cf0dbfe42d0.</p> Arvados - Bug #5856: gzip.open(f) sometimes throws struct.error (unpack requires a string argument of length 4) on File handle from CollectionReaderhttps://dev.arvados.org/issues/5856?journal_id=240322015-04-30T17:02:07ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>Resolved</i> to <i>In Progress</i></li><li><strong>Assigned To</strong> set to <i>Peter Amstutz</i></li></ul> Arvados - Bug #5856: gzip.open(f) sometimes throws struct.error (unpack requires a string argument of length 4) on File handle from CollectionReaderhttps://dev.arvados.org/issues/5856?journal_id=240332015-04-30T17:25:02ZBrett Smithbrett.smith@curii.com
<ul></ul><p>Reviewing <a class="changeset" title="ArvadosFileReader object always tries to return the exact amount of data asked for. This is to a..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/4570521e9d639806b1f25c93822d5cf0dbfe42d0">4570521</a></p>
<p>I'm not sure it makes much sense for the tests to manipulate <code>sfile.arvadosfile</code> directly. While it's not marked as private, it's not really part of the interface users are expected to use. What do you think of this:</p>
<ul>
<li>Rename <code>test_read_returns_first_block</code> to <code>test_read_block_crossing_behavior</code> in both test case classes. In ArvadosFileReaderTestCase, this would have the code you added for <code>test_read_crosses_blocks</code>. This way the name is sensible in both places and we don't have to try to erase one in the child.</li>
<li><code>test_successive_reads</code> is meant to ensure that state is maintained correctly across calls, so calling the stateless <code>readfrom</code> misses the mark. What if it called <code>sfile.read(4)</code> multiple times and checked for correct results until EOF? That would maintain the spirit of the test, and double-check that block crossing happens correctly.</li>
</ul>
<p>In the docstring, "upto" should be two words.</p>
<p>Thanks.</p> Arvados - Bug #5856: gzip.open(f) sometimes throws struct.error (unpack requires a string argument of length 4) on File handle from CollectionReaderhttps://dev.arvados.org/issues/5856?journal_id=240392015-04-30T18:22:23ZPeter Amstutzpeter.amstutz@curii.com
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>Assigned To</strong> deleted (<del><i>Peter Amstutz</i></del>)</li></ul><p>Brett Smith wrote:</p>
<blockquote>
<p>Reviewing <a class="changeset" title="ArvadosFileReader object always tries to return the exact amount of data asked for. This is to a..." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/4570521e9d639806b1f25c93822d5cf0dbfe42d0">4570521</a></p>
<p>I'm not sure it makes much sense for the tests to manipulate <code>sfile.arvadosfile</code> directly. While it's not marked as private, it's not really part of the interface users are expected to use. What do you think of this:</p>
<ul>
<li>Rename <code>test_read_returns_first_block</code> to <code>test_read_block_crossing_behavior</code> in both test case classes. In ArvadosFileReaderTestCase, this would have the code you added for <code>test_read_crosses_blocks</code>. This way the name is sensible in both places and we don't have to try to erase one in the child.</li>
<li><code>test_successive_reads</code> is meant to ensure that state is maintained correctly across calls, so calling the stateless <code>readfrom</code> misses the mark. What if it called <code>sfile.read(4)</code> multiple times and checked for correct results until EOF? That would maintain the spirit of the test, and double-check that block crossing happens correctly.</li>
</ul>
</blockquote>
<p>Done.</p>
<blockquote>
<p>In the docstring, "upto" should be two words.</p>
</blockquote>
<p>Fixed.</p> Arvados - Bug #5856: gzip.open(f) sometimes throws struct.error (unpack requires a string argument of length 4) on File handle from CollectionReaderhttps://dev.arvados.org/issues/5856?journal_id=240412015-04-30T18:29:20ZBrett Smithbrett.smith@curii.com
<ul><li><strong>Assigned To</strong> set to <i>Peter Amstutz</i></li></ul><p><a class="changeset" title="5856: Tweak tests. Fix typo." href="https://dev.arvados.org/projects/arvados/repository/arvados/revisions/c42ccd1f88e75b7910623581fbdf8d88c42b55dd">c42ccd1f</a> is good to merge. Thanks.</p>