Use the Seqera Platform default workspace when none is set locally - #7453
Draft
ewels wants to merge 7 commits into
Draft
Use the Seqera Platform default workspace when none is set locally#7453ewels wants to merge 7 commits into
ewels wants to merge 7 commits into
Conversation
`nextflow launch` and the `-with-tower` monitoring path previously fell back to the user's Personal workspace whenever no workspace was configured locally. Seqera Platform now exposes a per-user (and system) default workspace as the top-level `defaultWorkspaceId` field of the `GET /user-info` response. Read that field and use it as the fallback when no workspace is set via config (`tower.workspaceId`), environment (`TOWER_WORKSPACE_ID`), or the `-workspace` flag. Local/CLI settings still take precedence, and Platform-driven runs (with `TOWER_WORKFLOW_ID` set) are unaffected. The lookup is null-safe, so Platform versions predating the field degrade cleanly to the Personal workspace. - TowerClient.getDefaultWorkspaceId() reads the new field defensively - LaunchCommandImpl.resolveWorkspaceId() falls back to it before returning null - TowerFactory.create() applies it to the monitoring observer - `nextflow auth status` surfaces the Platform default workspace - docs and tests updated Assisted-by: Claude Code Signed-off-by: Claude <noreply@anthropic.com>
Follow-up cleanup on the Platform default-workspace change. The fallback was implemented independently in three places, which had already produced divergent behaviour, and the /user-info response was fetched more than once per command. - Add PlatformHelper.getEffectiveWorkspaceId(), the single definition of "a local setting wins, otherwise use the Platform default". The Platform lookup is passed in as a supplier so PlatformHelper stays free of I/O and of any dependency on the API client (nf-wave has no TowerClient). - Add PlatformHelper.isPlatformRun() and route the four getters plus the new call site through it, so the TOWER_WORKFLOW_ID rule has one definition. - Route TowerFactory, LaunchCommandImpl and AuthCommandImpl through the shared resolver. This also fixes `nextflow launch` ignoring TOWER_WORKSPACE_ID, which the docs already claimed was honoured. - Memoize the GET /user-info fetch in TowerClient so getUserInfo() and getDefaultWorkspaceId() share one round-trip: `nextflow auth status` drops from 3 identical /user-info calls to 1. - Collapse the duplicated status-rendering branch in AuthCommandImpl and reuse the already-created client for the compute-env lookup. - Document the new `platform` source value and align the TowerConfig workspaceId description with the docs. Assisted-by: Claude Code Signed-off-by: Claude <noreply@anthropic.com>
Second cleanup pass on the Platform default-workspace change. The resolved workspace was handed only to TowerObserver, so three subsystems that scope themselves to the workspace still resolved it independently and saw the personal workspace while the run was reported into the Platform default: the Fusion licence request (a hard abort), Wave registry credentials (private pulls failing as unauthorized) and the Seqera executor's run creation. This hit exactly the target user: an org member with no local tower.workspaceId. TowerFactory now publishes the resolved value into session.config.tower. workspaceId, following the existing WaveFactory precedent of a trace observer factory writing resolved facts back into the session config. All four consumers already read through PlatformHelper.getWorkspaceId(session.config.tower, env), so they are fixed without any edit at the consumers. The lookup also ran with the run-critical retry policy (10 attempts, up to 90s apart) on the session-init path, so an unreachable endpoint could stall the start of a run for minutes before falling back to the personal workspace anyway. It now uses a dedicated client bounded to a single attempt and a 10s timeout, matching its best-effort contract. Note a user-visible precedence change: `-workspace <name>` now takes priority over the `tower.workspaceId` config setting, where previously the config won. An explicit CLI flag beating a config file is the conventional order; it is now covered by a test and the full precedence is documented. Also in this pass: - getDefaultWorkspaceId returns String, matching every other workspace-id accessor and removing a Long/String round-trip - towerOpts moves to PlatformHelper so the two remaining hand-rolled copies of the `tower.` prefix stripping can use it, including the one in modules/nextflow - `nextflow auth config` no longer labels the no-selection case "Personal workspace" when an account default exists, and uses the isPlatformRun seam - memoize listUserWorkspacesAndOrgs: `launch -workspace NAME` fetched it twice - drop a redundant local, an inert branch and a stray null-guard in auth status Assisted-by: Claude Code Signed-off-by: Claude <noreply@anthropic.com>
Third cleanup pass on the Platform default-workspace change.
The previous commit published the resolved workspace into the session config
unconditionally, so it also rewrote values the user had set: on a Platform-driven
run with `tower.workspaceId` in the config and TOWER_WORKSPACE_ID in the
environment it replaced the former with the latter, and with only the env var set
it invented a config key nobody wrote. That was harmless only because
PlatformHelper.getWorkspaceId ignores the config on a Platform-driven run -- the
safety came from a rule enforced somewhere else rather than from this code. The
publish is now guarded on there being no local setting at all, which is the only
case the mechanism exists for, and it logs the workspace it picked so a run
landing in an unconfigured workspace is no longer unexplained.
The bounded lookup added in the previous commit had no test: every test stubbed
the seam itself, so renaming a timeout field in TowerConfig would have silently
restored the 10-attempt, ~3-minute retry policy on the session-init path with a
green build. The bounding now lives in TowerConfig.forLookup(), which is pure and
directly tested, and is shared by all three sites that make this lookup --
`nextflow launch` and `nextflow auth status` were still using the unbounded
policy for the same best-effort call.
Also fixes a pre-existing trap that this change depends on: TowerRetryPolicy
resolved its defaults with the elvis operator, and 0 is falsy in Groovy, so
`maxAttempts = 0` ("do not retry") silently became 10 attempts and `jitter = 0`
became 0.25. Values are now resolved with explicit null checks, and a maxAttempts
that would never run the operation is rejected rather than silently replaced.
Smaller items: reuse PlatformHelper.towerOpts for the last hand-rolled copy of
the prefix stripping; note at the Wave, Fusion and Seqera-executor read sites
that the workspace may have been resolved during session init; make the
"nothing was published" assertion able to fail; drop a test wholly subsumed by
another and a stub whose arity never matched.
Assisted-by: Claude Code
Signed-off-by: Claude <noreply@anthropic.com>
Fourth cleanup pass on the Platform default-workspace change. The previous commit routed the CLI commands through a separate bounded client for the default-workspace lookup. That turns out to cost without buying anything: `nextflow launch` and `nextflow auth status` both make other calls through the shared client in the same flow, and those keep the default 10-attempt retry policy, so the command can still hang for minutes regardless. Meanwhile the second client has its own `describeUser()` memo and its own connection pool, so each command paid an extra `GET /user-info` and an extra TLS handshake. Both CLI sites go back to the shared client, and the bounded config is kept for `TowerFactory` alone, where the lookup runs unattended during session init and there is no earlier unbounded call. An invalid `tower.retryPolicy.maxAttempts` now warns and falls back to a single attempt instead of aborting. Throwing would have broken configs that work today -- `maxAttempts = 0` is currently ignored and silently means 10 -- and the default-workspace feature never depended on that validation, since the bounded config passes an explicit 1. Note the same falsy-elvis trap exists in six sibling retry classes (nf-wave, nf-seqera, nf-k8s, nf-google, nf-azure and the SRA datasource). They are left alone here: routing them all through a shared helper is its own change. Also: document the session-config coupling once on the javadoc of PlatformHelper.getWorkspaceId -- the function all the affected consumers call -- instead of three copies of a comment that the previous commit had already made stale; drop the BaseCommandImpl.towerOpts pass-through in favour of the PlatformHelper static; correct the lookup timeout doc, which overstated the worst case; remove a dead import and collapse two duplicated retry tests. Assisted-by: Claude Code Signed-off-by: Claude <noreply@anthropic.com>
✅ Deploy Preview for nextflow-docs canceled.
|
Member
Author
|
Ran the skill mentioned by @bentsherman in Slack to generate this explainer HTML: |
Resolves conflicts with the "Separate CLI from runtime" refactor (#5971), which split the CLI out of the runtime module and renamed the extension point types: nextflow.cli.CmdLaunch.LaunchCommand became nextflow.cli.LaunchCommand, CmdLaunch.LaunchOptions became nextflow.cli.LaunchOptions, and CmdAuth moved to the new nf-cli-v1 module. Both conflicts were import-only, in LaunchCommandImpl and its test: this branch had added `nextflow.SysEnv` and `nextflow.platform.PlatformHelper` alongside the old `CmdLaunch` import that master replaced. Kept the new imports and took master's renamed types. Assisted-by: Claude Code Signed-off-by: Claude <noreply@anthropic.com>
When a run is refused by Seqera Platform because the token cannot launch in the target workspace, Nextflow dumped the raw HTTP response. That reads as an unexpected transport error when it is in fact a common, well understood condition with a clear remedy. Every workspace reference was also a bare 14-digit ID that nobody can map to a workspace by eye. Diagnostics only -- no behavioural change. The abort on a refused workspace is still correct; it is now self-explanatory. TowerClient.workspaceAccessError explains a 403/404 on a workspace-scoped call, using the user's workspace list to separate the two causes: a workspace the account can see exists, so a refusal means the role is insufficient; one it cannot see means the ID is not usable by this account. The message also says whether the workspace came from the Platform account default, because that decides the fix, and links to the roles documentation. When the visibility lookup itself fails, no cause is claimed and the raw response is reported as before -- likewise for a 401, which says nothing about the workspace. traceProgress now shares the same error-message construction rather than duplicating it. TowerClient.workspaceLabel renders "123 [org / workspace]" for the sites that show an ID to a human. It is best-effort: it never throws and degrades to the bare ID, which is itself a signal that the workspace is not visible. It is applied where the data is already fetched or the message warrants the lookup -- the account-default log line and the new errors -- and deliberately not on debug lines that would otherwise pay for API calls unconditionally. Assisted-by: Claude Code Signed-off-by: Claude <noreply@anthropic.com>
Member
Author
Blocked:
|
ewels
marked this pull request as draft
August 8, 2026 16:15
Member
Author
|
Now discussing internally on slack. |
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
2 participants
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
Nextflow's local workspace selection —
nextflow auth config,tower.workspaceId, the-workspaceflag — exists largely because Seqera Platform historically had no concept of a default workspace. Every user had to tell Nextflow, on every machine, something Platform could not tell it.Platform now has that concept: since API v1.173.0 the
GET /user-inforesponse carries a top-leveldefaultWorkspaceId(a per-user preference, falling back to a global system default, and access-validated server-side).This PR teaches Nextflow to pick it up. Local configuration is untouched and still wins. The only behaviour that changes is the fallback: where Nextflow previously jumped straight to the personal workspace, it now asks Platform first.
The practical effect: an org user who never ran
nextflow auth configgets their runs in the right workspace instead of silently in Personal.Discoveries along the way
Three things surfaced that were not part of the original ask, and two of them mattered more than the feature:
The workspace was resolved in more places than expected. Making the observer aware of the default is not enough —
TowerFusionToken, nf-wave'sTowerConfigandSeqeraExecutoreach resolve the workspace independently. Left alone, a run would be reported into the Platform default while its Fusion licence was requested against Personal (a hard abort, surfacing as "Unable to validate Fusion license"), its Wave registry credentials looked up in Personal, and its scheduler run created in Personal. This hits precisely the users the feature targets. Fixed by resolving once and publishing the result, so every subsystem reads the same value.nextflow launchnever honouredTOWER_WORKSPACE_ID.readConfig()reads the config file only. Harmless before — the env var was ignored and you got Personal — but once a Platform default existed, ignoring it would have sent runs to a different real workspace. Fixed by routing the launch path through the same precedence ladder as everything else.A best-effort lookup was inheriting a run-critical retry budget. The new call sits in
Session.init(), before the pipeline script is parsed, and initially used the telemetry retry policy: 10 attempts with exponential backoff. An unreachable Platform could have stalled the start of every run for minutes before falling back to Personal anyway. Now bounded to a single attempt with a short timeout.Implementation and decisions
Resolution policy lives in one place
PlatformHelper.getEffectiveWorkspaceId(opts, env, Supplier<String>)is the single definition of "local wins, otherwise use the Platform default". The Platform lookup is passed in as a supplier soPlatformHelperstays free of I/O and of any dependency on the API client — nf-wave consumes it and has noTowerClient. A siblingisPlatformRun(env)gives theTOWER_WORKFLOW_IDrule one definition instead of five.A Platform-driven run is authoritative: when
TOWER_WORKFLOW_IDis set, the environment decides and the account default is never consulted.Publishing the resolved value
TowerFactory.create()writes the resolved workspace intosession.config.tower.workspaceId. All four consumers already read throughPlatformHelper.getWorkspaceId(session.config.tower, env), so they are fixed with no edits at the consumers. There is precedent:WaveFactoryalready writes resolved facts back intosession.config.It publishes only when the user set nothing — it never overwrites a configured value, and never invents a config key. A
log.infonames the workspace, so a run landing somewhere the user did not configure is explained rather than mysterious.Resolution stays in
TowerFactoryrather than moving toSession.init: it is correctly gated behindisEnabled(), so a plain local run that merely has a token inseqera-auth.configpays nothing.Bounding the lookup
TowerConfig.forLookup()produces a config with one attempt and a 10s timeout, used for the session-init lookup. The CLI commands deliberately do not use it:nextflow launchandnextflow auth statusmake other calls through the shared client in the same flow that keep the default retry policy, so bounding just this one call buys no real guarantee while costing an extra/user-infofetch and TLS handshake (a second client has its own memo and connection pool).Other changes
TowerClient.describeUser()is memoized sogetUserInfo()andgetDefaultWorkspaceId()share one round-trip —nextflow auth statusdrops from 3 identical/user-infocalls to 1.listUserWorkspacesAndOrgslikewise (launch -workspace NAMEfetched it twice).nextflow auth statusreports sourceplatformwhen the value came from the account default;nextflow auth configno longer labels the no-selection case "Personal workspace" when a default exists.null, which degrades cleanly to the personal workspace. The lookup swallows failures — it must never abort a run.Behaviour change worth flagging
-workspace <name>now takes priority overtower.workspaceId, where previously the config setting won. An explicit CLI flag beating a config file is the conventional order, and the full precedence is now documented and covered by a test — but it is a user-visible change.Notes for reviewers
PlatformHelper.getWorkspaceId's javadoc documents the session-config coupling, since that is the function every affected consumer calls.TowerRetryPolicy: defaults were resolved with the elvis operator, and0is falsy in Groovy, somaxAttempts = 0("don't retry") silently became 10 andjitter = 0became0.25. Invalid values now warn and fall back rather than aborting a run, since configs relying on the old silent behaviour work today.Follow-ups (deliberately not in this PR)
session.workflowMetadata.platform, which already carriesworkflowIdandworkflowUrl— a typed channel instead of a config-map key. Larger blast radius across three plugins.Testing
Unit tests cover the precedence ladder, the publication (including that it is a no-op when the user configured a workspace), the bounded lookup config, and the
auth statusdisplay. Full suites pass for nf-tower, nf-wave, nf-seqera and the affected core tests.Not yet exercised against a live Platform account with a default workspace set — worth a manual check before merge that Wave and Fusion land in the same workspace as the run.
Generated by Claude Code