Repository navigation
Conversation
connect() required NGS360_AUTH_TOKEN. That is fine for a launcher started from a user's session, but a launcher running as an AWS Batch job has no session to source a token from -- whoever submits the job has to inject a credential, and the service key is the one WES already provisions for callers acting without a user session (X-Internal-Service-Key plus X-On-Behalf-Of for the audit trail). The bearer token still wins when both are set: it names one real user and is scoped to them, whereas a holder of the service key is trusted to assert whatever identity it likes, so injecting the key into a container widens WES's trust boundary to that container. When authenticating with the service key, skip the /api/v1/auth/me probe: that endpoint validates a *user's* bearer token, so there is nothing for it to check. The service-info call already proved we can reach and authenticate against WES. Tests hide only the four launcher environment variables (via hidden_launcher_env) rather than clearing os.environ, so a variable left set in the developer's shell cannot change what the tests exercise while everything else in the environment is left alone. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
get_current_task() returned None with the note that "WES API doesn't have a concept of current task". That was accurate when it was written: a run could not be identified from inside its own container. The corresponding WES service change makes a launcher execution a first-class WES run, so the launcher can be told its own run id -- WES_RUN_ID, the same shape as SevenBridges' TASK_ID -- and read its own run record back. This is what the RNA-Seq launcher needs to reach its own inputs (platform.get_task_input(current_task, "factors_file")). The inputs come from request.workflow_params, which is what the run was submitted with. Raises ValueError when WES_RUN_ID is unset, matching what the SevenBridges and Arvados platforms do for their equivalent variables -- a launcher that cannot find itself is misconfigured, not idle. test_current_task_and_users loses its assertIsNone half and becomes test_project_users; the current-task behaviour now has its own two tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing recorded which launcher execution submitted a given run, so a launcher's progress could only be answered by asking the launcher. Adding ParentRunId to the tags submit_task already sends -- ProjectId and TaskName -- links each child back to the launcher run that created it. WES promotes ParentRunId into an indexed column, so the service can roll up launcher progress from persisted rows instead of the launcher exposing a status endpoint, and a restarted launcher can find the work it already submitted. Only set when WES_RUN_ID is present, so a launcher run outside WES (a developer's shell, an external platform) submits exactly the tags it does today. test_submit_task hides the launcher variables so its exact-tags assertion does not depend on the developer's shell. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
When platform detection fails, PlatformFactory logs every environment variable so whoever is debugging can see what was set. That dump goes wherever the container's logs go -- CloudWatch for a launcher running as an AWS Batch job -- which is a wider audience than whoever provisioned the credential, and the launcher's environment now carries WES_SERVICE_KEY alongside the pre-existing NGS360_AUTH_TOKEN and SB_AUTH_TOKEN. Mask values whose variable name contains TOKEN, KEY, SECRET, or PASSWORD, keeping the character count: "set but empty" and "set to a truncated value" are both real misconfigurations, and the dump exists to tell them apart. Everything else is logged unchanged. Adds tests/test_platform_factory.py -- the factory had no test of its own. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The module reads six environment variables across connect(), get_current_task() and submit_task(), so the full contract was only discoverable by reading the whole file. Whoever writes the AWS Batch job definition -- or debugs a launcher that starts and cannot find itself -- needs it in one place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
Author
|
Superseded by #132, opened from the branch on NGS360/PAML directly rather than my fork (which was stale). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Bioinformatics launchers (e.g. the RNA-Seq launcher) will run directly as AWS Batch jobs rather than
wrapped in CWL — see NGS360/GA4GH-WES-API-Service#183
for the corresponding service-side change, which makes a launcher execution a first-class GA4GH WES
run and links every child run it submits back to it via a
ParentRunIdtag. A launcher running thatway has no user session and needs to identify its own run — this PR is what
NGS360Platformneeds tosupport that, once merged alongside #183.
What changed
connect()acceptsWES_SERVICE_KEY(+WES_ON_BEHALF_OF) when noNGS360_AUTH_TOKENis available — the shape WES already provisions for callers with no user sessionto source a bearer token from. A bearer token still wins when both are set: it names one real user,
where a service key holder can assert whatever identity it likes.
get_current_task()now returns aWESTaskbuilt fromWES_RUN_ID, the same shape SevenBridgesuses
TASK_IDfor, instead of unconditionally returningNone. This is what lets a launcher readits own inputs back via
get_task_input.submit_task()tags every submitted run withParentRunIdwhenWES_RUN_IDis set, so WES canderive launcher progress from persisted rows instead of the launcher exposing a status endpoint.
PlatformFactory.detect_platform()now redacts credential-shaped environment variable values(
TOKEN/KEY/SECRET/PASSWORDin the name) before logging them on a detection failure — foundwhile adding
WES_SERVICE_KEYto the set of variables that dump could leak.Not in this PR
Two real bugs in
get_tasks_by_namewere found while doing this work and filed instead of fixed here,to keep this change reviewable:
at most the 10 newest matching runs.
the launcher passes
inputs_to_comparepositionally intoget_tasks_by_name'sworkflowparameter, so the reuse parameter check never runs.
Testing
PYTHONPATH=src pytest— 146 passed, 18 skipped.pylint src tests scripts --max-line-length=120 --ignore-imports=y— 9.86/10, no new message categories vs.main.Opened as a draft: this is the launcher-side counterpart to
GA4GH-WES-API-Service#183 and is not
usable on its own until that lands too.
🤖 Generated with Claude Code