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

CORE-191: list/get/create import jobs works for requester-pays workspace #1502

Merged
merged 2 commits into from
Nov 26, 2024
Merged

CORE-191: list/get/create import jobs works for requester-pays workspace #1502

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
d41c5c8
optimized method to get workspaceId from Rawls
davidangb Nov 26, 2024
a565861
remove EntityService's custom getWorkspaceId implementation
davidangb Nov 26, 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
19 changes: 8 additions & 11 deletions src/main/scala/org/broadinstitute/dsde/firecloud/EntityService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,12 @@ class EntityService(rawlsDAO: RawlsDAO,
): Future[PerRequestMessage] = {
import spray.json._
val dataBytes = rawlsCalls.toJson.prettyPrint.getBytes(StandardCharsets.UTF_8)
getWorkspaceId(workspaceNamespace, workspaceName, userInfo) map { workspaceId =>
rawlsDAO.getWorkspaceId(workspaceNamespace, workspaceName)(userInfo) map { workspaceId =>
val bucketToWrite = GcsBucketName(FireCloudConfig.Cwds.bucket)
val fileToWrite = GcsObjectName(s"to-cwds/${workspaceId}/${java.util.UUID.randomUUID()}.json")
val gcsPath = writeDataToGcs(bucketToWrite, fileToWrite, dataBytes)
val importRequest = getRawlsJsonImportRequest(gcsPath, isUpsert)
importToCWDS(workspaceNamespace, workspaceName, workspaceId, userInfo, importRequest)
importToCWDS(workspaceNamespace, workspaceName, workspaceId.toString, userInfo, importRequest)
}
}

Expand All @@ -220,9 +220,6 @@ class EntityService(rawlsDAO: RawlsDAO,
private def getRawlsJsonImportRequest(gcsPath: String, isUpsert: Boolean): AsyncImportRequest =
AsyncImportRequest(gcsPath, FILETYPE_RAWLS, Some(ImportOptions(None, Some(isUpsert))))

private def getWorkspaceId(workspaceNamespace: String, workspaceName: String, userInfo: UserInfo): Future[String] =
rawlsDAO.getWorkspace(workspaceNamespace, workspaceName)(userInfo).map(_.workspace.workspaceId)

private def importToCWDS(workspaceNamespace: String,
workspaceName: String,
workspaceId: String,
Expand Down Expand Up @@ -339,8 +336,8 @@ class EntityService(rawlsDAO: RawlsDAO,
if (importRequest.filetype.isEmpty)
throw new FireCloudExceptionWithErrorReport(ErrorReport(BadRequest, "filetype must be specified"))

getWorkspaceId(workspaceNamespace, workspaceName, userInfo) map { workspaceId =>
importToCWDS(workspaceNamespace, workspaceName, workspaceId, userInfo, importRequest)
rawlsDAO.getWorkspaceId(workspaceNamespace, workspaceName)(userInfo) map { workspaceId =>
importToCWDS(workspaceNamespace, workspaceName, workspaceId.toString, userInfo, importRequest)
} recover { case apiEx: ApiException =>
throw wrapCwdsException(apiEx)
}
Expand All @@ -351,8 +348,8 @@ class EntityService(rawlsDAO: RawlsDAO,
runningOnly: Boolean,
userInfo: UserInfo
): Future[List[CwdsListResponse]] =
rawlsDAO.getWorkspace(workspaceNamespace, workspaceName)(userInfo) map { workspace =>
cwdsDAO.listJobsV1(workspace.workspace.workspaceId, runningOnly)(userInfo)
rawlsDAO.getWorkspaceId(workspaceNamespace, workspaceName)(userInfo) map { workspaceId =>
cwdsDAO.listJobsV1(workspaceId.toString, runningOnly)(userInfo)
} recover { case apiEx: ApiException =>
throw wrapCwdsException(apiEx)
}
Expand All @@ -362,8 +359,8 @@ class EntityService(rawlsDAO: RawlsDAO,
jobId: String,
userInfo: UserInfo
): Future[CwdsListResponse] =
rawlsDAO.getWorkspace(workspaceNamespace, workspaceName)(userInfo) map { workspace =>
val cwdsResponse = cwdsDAO.getJobV1(workspace.workspace.workspaceId, jobId)(userInfo)
rawlsDAO.getWorkspaceId(workspaceNamespace, workspaceName)(userInfo) map { workspaceId =>
val cwdsResponse = cwdsDAO.getJobV1(workspaceId.toString, jobId)(userInfo)
logger.info(s"Found job $jobId in cWDS")
cwdsResponse
} recover { case apiEx: ApiException =>
Expand Down
9 changes: 9 additions & 0 deletions src/main/scala/org/broadinstitute/dsde/firecloud/dataaccess/HttpRawlsDAO.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package org.broadinstitute.dsde.firecloud.dataaccess
import akka.actor.ActorSystem
import akka.http.scaladsl.marshallers.sprayjson.SprayJsonSupport
import akka.http.scaladsl.model.StatusCodes._
import akka.http.scaladsl.model.Uri.Query
import akka.http.scaladsl.model._
import akka.http.scaladsl.unmarshalling.Unmarshal
import akka.stream.Materializer
Expand All @@ -28,6 +29,7 @@ import org.broadinstitute.dsde.workbench.util.health.SubsystemStatus
import spray.json.DefaultJsonProtocol._
import spray.json._

import java.util.UUID
import scala.concurrent.{ExecutionContext, Future}
import scala.util.control.NonFatal

Expand Down Expand Up @@ -91,6 +93,13 @@ class HttpRawlsDAO(implicit val system: ActorSystem,
override def getWorkspace(ns: String, name: String)(implicit userToken: WithAccessToken): Future[WorkspaceResponse] =
authedRequestToObject[WorkspaceResponse](Get(getWorkspaceUrl(ns, name)))

override def getWorkspaceId(ns: String, name: String)(implicit userToken: WithAccessToken): Future[UUID] = {
val targetUri = Uri(getWorkspaceUrl(ns, name)).withQuery(Query(("fields", "workspace.workspaceId")))
authedRequestToObject[WorkspaceIdResponse](Get(targetUri)) map { resp: WorkspaceIdResponse =>
resp.workspace.workspaceId
}
}

override def patchWorkspaceAttributes(ns: String, name: String, attributeOperations: Seq[AttributeUpdateOperation])(
implicit userToken: WithAccessToken
): Future[WorkspaceDetails] =
Expand Down
3 changes: 3 additions & 0 deletions src/main/scala/org/broadinstitute/dsde/firecloud/dataaccess/RawlsDAO.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import org.broadinstitute.dsde.rawls.model._
import org.broadinstitute.dsde.workbench.util.health.Subsystems
import org.broadinstitute.dsde.workbench.util.health.Subsystems.Subsystem

import java.util.UUID
import scala.concurrent.Future

/**
Expand Down Expand Up @@ -72,6 +73,8 @@ trait RawlsDAO extends LazyLogging with ReportsSubsystemStatus {

def getWorkspace(ns: String, name: String)(implicit userToken: WithAccessToken): Future[WorkspaceResponse]

def getWorkspaceId(ns: String, name: String)(implicit userToken: WithAccessToken): Future[UUID]

def patchWorkspaceAttributes(ns: String, name: String, attributes: Seq[AttributeUpdateOperation])(implicit
userToken: WithAccessToken
): Future[WorkspaceDetails]
Expand Down
4 changes: 4 additions & 0 deletions src/main/scala/org/broadinstitute/dsde/firecloud/model/ModelJsonProtocol.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -378,4 +378,8 @@ object ModelJsonProtocol extends WorkspaceJsonSupport with SprayJsonSupport {
}

implicit val impShareFormat: RootJsonFormat[Share] = jsonFormat4(Share)

implicit val impWorkspaceIdFormat: RootJsonFormat[WorkspaceId] = jsonFormat1(WorkspaceId)
implicit val impWorkspaceIdResponseFormat: RootJsonFormat[WorkspaceIdResponse] = jsonFormat1(WorkspaceIdResponse)

}
5 changes: 5 additions & 0 deletions src/main/scala/org/broadinstitute/dsde/firecloud/model/Workspace.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import org.broadinstitute.dsde.firecloud.model.OrchMethodRepository.AgoraConfigu
import org.broadinstitute.dsde.rawls.model._
import org.joda.time.DateTime

import java.util.UUID

case class UIWorkspaceResponse(accessLevel: Option[String] = None,
canShare: Option[Boolean] = None,
catalog: Option[Boolean] = None,
Expand Down Expand Up @@ -96,3 +98,6 @@ case class RawlsGroupMemberList(userEmails: Option[Seq[String]] = None,
)

case class WorkspaceStorageCostEstimate(estimate: String, lastUpdated: Option[DateTime])

case class WorkspaceId(workspaceId: UUID)
case class WorkspaceIdResponse(workspace: WorkspaceId)
12 changes: 6 additions & 6 deletions src/test/scala/org/broadinstitute/dsde/firecloud/EntityServiceSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ class EntityServiceSpec extends BaseServiceSpec with BeforeAndAfterEach {

when(cwdsDAO.isEnabled).thenReturn(true)
when(cwdsDAO.getSupportedFormats).thenReturn(List("pfb", "tdrexport", "rawlsjson"))
when(rawlsDAO.getWorkspace(any[String], any[String])(any[UserInfo]))
.thenReturn(Future.successful(workspaceResponse))
when(rawlsDAO.getWorkspaceId(any[String], any[String])(any[UserInfo]))
.thenReturn(Future.successful(UUID.fromString(workspaceResponse.workspace.workspaceId)))

entityService
.importEntitiesFromTSV("workspaceNamespace", "workspaceName", tsvData, dummyUserInfo("token"), true)
Expand All @@ -176,7 +176,7 @@ class EntityServiceSpec extends BaseServiceSpec with BeforeAndAfterEach {
val capturedRequest = argumentCaptor.getValue
capturedRequest.options should be(Some(ImportOptions(None, Some(tsvType != "update"))))
verify(rawlsDAO, times(1))
.getWorkspace(any[String], any[String])(any[UserInfo])
.getWorkspaceId(any[String], any[String])(any[UserInfo])

}
}
Expand Down Expand Up @@ -289,8 +289,8 @@ class EntityServiceSpec extends BaseServiceSpec with BeforeAndAfterEach {

when(cwdsDAO.importV1(any[String], any[AsyncImportRequest])(any[UserInfo])).thenReturn(genericJob)

when(rawlsDAO.getWorkspace(any[String], any[String])(any[UserInfo]))
.thenReturn(Future.successful(workspaceResponse))
when(rawlsDAO.getWorkspaceId(any[String], any[String])(any[UserInfo]))
.thenReturn(Future.successful(UUID.fromString(workspaceResponse.workspace.workspaceId)))

// create input
val input = AsyncImportRequest(url = "https://example.com", filetype = importFiletype)
Expand All @@ -302,7 +302,7 @@ class EntityServiceSpec extends BaseServiceSpec with BeforeAndAfterEach {
verify(cwdsDAO, times(1))
.importV1(any[String], any[AsyncImportRequest])(any[UserInfo])
verify(rawlsDAO, times(1))
.getWorkspace(any[String], any[String])(any[UserInfo])
.getWorkspaceId(any[String], any[String])(any[UserInfo])
}

}
Expand Down
7 changes: 6 additions & 1 deletion src/test/scala/org/broadinstitute/dsde/firecloud/dataaccess/MockRawlsDAO.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ class MockRawlsDAO extends RawlsDAO {
private val rawlsWorkspaceWithAttributes = WorkspaceDetails(
"attributes",
"att",
"id",
"00000000-0000-0000-0000-000000000000",
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 needed to update this example test data; UUID.fromString("id") would cause problems. This is more realistic test data anyway.

"", // bucketname
Some("wf-collection"),
DateTime.now(),
Expand Down Expand Up @@ -563,6 +563,11 @@ class MockRawlsDAO extends RawlsDAO {
)
}

override def getWorkspaceId(ns: String, name: String)(implicit
userToken: WithAccessToken
): Future[UUID] =
getWorkspace(ns, name) map { workspaceResponse => UUID.fromString(workspaceResponse.workspace.workspaceId) }

override def getWorkspaces(implicit userInfo: WithAccessToken): Future[Seq[WorkspaceListResponse]] =
Future.successful(
Seq(
Expand Down