Skip format validation for cloud storage paths (S3, Azure, GCS) #191
Conversation
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
pinin4fjords
force-pushed
the
fix/cloud-storage-path-validation
branch
from
5ac6f8f to
fd3606b
Compare
Cloud storage paths (s3://, az://, gs://) may fail validation when users don't have credentials or permissions to access them. This causes errors like: "could not validate file format of 's3://...': Unable to marshall request to JSON: Key cannot be empty" This fix takes a targeted approach: - Still attempt validation for cloud paths (preserves validation for users with proper access) - Catch exceptions from exists()/isDirectory() calls for cloud paths and skip validation gracefully instead of failing - Also handle the case where cloud storage paths report incorrect file/directory type (similar to PR nextflow-io#179's Azure fix) This allows pipelines with default cloud storage paths to be launched without requiring access to those paths during parameter validation, while still validating when access is available. Fixes nf-core/sarek#2079 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
pinin4fjords
force-pushed
the
fix/cloud-storage-path-validation
branch
from
fd3606b to
f22ddb5
Compare
Complete the cloud storage path graceful handling by adding the same pattern to ExistsEvaluator. This ensures that the `exists: true` schema keyword also handles inaccessible cloud paths (S3, Azure, GCS) without failing validation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The try/catch blocks correctly handle inaccessible cloud paths by skipping validation when access fails. However, the additional checks that gave cloud paths a free pass on legitimate validation failures (e.g., "path is a file not a directory") were too aggressive. If we can successfully access a cloud path and determine its type, we should validate it properly - not skip the check. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move duplicated isCloudStoragePath() function to Common.groovy to eliminate code duplication across 5 evaluator classes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When catching exceptions for cloud storage paths, we don't actually know if it's a credentials issue - could be malformed path, network error, etc. Updated log messages to say "due to exception" instead of claiming the path is "inaccessible". 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Author
|
Closing this PR after further analysis. The root cause is that Nextflow/AWS SDK throws unhelpful exceptions for missing credentials, and nf-schema catching these exceptions is a workaround that might hide legitimate errors. A better fix would be in Nextflow core - e.g., providing a way to check cloud path accessibility without throwing, or better exception types that distinguish 'no credentials' from 'malformed path'. Will open an issue against Nextflow instead. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
Gracefully handle inaccessible cloud storage paths in format validation, instead of failing with cryptic errors.
Problem
Cloud storage paths (
s3://,az://,gs://) may fail validation when users don't have credentials or permissions to access them. This causes errors like:This affects multiple nf-core pipelines that have default S3 paths for resources like annotation caches or iGenomes.
Why this affects multiple pipelines
The issue is triggered by
format: directory-path(and similar format keywords) even withoutexists: true. The format evaluator callsfile.exists()to verify the path is accessible and is actually a directory:format: directory-path+exists: trueformat: directory-pathonlySame symptom, slightly different triggers - but both fixed by handling inaccessible cloud paths gracefully in the evaluators.
Solution
Take a targeted approach that preserves validation when possible:
exists()/isDirectory()calls for cloud paths and skip validation gracefully instead of failingThis means:
Changes
isCloudStoragePath()helper toCommon.groovy(single source of truth)FormatDirectoryPathEvaluator.groovy- Graceful handling for inaccessible cloud pathsFormatFilePathEvaluator.groovy- Graceful handling for inaccessible cloud pathsFormatFilePathPatternEvaluator.groovy- Graceful handling for inaccessible cloud pathsExistsEvaluator.groovy- Graceful handling for inaccessible cloud pathsSchemaEvaluator.groovy- Graceful handling for inaccessible cloud pathsRelationship with PR #179
PR #179 addresses a related issue specifically for Azure. This PR:
Related
Test plan
🤖 Generated with Claude Code