Align Seqera executor maxSpotAttempts and Fusion handling - #7412
Merged
Conversation
Make the Seqera executor follow the same pattern used by AWS Batch and Google Batch: - The spot attempts policy now lives in a `maxSpotAttempts` method in `SeqeraTaskHandler`, matching `AwsBatchTaskHandler` and `GoogleBatchTaskHandler`: the config value wins, otherwise fall back to `FusionConfig.DEFAULT_SNAPSHOT_MAX_SPOT_ATTEMPTS` when Fusion snapshots are enabled. `SchemaMapperUtil` just maps the resolved value. - The Fusion snapshot flag is guarded by `fusionEnabled()` at the call site, as done by the other executors. - `SeqeraExecutor.isFusionEnabled` now simply delegates to `FusionHelper.isFusionEnabled` instead of throwing from a boolean getter; the "requires Fusion" check is done in `register`. Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Assisted-by: Claude Code (Opus 5) Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
✅ Deploy Preview for nextflow-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
bentsherman
reviewed
Jul 30, 2026
Fusion is mandatory for the Seqera executor -- `register` aborts when it is not enabled -- so `fusionEnabled()` is always true by the time a task is submitted. Test the snapshot flag directly instead, both at the `toMachineRequirement` call site and in `maxSpotAttempts`. Also move the misplaced `createHandlerWithError` javadoc back onto its method. Assisted-by: Claude Code (Opus 5) Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
A negative `machineRequirement.maxSpotAttempts` was silently dropped: `maxSpotAttempts` returned it as-is and `SchemaMapperUtil` then mapped any non-positive value to null. Fail early instead, using the same `IllegalArgumentException` style as the neighbouring machine requirement validation in `SchemaMapperUtil`. Assisted-by: Claude Code (Opus 5) Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
stefanoboriero
approved these changes
Aug 4, 2026
bentsherman
approved these changes
Aug 4, 2026
bentsherman
left a comment
bentsherman
left a comment
Member
There was a problem hiding this comment.
CI failure looks like a flaky cloudinfo test
Member
Author
|
Weird, twice in a row on the Java 25 suite |
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
3 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.
Make the Seqera executor follow the same pattern used by AWS Batch and Google Batch.
maxSpotAttemptsmethod inSeqeraTaskHandler, mirroringAwsBatchTaskHandlerandGoogleBatchTaskHandler: the config value wins, otherwise fall back toFusionConfig.DEFAULT_SNAPSHOT_MAX_SPOT_ATTEMPTSwhen Fusion snapshots are enabled.SchemaMapperUtilno longer owns that policy, it just maps the resolved value. The method takes the hint-overlaidMachineRequirementOptssince Seqera resolvesseqera/machineRequirement.*hints per task.fusionEnabled()at the call site, as done by the other two executors.SeqeraExecutor.isFusionEnablednow simply delegates toFusionHelper.isFusionEnabledinstead of throwing from a boolean getter; the "requires Fusion" abort moved toregister.Behavior is unchanged in practice — Fusion is mandatory for this executor, so the new guard is always true. This aligns the structure and adds test coverage for the spot attempts resolution.
🤖 Generated with Claude Code