Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor code around cloud support #1975

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Refactor code around cloud support #1975

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6390929
first draft
takutosato Aug 9, 2024
5454cdb
clean up
takutosato Aug 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/java/picard/cmdline/CommandLineProgram.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import com.intel.gkl.compression.IntelDeflaterFactory;
import com.intel.gkl.compression.IntelInflaterFactory;
import htsjdk.io.IOPath;
import htsjdk.samtools.Defaults;
import htsjdk.samtools.SAMFileHeader;
import htsjdk.samtools.SAMFileWriterFactory;
Expand Down Expand Up @@ -341,7 +342,7 @@ protected boolean parseArgs(final String[] argv) {
// object created by this code path won't be valid - but we still have to set it here in case
// the tool tries to access REFERENCE_SEQUENCE directly (such tools will subsequently fail given
// a non-local file anyway, but this prevents them from immediately throwing an NPE).
final PicardHtsPath refHtsPath = referenceSequence.getHtsPath();
final IOPath refHtsPath = referenceSequence.getHtsPath();
REFERENCE_SEQUENCE = ReferenceArgumentCollection.getFileSafe(refHtsPath, Log.getInstance(this.getClass()));

// The TMP_DIR setting section below was moved from instanceMain() to here due to timing issues
Expand Down
5 changes: 3 additions & 2 deletions src/main/java/picard/cmdline/argumentcollections/ReferenceArgumentCollection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import java.io.File;
import java.nio.file.Path;

import htsjdk.io.IOPath;
import htsjdk.samtools.util.Log;
import picard.nio.PicardBucketUtils;
import picard.nio.PicardHtsPath;
Expand Down Expand Up @@ -61,7 +62,7 @@ default Path getReferencePath(){
*
* @return The reference provided by the user, if any, or the default, if any, as a PicardHtsPath. May be null.
*/
default PicardHtsPath getHtsPath(){
default IOPath getHtsPath(){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name of this method should probably change to reflect the new return type , i.e., getIOPath.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

return getReferenceFile() == null ? null : new PicardHtsPath(getReferenceFile());
}

Expand All @@ -74,7 +75,7 @@ default PicardHtsPath getHtsPath(){
* the value returned by calls to getReferenceFile to not get an NPE, and to fail gracefully downstream
* with an error message that includes the reference file specifier.
*/
static File getFileSafe(final PicardHtsPath picardPath, final Log log) {
static File getFileSafe(final IOPath picardPath, final Log log) {
if (picardPath == null) {
return null;
} else if (picardPath.getScheme().equals(PicardBucketUtils.FILE_SCHEME)) {
Expand Down
139 changes: 45 additions & 94 deletions src/main/java/picard/nio/PicardBucketUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,25 @@
package picard.nio;

import com.google.cloud.storage.contrib.nio.CloudStorageFileSystem;
import com.google.cloud.storage.contrib.nio.CloudStoragePath;
import htsjdk.io.IOPath;
import htsjdk.samtools.util.FileExtensions;
import htsjdk.utils.ValidationUtils;
import picard.PicardException;

import java.io.File;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.UUID;


/**
* Derived from BucketUtils.java in GATK
*/
public class PicardBucketUtils {
public static final String GOOGLE_CLOUD_STORAGE_FILESYSTEM_SCHEME = "gs";
public static final String HTTP_FILESYSTEM_PROVIDER_SCHEME = "http";
public static final String HTTPS_FILESYSTEM_PROVIDER_SCHEME = "https";
public static final String HDFS_SCHEME = "hdfs";
public static final String FILE_SCHEME = "file";

// This Picard test staging bucket has a TTL of 180 days (DeleteAction with Age = 180)
public static final String GCLOUD_PICARD_STAGING_DIRECTORY = "gs://hellbender-test-logs/staging/picard/";
public static final String GCLOUD_PICARD_STAGING_DIRECTORY_STR = "gs://hellbender-test-logs/staging/picard/";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These really need to be reconciled with the ones in GCloudTestUtils, where there is a redundant definition of the staging area root gs://hellbender-test-logs/staging/. There really should be only one definition, and all direct usages of this constant should be replaced with calls to the methods in GCloudTestUtils that interrogate where the user wants the staging area to be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out. I think it should be handled in a separate PR, if that's alright with you.

public static final PicardHtsPath GCLOUD_PICARD_STAGING_DIRECTORY = new PicardHtsPath(GCLOUD_PICARD_STAGING_DIRECTORY_STR);


// slashes omitted since hdfs paths seem to only have 1 slash which would be weirder to include than no slashes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git blame seems to implicate me on this comment, but I have no clue what it means ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a remnant from when we copied over code from GATK. Removed.

private PicardBucketUtils(){} //private so that no one will instantiate this class
Expand All @@ -41,59 +36,43 @@ private PicardBucketUtils(){} //private so that no one will instantiate this cla
* @return a new temporary path of the form [directory]/[prefix][random chars][.extension]
*
*/
public static PicardHtsPath getTempFilePath(final String directory, String prefix, final String extension){
public static IOPath getTempFilePath(final IOPath directory, String prefix, final String extension){
ValidationUtils.validateArg(extension.startsWith("."), "The new extension must start with a period '.'");
final String defaultPrefix = "tmp";

if (directory == null){
// If directory = null, we are creating a local temp file.
// File#createTempFile requires that the prefix be at least 3 characters long
prefix = prefix.length() >= 3 ? prefix : defaultPrefix;
return new PicardHtsPath(PicardIOUtils.createTempFile(prefix, extension));
}

if (isGcsUrl(directory) || isHadoopUrl(directory)){
final PicardHtsPath path = PicardHtsPath.fromPath(randomRemotePath(directory, prefix, extension));
PicardIOUtils.deleteOnExit(path.toPath());
// Mark auxiliary files to be deleted
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, FileExtensions.TRIBBLE_INDEX, true).toPath());
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, FileExtensions.TABIX_INDEX, true).toPath());
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, FileExtensions.BAI_INDEX, true).toPath()); // e.g. file.bam.bai
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, FileExtensions.BAI_INDEX, false).toPath()); // e.g. file.bai
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, ".md5", true).toPath());
return path;
} else {
} else if (PicardBucketUtils.isLocalPath(directory)) {
// Assume the (non-null) directory points to a directory on a local filesystem
prefix = prefix.length() >= 3 ? prefix : defaultPrefix;
return new PicardHtsPath(PicardIOUtils.createTempFileInDirectory(prefix, extension, new File(directory)));
return new PicardHtsPath(PicardIOUtils.createTempFileInDirectory(prefix, extension, directory.toPath().toFile()));
} else {
if (isSupportedCloudFilesystem(directory)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the right way to branch here is to invert the logic and test for a file protocol scheme. See my comments on isSupportedCloudFileSystem.

final IOPath path = randomRemotePath(directory, prefix, extension);
PicardIOUtils.deleteOnExit(path.toPath());
// Mark auxiliary files to be deleted
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, FileExtensions.TRIBBLE_INDEX, true).toPath());
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, FileExtensions.TABIX_INDEX, true).toPath());
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, FileExtensions.BAI_INDEX, true).toPath()); // e.g. file.bam.bai
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, FileExtensions.BAI_INDEX, false).toPath()); // e.g. file.bai
PicardIOUtils.deleteOnExit(PicardHtsPath.replaceExtension(path, ".md5", true).toPath());
return path;
} else {
throw new PicardException("Unsupported cloud filesystem: " + directory.getURIString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isSupportedCloudFileSystem currently throws (I think it should not - see my comments there). If you keep this code, I suggest it to IllegalArgumentException (I'm still not sure what the intended use of PicardException is in this codebase, but IllegalArgumentException would seem more correct to me), and changing isSupportedFileSystem to just return a boolean and never throw.

}
}
}

/**
* This overload of getTempFilePath takes the directory of type PicardHtsPath instead of String.
*
* @see #getTempFilePath(String, String, String)
*
*/
public static PicardHtsPath getTempFilePath(final IOPath directory, String prefix, final String extension){
return getTempFilePath(directory.getURIString(), prefix, extension);
}

/**
* Calls getTempFilePath with the empty string as the prefix.
*
* @see #getTempFilePath(String, String, String)
*/
public static PicardHtsPath getTempFilePath(String directory, String extension){
return getTempFilePath(directory, "", extension);
}

/**
* Creates a temporary file in a local directory.
*
* @see #getTempFilePath(String, String, String)
* @see #getTempFilePath(IOPath, String, String)
*/
public static PicardHtsPath getLocalTempFilePath(final String prefix, final String extension){
return getTempFilePath((String) null, prefix, extension);
public static IOPath getLocalTempFilePath(final String prefix, final String extension){
return getTempFilePath(null, prefix, extension);
}

/**
Expand All @@ -110,10 +89,10 @@ public static PicardHtsPath getLocalTempFilePath(final String prefix, final Stri
* @param relativePath The relative location for the new "directory" under the harcoded staging bucket with a TTL set e.g. "test/RevertSam/".
* @return A PicardHtsPath object to a randomly generated "directory" e.g. "gs://hellbender-test-logs/staging/picard/test/RevertSam/{randomly-generated-string}/"
*/
public static PicardHtsPath getRandomGCSDirectory(final String relativePath){
public static IOPath getRandomGCSDirectory(final String relativePath){
ValidationUtils.validateArg(relativePath.endsWith("/"), "relativePath must end in backslash '/': " + relativePath);

return PicardHtsPath.fromPath(PicardBucketUtils.randomRemotePath(GCLOUD_PICARD_STAGING_DIRECTORY + relativePath, "", "/"));
return PicardBucketUtils.randomRemotePath(PicardHtsPath.resolve(GCLOUD_PICARD_STAGING_DIRECTORY, relativePath), "", "/");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every direct reference to GCLOUD_PICARD_STAGING_DIRECTORY like this is bypassing the GCloudUtils methods such as getTestStaging that check the environment to determine if the user has specified an override staging area to use.

}

/**
Expand All @@ -123,39 +102,14 @@ public static PicardHtsPath getRandomGCSDirectory(final String relativePath){
* @param prefix The beginning of the file name
* @param suffix The end of the file name, e.g. ".tmp"
*/
public static Path randomRemotePath(String stagingLocation, String prefix, String suffix) {
public static IOPath randomRemotePath(final IOPath stagingLocation, final String prefix, final String suffix) {
if (isGcsUrl(stagingLocation)) {
return getPathOnGcs(stagingLocation).resolve(prefix + UUID.randomUUID() + suffix);
} else if (isHadoopUrl(stagingLocation)) {
return Paths.get(stagingLocation, prefix + UUID.randomUUID() + suffix);
return PicardHtsPath.resolve(stagingLocation, prefix + UUID.randomUUID() + suffix);
} else {
throw new IllegalArgumentException("Staging location is not remote: " + stagingLocation);
}
}

/**
* String -> Path. This *should* not be necessary (use Paths.get(URI.create(...)) instead) , but it currently is
* on Spark because using the fat, shaded jar breaks the registration of the GCS FilesystemProvider.
* To transform other types of string URLs into Paths, use IOUtils.getPath instead.
*/
private static CloudStoragePath getPathOnGcs(String gcsUrl) {
// use a split limit of -1 to preserve empty split tokens, especially trailing slashes on directory names
final String[] split = gcsUrl.split("/", -1);
final String BUCKET = split[2];
final String pathWithoutBucket = String.join("/", Arrays.copyOfRange(split, 3, split.length));
return CloudStorageFileSystem.forBucket(BUCKET).getPath(pathWithoutBucket);
}

/**
*
* @param path path to inspect
* @return true if this path represents a gcs location
*/
private static boolean isGcsUrl(final String path) {
GATKUtils.nonNull(path);
return path.startsWith(GOOGLE_CLOUD_STORAGE_FILESYSTEM_SCHEME + "://");
}

/**
*
* Return true if this {@code PicardHTSPath} represents a gcs URI.
Expand All @@ -168,34 +122,31 @@ public static boolean isGcsUrl(final IOPath pathSpec) {
}

/**
* @param pathSpec specifier to inspect
* @return true if this {@code GATKPath} represents a remote storage system which may benefit from prefetching (gcs or http(s))
* @param path specifier to inspect
* @return true if this {@code IOPath} represents a remote storage system which may benefit from prefetching (gcs or http(s))
*/
public static boolean isEligibleForPrefetching(final IOPath pathSpec) {
GATKUtils.nonNull(pathSpec);
return isEligibleForPrefetching(pathSpec.getScheme());
}

/**
* @param path path to inspect
* @return true if this {@code Path} represents a remote storage system which may benefit from prefetching (gcs or http(s))
*/
public static boolean isEligibleForPrefetching(final Path path) {
public static boolean isEligibleForPrefetching(final IOPath path) {
GATKUtils.nonNull(path);
return isEligibleForPrefetching(path.toUri().getScheme());
}

private static boolean isEligibleForPrefetching(final String scheme){
final String scheme = path.getScheme();
return scheme != null
&& (scheme.equals(GOOGLE_CLOUD_STORAGE_FILESYSTEM_SCHEME)
|| scheme.equals(HTTP_FILESYSTEM_PROVIDER_SCHEME)
|| scheme.equals(HTTPS_FILESYSTEM_PROVIDER_SCHEME));
}

public static boolean isLocalPath(final IOPath path){
return path.getScheme().equals(FILE_SCHEME);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should really be an instance method on PicardHtsPath for now (ultimately, really it should be on IOPath - I may create a PR to add it to the IOPath interface in htsjdk).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to happen in IOPath, because all the objects that would call the instance method isLocalPath have type IOPath, not PicardHtsPath. If you could add it to htsjdk, that would be great.


/**
* Returns true if the given path is a HDFS (Hadoop filesystem) URL.
* As of August 2024, we only support Google Cloud.
* Will add other filesystems (e.g. Azure, AWS) when ready.
*
* @return whether the cloud filesystem is currently supported by Picard.
*/
private static boolean isHadoopUrl(String path) {
return path.startsWith(HDFS_SCHEME + "://");
public static boolean isSupportedCloudFilesystem(final IOPath path){
ValidationUtils.validateArg(! isLocalPath(path), "isSupportedCloudFilesystem should be called on a cloud path but was given: " +
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't actually think this method should be retained (or adds much value), for several reasons.

First, enumerating all of the supported protocol schemes may become difficult as we add additional cloud providers (I think azure, for example, uses multiple schemes). Also, what schemes will work may depend on what additional providers the user has on their classpath.

Having said that, if you're going to keep it, it should be moved to PicardHtsPath, and a test added, with test cases at least for file, http, hdfs, s3, and gcs schemes.

Finally, for a predicate method like this, where you're just testing the protocol scheme, it should be pure and not have a side effect like throwing an exception (with the possible exception of requiring that the input is non-null, since the code can't recover from that). I don't see any reason to require the caller to first test if the path has a cloud scheme to prevent it from throwing. It should just return a boolean if the scheme is a supported cloud scheme, and let the caller decide when to throw.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing sounds good

path.getURIString());
return isGcsUrl(path);
}
}
2 changes: 1 addition & 1 deletion src/main/java/picard/nio/PicardHtsPath.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public static PicardHtsPath replaceExtension(final IOPath path, final String new
/**
* Wrapper for Path.resolve()
*/
public static PicardHtsPath resolve(final PicardHtsPath absPath, final String relativePath){
public static PicardHtsPath resolve(final IOPath absPath, final String relativePath){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the return value for this should also be IOPath, unless there is some reason that would cause problems. (Ideally this would be a method on IOPath, but since it has to return a new object of the subclass type, it can't live there, unless we add some kind of abstract "toSelf" method that each subclass has to implement. For another day).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, and I see your point. Another day sounds good.

return PicardHtsPath.fromPath(absPath.toPath().resolve(relativePath));
}
}
15 changes: 8 additions & 7 deletions src/test/java/picard/cmdline/CommandLineProgramTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package picard.cmdline;

import htsjdk.io.IOPath;
import org.apache.commons.io.FileUtils;
import org.testng.annotations.AfterClass;
import picard.PicardException;
Expand All @@ -24,19 +25,19 @@ public abstract class CommandLineProgramTest {
public static final File CHR_M_DICT = new File(REFERENCE_TEST_DIR,"chrM.reference.dict");

// These are the hg19 references with chromosome names "1" (rather than "chr1")
public static final PicardHtsPath HG19_CHR2021_GCLOUD = new PicardHtsPath(GCloudTestUtils.getTestInputPath() + "picard/references/human_g1k_v37.20.21.fasta");
public static final PicardHtsPath HG19_CHR2021 = new PicardHtsPath("testdata/picard/reference/human_g1k_v37.20.21.fasta.gz");
public static final IOPath HG19_CHR2021_GCLOUD = PicardHtsPath.resolve(GCloudTestUtils.getTestInputPath(), "picard/references/human_g1k_v37.20.21.fasta");
public static final IOPath HG19_CHR2021 = new PicardHtsPath("testdata/picard/reference/human_g1k_v37.20.21.fasta.gz");

public static final PicardHtsPath NA12878_MINI_GCLOUD = new PicardHtsPath(GCloudTestUtils.getTestInputPath() + "picard/bam/CEUTrio.HiSeq.WGS.b37.NA12878.20.21_n100.bam");
public static final PicardHtsPath NA12878_MINI_CRAM_GCLOUD = new PicardHtsPath(GCloudTestUtils.getTestInputPath() + "picard/bam/CEUTrio.HiSeq.WGS.b37.NA12878.20.21_n100.cram");
public static final PicardHtsPath NA12878_MEDIUM_GCLOUD = new PicardHtsPath(GCloudTestUtils.getTestInputPath() + "picard/bam/CEUTrio.HiSeq.WGS.b37.NA12878.20.21_n10000.bam");
public static final PicardHtsPath NA12878_MEDIUM_CRAM_GCLOUD = new PicardHtsPath(GCloudTestUtils.getTestInputPath() + "picard/bam/CEUTrio.HiSeq.WGS.b37.NA12878.20.21_n10000.cram");
public static final IOPath NA12878_MINI_GCLOUD = PicardHtsPath.resolve(GCloudTestUtils.getTestInputPath(), "picard/bam/CEUTrio.HiSeq.WGS.b37.NA12878.20.21_n100.bam");
public static final IOPath NA12878_MINI_CRAM_GCLOUD = PicardHtsPath.resolve(GCloudTestUtils.getTestInputPath(), "picard/bam/CEUTrio.HiSeq.WGS.b37.NA12878.20.21_n100.cram");
public static final IOPath NA12878_MEDIUM_GCLOUD = PicardHtsPath.resolve(GCloudTestUtils.getTestInputPath(), "picard/bam/CEUTrio.HiSeq.WGS.b37.NA12878.20.21_n10000.bam");
public static final IOPath NA12878_MEDIUM_CRAM_GCLOUD = PicardHtsPath.resolve(GCloudTestUtils.getTestInputPath(), "picard/bam/CEUTrio.HiSeq.WGS.b37.NA12878.20.21_n10000.cram");

// A per-test-class directory that will be deleted after the tests are complete.
private File tempOutputDir;

/**
* returns an directory designated for output which will be deleted after the test class is tested
* returns a directory designated for output which will be deleted after the test class is tested
*/
public File getTempOutputDir() {
if (tempOutputDir == null) {
Expand Down
29 changes: 11 additions & 18 deletions src/test/java/picard/nio/PicardBucketUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,11 @@
package picard.nio;

import htsjdk.io.IOPath;
import org.testng.Assert;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
import picard.util.GCloudTestUtils;

import java.net.URI;
import java.nio.file.Path;
import java.nio.file.Paths;

public class PicardBucketUtilsTest {

@DataProvider(name="testGetTempFilePathDataProvider")
Expand All @@ -23,32 +20,28 @@ public Object[][] testGetTempFilePathDataProvider() {

// Check that the extension scheme is consistent for cloud and local files
@Test(dataProvider = "testGetTempFilePathDataProvider", groups = "cloud")
public void testGetTempFilePath(final String directory, final String prefix, final String extension){
PicardHtsPath path = PicardBucketUtils.getTempFilePath(directory, prefix, extension);
public void testGetTempFilePath(final IOPath directory, final String prefix, final String extension){
final IOPath path = PicardBucketUtils.getTempFilePath(directory, prefix, extension);
Assert.assertTrue(path.hasExtension(extension));
if (directory != null){
Assert.assertTrue(path.getURIString().startsWith(directory));
}

if (directory == null){
Assert.assertEquals(path.getScheme(), "file");
Assert.assertTrue(path.getURIString().startsWith(directory.getURIString()));
} else {
Assert.assertEquals(path.getScheme(), PicardBucketUtils.FILE_SCHEME);
}
}

@DataProvider
public Object[][] getVariousPathsForPrefetching(){
return new Object[][]{
{"file:///local/file", false},
{"gs://abucket/bucket", true},
{"gs://abucket_with_underscores", true},
{new PicardHtsPath("file:///local/file"), false},
{new PicardHtsPath("gs://abucket/bucket"), true},
{new PicardHtsPath("gs://abucket_with_underscores"), true},
};
}

@Test(groups="bucket", dataProvider = "getVariousPathsForPrefetching")
public void testIsEligibleForPrefetching(String path, boolean isPrefetchable){
final URI uri = URI.create(path);
final Path uriPath = Paths.get(uri);
Assert.assertEquals(PicardBucketUtils.isEligibleForPrefetching(uriPath), isPrefetchable);
public void testIsEligibleForPrefetching(final IOPath path, boolean isPrefetchable){
Assert.assertEquals(PicardBucketUtils.isEligibleForPrefetching(path), isPrefetchable);
}

}
Loading
Loading