Skip to content

Commit

Permalink
Fixes size() returning 0 for dos inputs [BA-5731] (#5039)
Browse files Browse the repository at this point in the history
  • Loading branch information
salonishah11 authored Jun 26, 2019
1 parent 90154ed commit a439100
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
version 1.0

workflow wf_level_file_size {
File input1 = "dos://wb-mock-drs-dev.storage.googleapis.com/4a3908ad-1f0b-4e2a-8a92-611f2123e8b0"
File input2 = "dos://wb-mock-drs-dev.storage.googleapis.com/0c8e7bc6-fd76-459d-947b-808b0605beb3"

output {
Float fileSize1 = size(input1)
Float fileSize2 = size(input2)
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: drs_wf_level_read_size
testFormat: workflowsuccess
backends: [Papiv2NoDockerHubConfig]

files {
workflow: drs_tests/wf_level_file_size.wdl
}

metadata {
workflowName: wf_level_file_size
status: Succeeded

"outputs.wf_level_file_size.fileSize1": 43.0
"outputs.wf_level_file_size.fileSize2": 45.0
}


Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,18 @@ class DrsCloudNioFileProvider(scheme: String,


private def checkIfPathExistsThroughMartha(drsPath: String): IO[Boolean] = {
drsPathResolver.rawMarthaResponse(drsPath).use { marthaResponse =>
val errorMsg = s"Status line was null for martha response $marthaResponse."
toIO(Option(marthaResponse.getStatusLine), errorMsg)
}.map(_.getStatusCode == HttpStatus.SC_OK)
/*
* Unlike other cloud providers where directories are identified with a trailing slash at the end like `gs://bucket/dir/`,
* DRS has a concept of bundles for directories (not supported yet). Hence for method `checkDirectoryExists` which appends a trailing '/'
* to see if the current path is a directory, return false
*/
if (drsPath.endsWith("/")) IO(false)
else {
drsPathResolver.rawMarthaResponse(drsPath).use { marthaResponse =>
val errorMsg = s"Status line was null for martha response $marthaResponse."
toIO(Option(marthaResponse.getStatusLine), errorMsg)
}.map(_.getStatusCode == HttpStatus.SC_OK)
}
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,25 @@ case class DrsPathBuilder(fileSystemProvider: DrsCloudNioFileSystemProvider) ext

override def name: String = "DRS"

/**
* Unlike other cloud providers where directories are identified with a trailing slash at the end like `gs://bucket/dir/`,
* DRS has a concept of bundles for directories (not supported yet). This sanitizes the path by removing the trailing '/'(s)
* so that the CloudNioFileSystemProvider & CloudNioPath does not treat a DRS path ending with '/' as a directory, otherwise
* its size is returned as 0
*/
private def removeTrailingSlashes(path: String): String = {
if (path.length > (drsScheme.length + 3)) { //3: length of '://'
val pathArray = path.split(s"://")
val transformedPath = pathArray(1).replaceAll("[/]+$", "")
s"$drsScheme://$transformedPath"
}
else path
}

override def build(pathAsString: String): Try[Path] = {
if (pathAsString.startsWith(s"$drsScheme://")) {
Try(URI.create(UrlEscapers.urlFragmentEscaper().escape(pathAsString))) flatMap { uri =>
val pathWithoutTrailingSlashes = removeTrailingSlashes(pathAsString)
Try(URI.create(UrlEscapers.urlFragmentEscaper().escape(pathWithoutTrailingSlashes))) flatMap { uri =>
if (!Option(uri.getScheme).exists(_.equalsIgnoreCase(fileSystemProvider.getScheme))) {
Failure(new IllegalArgumentException(s"$pathAsString does not have a $drsScheme scheme."))
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ class DrsPathBuilderSpec extends TestKitSuite with FlatSpecLike with Matchers wi
description = "a path ending in /",
path = s"dos://$bucket/hello/world/",
normalize = false,
pathAsString = s"dos://$bucket/hello/world/",
pathWithoutScheme = s"$bucket/hello/world/",
pathAsString = s"dos://$bucket/hello/world",
pathWithoutScheme = s"$bucket/hello/world",
parent = s"dos://$bucket/hello/",
getParent = s"dos://$bucket/hello/",
root = s"dos://$bucket/",
Expand Down Expand Up @@ -278,8 +278,8 @@ class DrsPathBuilderSpec extends TestKitSuite with FlatSpecLike with Matchers wi
root = s"dos://blah/",
name = "",
getFileName = null,
getNameCount = 0,
isAbsolute = true),
getNameCount = 1,
isAbsolute = false),

GoodPath(
description = "an absolute path without a host",
Expand Down
12 changes: 12 additions & 0 deletions src/ci/resources/papi_application.inc.conf.ctmpl
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ engine {
auth = "service_account"
project = "broad-dsde-cromwell-dev"
}
drs {
auth = "service_account"
}
}
}

Expand Down Expand Up @@ -63,3 +66,12 @@ services {
}
}
}

filesystems.drs.global.config.martha.url = "https://us-central1-broad-dsde-dev.cloudfunctions.net/martha_v2"

drs {
localization {
docker-image = "broadinstitute/cromwell-dos:34-d8acfe3"
command-template = "mkdir -p $(dirname ${containerPath}) && /scripts/dosUrlLocalizer.sc ${drsPath} ${containerPath}"
}
}

0 comments on commit a439100

Please sign in to comment.