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

lint does not allow task output keys in parameter_meta #3

Closed
williamrowell opened this issue Apr 14, 2024 · 2 comments
Closed

lint does not allow task output keys in parameter_meta #3

williamrowell opened this issue Apr 14, 2024 · 2 comments

Comments

@williamrowell
Copy link

Thanks for the nice tool!

Trying it out for automated testing, and noticed that sprocket lint does not allow task outputs as parameter_meta keys.

From the WDL spec v1.0:

Additional requirement: Any key in this section MUST correspond to a task input or output.

warning[v1::W003::Completeness/Medium]: extraneous parameter meta within task: stat_sv_INS_count
    ┌─ workflows/wdl-common/wdl/tasks/bcftools.wdl:365:3
    │
365 │         stat_sv_INS_count: {
    │         ^^^^^^^^^^^^^^^^^ A parameter meta entry with no corresponding input parameter was detected
    │
    = fix: Remove the parameter meta entry
@a-frantz
Copy link
Member

Hi @williamrowell !

You're bumping up against what I understand to be a controversial opinion of our team (and therefore sprocket). We don't like having our "outputs" documented under a section titled "parameter"_meta. Of course, that is where the official docs suggest they be documented.

I have opened an issue with the WDL maintainers asking for this section to be revisited in a future version of the specification.

While dealing with the current spec, we suggest outputs be documented under an outputs key in the meta section.

You have also encouraged us to adopt some changes to our tooling. Unfortunately I can't provide an estimate as to when these changes will hit release status. But, here's our tentative plan of attack to keep everyone happy:

  1. The extraneous_parameter_meta rule you are seeing will be reworked into non_input_parameter_meta
  2. We would add a new rule, non_input_or_output_parameter_meta
  3. The above two rules would be mutually exclusive (only one could be actively run at a time)
  4. Users will be able to toggle between these rules (or disable both if they want)

As you've probably noticed, the user configuration available in sprocket is limited. The above plan requires changes to the underlying architecture which will surface more configuration options. Those changes are coming! I'll make sure we link any relevant Pull Requests back to this issue so you can keep track 😃

@a-frantz
Copy link
Member

a-frantz commented Jul 1, 2024

@williamrowell the newest release has added the ability to disable specific lints. Please let us know how that works out for you!

I'm going to close this issue as being completed by the latest release --except feature. I'm also going to open up a new issue in our upstream wdl repo to better address the actual rules here.

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

No branches or pull requests

2 participants