search

treasure-data / digdag

Workload Automation System
https://www.digdag.io/
Apache License 2.0
1.3k stars 221 forks source link

digdag should fail on multiple operators within a task #21

Closed danielnorberg closed 8 years ago

danielnorberg commented 8 years ago

I.e., the below workflow definition should fail instead of silently just executing echo bar.

run: +foo
+foo:
  sh>: echo bar
  py>: baz

Also, it might be worth considering if multiple identical keys should fail. In the below workflow definition the second sh> operator implicitly overrides the first operator when the yaml is parsed because yaml.

run: +foo
+foo:
  sh>: echo bar
  sh>: echo baz
rbparrish commented 8 years ago

From how I think about the syntax, I feel multiple identical operators should also fail. Only one primary operator per task (not including the case of _check: & _error: usage, or multiple tasks within a task)

danielnorberg commented 8 years ago

Yeah, tricky thing is that the yaml parsing happens before validation, and at that point the parser has already discarded all preceding identical keys.

danielnorberg commented 8 years ago

Turns out though that the yaml parser can, at gunpoint, be persuaded into disallowing duplicate keys by injecting some validation logic. Kind of icky but it gets the job done.

frsyuki commented 8 years ago

SnakeYAML should have an option that throws an exception. It's good for all users. SnakeYAML is not on github (BitBucket!) but the author is still active.

danielnorberg commented 8 years ago

Yeah, let's submit a SnakeYAML PR.

danielnorberg commented 8 years ago

I opened an issue in the SnakeYAML project to propose adding this validation to SnakeYAML itself: https://bitbucket.org/asomov/snakeyaml/issues/337/option-to-disallow-duplicate-keys

Meanwhile, it seems that the very same issue has been addressed in the past by implementing a custom constructor, similarly to what my current PR does: https://jira.spring.io/browse/SPR-12318

Should we go ahead with my PR until we have managed to get unique key validation into SnakeYAML proper?