Implemented HTTPRangedByteAccess. #277
Conversation
|
@tdanford URL is fine to use, but (as someone who often works without internet access) I would prefer to have any test that needs internet access be part of the integration tests and not the unit tests. |
| @@ -16,8 +16,12 @@ | |||
| package org.bdgenomics.adam.util | |||
|
|
|||
| import java.io._ | |||
There was a problem hiding this comment.
Is there a tool to do this? (I can't find it, if it's in IntelliJ.) Alternatively, can Scalariform do it?
There was a problem hiding this comment.
Scalariform has "lost momentum," :-(
|
Sure thing, Frank, let's talk about it on the call in a few minutes -- assuming you're going to make the call? |
| /** | ||
| * HTTPRangedByteAccess supports Ranged GET queries against HTTP resources. | ||
| * | ||
| * @param uri The URL of the resource, which should support ranged queries |
There was a problem hiding this comment.
Shouldn't this class be in a separate file?
There was a problem hiding this comment.
It ... could be, sure. All the other ByteAccess implementations are in this one though.
Happy to go either way.
There was a problem hiding this comment.
I'm just thinking that it'd be preferable to separate them out into their own package and files.
There was a problem hiding this comment.
Okay, I'll do that then.
|
All automated tests passed. |
| // on the range request, below -- that's just the length of the portion of the resource that | ||
| // was returned). | ||
| response.getAllHeaders.find(_.getName == "Content-Length") match { | ||
| case None => -1L |
There was a problem hiding this comment.
I think it'd be preferable to throw an exception here instead of a garbage value?
|
All automated tests passed. |
|
All automated tests passed. |
|
|
||
| val response = httpClient.execute(get) | ||
|
|
||
| // We want a 206 response to a ranged request, not your normal 200! |
There was a problem hiding this comment.
Out of curiosity, why is this?
There was a problem hiding this comment.
Ah, OK! Might make sense to clarify that in the comment, since people may not be familiar with a 206 code.
|
I've updated the tests to have a "networkconnected" profile which will run any network dependent unit tests. Also, I've moved the byte access & byte locator classes to an o.b.a.io package. |
|
|
||
| class ByteAccessSuite extends FunSuite with BeforeAndAfter { |
There was a problem hiding this comment.
Should this be changed to extend NetworkConnected now?
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/31/ |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/32/ |
|
Jenkins, test this please. |
|
I bumped up the PermGen on the Java commandline |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/33/ |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/36/ |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/38/ |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/39/ |
|
@tdanford Please run "./scripts/format-source" and push the changes to the topic branch for this pull request. Sorry for the inconvenience. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/41/ |
|
Jenkins, test this please. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/43/ |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/ADAM-prb/48/ |
|
Carl and I are at a loss, as to why this is still failing. |
- HTTPRangedByteAccess supports HTTP range requests for ranges of resources. Includes a test that executes against a few bytes of the test mouse chromosome on the berkeley server. - Added a new TestTags and moved the "NetworkConnected" ones into that - Running the "networkconnected" profile as part of the Jenkins test
|
All automated tests passed. |
Implemented HTTPRangedByteAccess.
|
Thanks Frank! |
HTTPRangedByteAccess supports HTTP range requests for ranges of resources. Includes a test
that executes against a few bytes of the test mouse chromosome on the berkeley server.
(Matt or Frank, check out the additional test case in ByteAccessSuite, let me know if using the URL of the mouse_chrM.bam, which is the same bam as used in the integration tests, is okay here in the unit tests too.)
This branch is a precursor to fixing the tests / integration tests on the big Parquet / RDD PR.