Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove storage account name if present from azure path #4692

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

endre-seqera
Copy link

Closes: #4683

Remove storage account name if present from azure path

Tests

Tested using nextflow-publishdir pipeline.

Before:

nextflow run https://github.com/endre-seqera/nextflow-publishdir -r main --outdir "az://nfazurestore.eend-test" #FAILS

ERROR ~ Error executing process > 'TEST_PUBLISH_DIR (3)'

Caused by:
  /nfazurestore.eend-test: Unable to determine if root directory exists


 -- Check '.nextflow.log' file for details

After:

 ./launch.sh run -c ~/azure-nextflow.config https://github.com/endre-seqera/nextflow-publishdir -r main --outdir "az://nfazurestore.eend-test"
[00/6cfd76] process > TEST_PUBLISH_DIR (3) [100%] 3 of 3 ✔
Screenshot 2024-01-26 at 14 32 03

Copy link

netlify bot commented Jan 26, 2024

Deploy Preview for nextflow-docs-staging canceled.

Name Link
🔨 Latest commit 4704d95
🔍 Latest deploy log https://app.netlify.com/sites/nextflow-docs-staging/deploys/65bcbd679c18c90007d0dc7a

Signed-off-by: endre-seqera <endre.sukosd@seqera.io>
@endre-seqera endre-seqera force-pushed the publishdir-azure-storage-account-name-remove branch from aee2c56 to fcd676f Compare January 26, 2024 16:09
Signed-off-by: endre-seqera <endre.sukosd@seqera.io>
@endre-seqera endre-seqera force-pushed the publishdir-azure-storage-account-name-remove branch from f64509e to 4704d95 Compare January 26, 2024 16:48
@pditommaso
Copy link
Member

Provided this may work, i'm bit concerned altering the container aka bucket name under the hood. This could result having nextflow reporting a different object file path compared to the one specified by the user, in the trace files, report, and other provenance record.

It may be worth to add full support for it

@endre-seqera
Copy link
Author

endre-seqera commented Jan 29, 2024

Provided this may work, i'm bit concerned altering the container aka bucket name under the hood. This could result having nextflow reporting a different object file path compared to the one specified by the user, in the trace files, report, and other provenance record.

I understand your concern, but the bucket (container) name is not altered, only the additional information (storage account name) is removed. So object file path will still be correct, just missing this extra information.

But this extra information is redundant in a way, because it is already known to the user, since the user has to configure it with their azure credentials in the nextflow.config, so the user should not doubt which storage account is being used:

 azure {
   storage {
     accountName = '<YOUR STORAGE ACCOUNT NAME>'
   }
 }

The "convention" used in SeqeraPlatform "${providerSchema}://${storageName}.${bucket}" is arbitrary, it is just to have a unique path and differentiate between potentially duplicated bucket (container) names in different storage accounts (like in Data Explorer, where results from multiple credentials can show up).

But no one is expecting azure paths in this format, and for a given, specific nextflow configuration (or for one specific credential) this information (storage account name) is not necessary.

@endre-seqera endre-seqera force-pushed the publishdir-azure-storage-account-name-remove branch from 456c4c3 to 4704d95 Compare February 2, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

publishDir compatibility with Azure Blob Storage
3 participants