-
Notifications
You must be signed in to change notification settings - Fork 360
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
Add "User Service Account Impersonation" as an auth mode for GCP #7224
base: develop
Are you sure you want to change the base?
Add "User Service Account Impersonation" as an auth mode for GCP #7224
Conversation
@@ -69,7 +69,7 @@ final private[spi] case class UnixPath(path: String) extends CharSequence { | |||
|
|||
def isAbsolute: Boolean = UnixPath.isAbsolute(path) | |||
|
|||
def isEmpty: Boolean = path.isEmpty | |||
override def isEmpty: Boolean = path.isEmpty |
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.
Compiler was breaking without this, not related to PR
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.
Out of curiosity, what is your Java version?
I believe Cromwell is usually compiled with Java 11, and CharSequence::isEmpty
is "Since: 15"
This override will need to be fixed, but maybe not right now?
Btw, I use SDKMAN! to keep all my Java SDKs under control. The JDK referenced by Cromwell's config isn't available for my M2 mac. I've just changed it on my machine to a newer 11 Temurin release.
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.
Thanks! That was the issue. Have you had a moment to review the PR otherwise?
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.
Have you had a moment to review the PR otherwise?
I've glanced at it, but I'm not an official committer at the moment. I've got multiple of own PRs in the queue to get reviewed 😅
In the meantime I've just coalesced my commits into a fork and just compile and run that melange to keep moving.
Issue: #7223
If the team would prefer a smaller PR
We could change this to only ever use applicationDefaultCredentials, in which case we could remove parts of the code that are altering ServiceAccountMode