-
Notifications
You must be signed in to change notification settings - Fork 19
Does not validate Azure or Google blob storage #171
Does not validate Azure or Google blob storage #171
Conversation
Azure and Google blob storage failed to validate if it was a directory or path. This PR skips validating directories so it does raise an inappropriate error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
(waiting for the tests, then I'll merge and release) |
Before we merge - is there a way of checking my regex? I couldn't see a good test to add it to. |
You could add a dummy |
yeah but, how do I test it? I've never used a custom plugin before 😅 |
Ah hahahah you can copy paste a test in this file. Feel free to ask me if something is not clear in it :) |
OK first attempt here: f70cecc |
The schema you used doesn't actually check for file existence :p can you use or create another one? |
Corrected? 38b48b7 |
Should we also do this whilst we're here? nextflow-io#37 |
There's also no Also better to not use a schema that has the It does look like no "type": "string",
"format": "file-path",
"exists": true |
Good idea! |
@adamrtalbot can you add this line to this file?
|
get yer own PR: #172 |
@nvnieuwk looks good now! can you give it a final check for me? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome looks good!
Can I merge this? |
I think so! I haven't been able to test for real yet 😬 |
Eh it should be fine :) |
Given the history of this plugin, please test it 🙈 😆 |
@adamrtalbot can you run a test? I don't have access to azure or google blob :) |
I'll give it a try but I've never done plugin development so might take me some time. Of course you don't actually need a file, you just need a parameter of type file = |
Dur, in |
|
|
What is your java version? |
java 17.0.10-tem Both installed with sdkman |
Can you try again with an older version (the plugin has been developed in java |
No dice! This is on a Mac using Sonoma 14.5 btw.
|
Yeah I've no idea if it's because of some Mac issue or something else :/. It's weird because the tests are running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some logging adjustments
...ns/nf-validation/src/main/nextflow/validation/FormatValidators/DirectoryPathValidator.groovy
Outdated
Show resolved
Hide resolved
.../nf-validation/src/main/nextflow/validation/FormatValidators/FilePathPatternValidator.groovy
Outdated
Show resolved
Hide resolved
plugins/nf-validation/src/main/nextflow/validation/FormatValidators/FilePathValidator.groovy
Outdated
Show resolved
Hide resolved
plugins/nf-validation/src/main/nextflow/validation/FormatValidators/PathValidator.groovy
Outdated
Show resolved
Hide resolved
plugins/nf-validation/src/main/nextflow/validation/SchemaValidator.groovy
Outdated
Show resolved
Hide resolved
…ators/DirectoryPathValidator.groovy Co-authored-by: Nicolas Vannieuwkerke <[email protected]>
Co-authored-by: Nicolas Vannieuwkerke <[email protected]>
…ators/FilePathPatternValidator.groovy Co-authored-by: Nicolas Vannieuwkerke <[email protected]>
Demonstrated to work:
include { validateParameters } from 'plugin/nf-validation'
workflow {
validateParameters()
println("Hi ${params.input}")
}
{
"$schema": "http://json-schema.org/draft-07/schema",
"$id": "https://raw.githubusercontent.com/nf-core/testpipeline/master/nextflow_schema.json",
"title": "nf-core/testpipeline pipeline parameters",
"description": "this is a test",
"type": "object",
"definitions": {
"file_patterns": {
"title": "Input/output options",
"type": "object",
"fa_icon": "fas fa-terminal",
"properties": {
"input": {
"type": "string",
"format": "file-path",
"exists": true,
"default": "az://igenomes/"
}
}
}
},
"allOf": [
{
"$ref": "#/definitions/file_patterns"
}
]
}
With [email protected]:
With [email protected]:
|
Azure and Google blob storage failed to validate if it was a directory or path. This PR skips validating directories so it does raise an inappropriate error.