Open
Conversation
- Fix listContainers0() discarding results (returned empty list) - Fix NPE when Azure task failureInfo is null - Remove dead setPermissions(BLOB_PERMS) overwritten by CONTAINER_PERMS - Fix hdi_isfolder truthy string check for ADLS Gen2 paths - Wire up env var fallbacks in AzManagedIdentityOpts per documentation Generated by Claude Code Signed-off-by: adamrtalbot <12817534+adamrtalbot@users.noreply.github.com>
✅ Deploy Preview for nextflow-docs-staging canceled.
|
Collaborator
Author
|
Sorry, I know this isn't a focused PR but these seemed like a grab bag of miscellaneous bugs I could fix quickly! |
pditommaso
force-pushed
the
master
branch
2 times, most recently
from
d9fa5cd to
d752bc2
Compare
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.
Summary
Fixes 5 correctness bugs in the nf-azure plugin identified during a technical debt audit. All existing tests pass.
Bug Fixes
🔴 Critical
AzFileSystem.listContainers0()returned empty list — TheforEachiterated over blob containers but discarded the result ofprovider.getPath()instead of adding it to thecontainerslist. Any code path relying on container listing at the filesystem level silently received nothing.NPE in
AzBatchTaskHandlerfailure handling —info.failureInfo.messagewas accessed without null-checkingfailureInfo. When Azure returns a failed task without failure detail, this throws an NPE. Fixed with safe navigation (?.).SAS token permissions silently wrong in
AzHelper—.setPermissions(BLOB_PERMS).setPermissions(CONTAINER_PERMS)caused the second call to overwrite the first, makingBLOB_PERMSdead code. Since this generates a container-level user delegation SAS, onlyCONTAINER_PERMSis needed. Removed the dead call.🟡 Moderate
ADLS Gen2
hdi_isfoldertruthy string bug inAzFileAttributes—directory = meta.get("hdi_isfolder")assigned a String to a boolean field. In Groovy, the string"false"is truthy (non-null, non-empty), so ADLS Gen2 paths withhdi_isfolder=falsemetadata were incorrectly treated as directories. Fixed to use"true".equalsIgnoreCase(...).AzManagedIdentityOptsignored documented env var fallbacks — The@Descriptionannotations documentAZURE_MANAGED_IDENTITY_USERandAZURE_MANAGED_IDENTITY_SYSTEMenv var fallbacks, but the constructor never read them. Wired up the env var fallbacks to match documented behavior.Files Changed
plugins/nf-azure/src/main/nextflow/cloud/azure/nio/AzFileSystem.groovyplugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzBatchTaskHandler.groovyplugins/nf-azure/src/main/nextflow/cloud/azure/batch/AzHelper.groovyplugins/nf-azure/src/main/nextflow/cloud/azure/nio/AzFileAttributes.groovyplugins/nf-azure/src/main/nextflow/cloud/azure/config/AzManagedIdentityOpts.groovyTesting
./gradlew :plugins:nf-azure:test— all tests passNXF_SMOKE=1 ./gradlew :plugins:nf-azure:test— all smoke tests pass