-
Notifications
You must be signed in to change notification settings - Fork 375
AN-732 WDL 1.1 localizationOptional support
#7823
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
base: develop
Are you sure you want to change the base?
Conversation
LizBaldo
left a comment
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.
Very clear, thanks for splitting the tickets, it makes this PR a lot easier to review.
Not sure what is going on with the integrations tests though, seems like there are some unrelated infra issues
| @@ -1,145 +0,0 @@ | |||
| version 1.0 | |||
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.
Left over and forgotten from the papi > Batch move?
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.
Even better, it's from the PAPIv1 => PAPIv2 transition from about 2018.
| val localizeOptional = jobDescriptor.findInputFilesByParameterMeta { | ||
| case MetaValueElementObject(values) => values.get("localization_optional").contains(MetaValueElementBoolean(true)) | ||
| case _ => false | ||
| // TODO: There is an AWS version of this that looks functionally identical. Consider unifying. |
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.
Worth a ticket in the CTM board? This might be a good onboarding ticket for a future dev
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.
I think I'm going to do this in the next PR, it's too small to document and re-explain later.
Description
"What the heck is localization optional?"
When this attribute is set, apply "localization optional" treatment to all files in the task. Previously, a user had to specify it for each file individually.
This is an enhancement over the old, nonstandard way that required listing each file:
The 1.1 spec also has a new way to treat individual files, which I'm saving for the next PR. There are some keys and comments referencing this.
Release Notes Confirmation
CHANGELOG.mdCHANGELOG.mdin this PRCHANGELOG.mdbecause it doesn't impact community usersTerra Release Notes