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

add parameters from schema top level as expected params #79

Merged
merged 2 commits into from
Aug 21, 2023

Conversation

mirpedrol
Copy link
Collaborator

Parameters defined in the schema top level or within definitions are considered expectedParams.

{
  "$schema": "http://json-schema.org/draft-07/schema",
  "type": "object",
  // Top level params
  "properties": {
    "test_param_1": { 
          "type": "string"
        }
  },
  // Definition groups
  "definitions": { 
    "my_group_of_params": { 
      "title": "A virtual grouping used for docs and pretty-printing",
      "type": "object",
      "required": ["foo", "bar"], 
      "properties": { 
        "foo": { 
          "type": "string"
        },
        "bar": {
          "type": "string"
        }
      }
    },
    // Params within definitions but not inside one of the definitions
    "properties": {
      "test_param_2": { 
        "type": "string"
      }
    }
  },
  // Contents of each definition group brought into main schema for validation
  "allOf": [
    { "$ref": "#/definitions/my_group_of_params" } ,
    { "$ref": "#/definitions" } 
  ]
}

@ewels
Copy link
Member

ewels commented Aug 18, 2023

// Params within definitions but not inside one of the definitions

This one is non-valid JSON schema. It would be parsed as a definition group called properties, then containing unrecognised keys.

I don't think that we need to consider this scenario? Just within definitions (as already done) and in the top level.

Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! I think that part of the support you've added is probably overkill and not needed, but still fine either way 👍🏻 😃

Comment on lines 290 to 295
// Collect expected parameters from the schema when parameters are specified within "definitions"
if (schemaParams.containsKey('properties')) {
def enumsTupleDefinitions = collectEnums(['definitions': ['properties': schemaParams.get('properties')]])
expectedParams += (List) enumsTupleDefinitions[0]
enums += (Map) enumsTupleDefinitions[1]
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that we need this one..

@mirpedrol mirpedrol merged commit 8a126eb into nextflow-io:master Aug 21, 2023
3 checks passed
@mirpedrol mirpedrol deleted the fix-definitions-params branch August 21, 2023 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants