-
Notifications
You must be signed in to change notification settings - Fork 4
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.
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
ID-1301 talk to ecm for nih username (Round 2) #1407
Conversation
…broadinstitute/firecloud-orchestration into tl_ID-1301_talk_to_ecm_for_nih_username
@@ -112,7 +112,7 @@ start_server () { | |||
--network=fc-orch \ | |||
-e JAVA_OPTS="$DOCKER_JAVA_OPTS" \ | |||
sbtscala/scala-sbt:eclipse-temurin-jammy-17.0.10_7_1.10.1_2.13.14 \ | |||
sbt \~reStart | |||
bash -c "git config --global --add safe.directory /app && sbt \~reStart" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fixes a
[info] loading settings for project root from build.sbt ...
fatal: detected dubious ownership in repository at '/app'
To add an exception for this directory, call:
git config --global --add safe.directory /app
error
val excludeSpring = ExclusionRule(organization = "org.springframework") | ||
val excludeSpringBoot = ExclusionRule(organization = "org.springframework.boot") | ||
val excludeSpringJcl = ExclusionRule(organization = "org.springframework", name = "spring-jcl") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exclusions to resolve assembly
merge clashes, and also to reduce the jar size
@@ -50,9 +54,11 @@ object Dependencies { | |||
excludeGuava("org.broadinstitute.dsde.workbench" %% "workbench-util" % s"0.10-$workbenchLibsHash"), | |||
"org.broadinstitute.dsde.workbench" %% "workbench-google2" % s"0.36-$workbenchLibsHash", | |||
"org.broadinstitute.dsde.workbench" %% "workbench-oauth2" % s"0.7-$workbenchLibsHash", | |||
"org.broadinstitute.dsde.workbench" %% "sam-client" % "0.1-ef83073", | |||
"org.broadinstitute.dsde.workbench" %% "sam-client" % "v0.0.263", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was ancient!
|
||
import scala.concurrent.Future | ||
|
||
class DisabledExternalCredsDAO extends ExternalCredsDAO with LazyLogging { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is needed so that Orch can run in BEEs. In the future, we'll need to get ECM to run in BEEs so that swat tests can pass without Thurloe.
@@ -50,6 +50,14 @@ class HttpSamDAO( implicit val system: ActorSystem, val materializer: Materializ | |||
authedRequestToObject[UserIdInfo](Get(samGetUserIdsUrl.format(URLEncoder.encode(email.value, UTF_8.name)))) | |||
} | |||
|
|||
// Sam's API only allows for 1000 user to be fetched at one time | |||
override def getUsersForIds(samUserIds: Seq[WorkbenchUserId])(implicit userInfo: WithAccessToken): Future[Seq[WorkbenchUserInfo]] = Future.sequence { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New Sam API to resolve a list of users from user ids.
@@ -2,6 +2,7 @@ package org.broadinstitute.dsde.firecloud.model | |||
|
|||
import akka.http.scaladsl.model.headers.OAuth2BearerToken | |||
|
|||
import java.time.OffsetDateTime |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this import?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nope!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lgtm!
https://broadworkbench.atlassian.net/browse/ID-1301
First PR caused Dev to be unable to load NIH Account status, so it was reverted. Trying again.
Getting ready for ECM to replace Thurloe for NIH Account linking. For now, we'll need to talk to both, since links last for 30 days. Once everything is deployed to prod and has been out for a while, we should be able to safely delete the Thurloe code.
Have you read CONTRIBUTING.md lately? If not, do that first.
I, the developer opening this PR, do solemnly pinky swear that:
In all cases: