search

kestra-io / plugin-scripts

https://kestra.io/plugins/
Apache License 2.0
9 stars 9 forks source link

fix(script): no Docker options when not using the Docker runner #92

Closed yuri1969 closed 7 months ago

yuri1969 commented 10 months ago

What changes are being made and why?

This prevents using incompatible settings with the PROCESS runner.

Note, merging this would bring a breaking change. There might be a better way around this.

close kestra-io/kestra/issues/2132

How the changes have been QAed?

id: hello-world-docker-options
namespace: company.team
tasks:
  - id: docker-only
    type: io.kestra.plugin.scripts.shell.Commands
    runner: PROCESS # FIXME
    docker:
      image: foo
    commands:
      - echo 'bar'
loicmathieu commented 10 months ago

Hi, Thanks for your contribution, this is indeed the way to do such validation.

However, as you said, this would be a breaking change, we'll discuss it internally.

loicmathieu commented 10 months ago

Hi, So we decided to postpone a little as we're not sure if we want it to fail the validation or just add a warning, which is not yet possible with our current validation implementation.

yuri1969 commented 10 months ago

Understood, having OK/warning/error validation states available would be useful.

anna-geller commented 7 months ago

closing as it will no longer be an issue with Task Runners https://develop.kestra.io/docs/concepts/task-runners

docker property will no longer be available when using the Process task runner

Thanks so much for flagging this issue @yuri1969, it helped to shape the implementation of task runners