Rules Criteria Documentation Clarification

[AndroxxTraxxon]

I just noticed this note on the doc page for Rules —

Due to a known, reported issue in PyYAML, criteria keys must be unique. This sometimes becomes relevant when you want to apply different operators (like contains and ncontains) to the same trigger data:

criteria:
    trigger.payload.commit.tags:  # duplicate key - ignored!
      type: ncontains
      pattern: StackStorm
    trigger.payload.commit.tags:  # duplicate key - evaluated
      type: contains
      pattern: pull request
    trigger.payload.commit.message:  # unique key - evaluated
      type: ncontains
      pattern: ST2

These are the "known, reported" issues against the PyYAML project:

• Duplicate keys are not handled properly yaml/pyyaml#165
• pyyaml should detect duplicate keys and report an error. yaml/pyyaml#41

You'll note that the actual issue is that in cases like this, instead of throwing an error, PyYAML will overwrite the mapped value with the last one found in the document.

The suggested format indicates a mismatch between in the structure of the rules criteria file format and the intended use. The known, reported issue with PyYAML is that the parser doesn’t throw an error when parsing a document with duplicate keys, not that the data structures are incorrectly interpreted. This suggested use of criteria violates the yaml format requirement that mapping keys be unique, and the workaround is a bit of an antipattern.

Regardless of what StackStorm decides to do with its data format, this kind of note complaining about misinterpreting an invalid yaml file is unnecessary, and explaining it in a way that doesn’t make PyYAML look like the bad guy would be preferred.

This is the suggested workaround, where you can see that the document doesn't violate the YAML format:

criteria:
    trigger.payload.commit.tags#1:
      type: ncontains
      pattern: StackStorm
    trigger.payload.commit.tags#2:
      type: contains
      pattern: pull request
    trigger.payload.commit.message:
      type: ncontains
      pattern: ST2

Clearly, this workaround is already implemented, and it works, but the StackStorm shouldn't make PyYAML out as the bad guy here -- the data format here clearly violates the YAML syntax, and that's not PyYAML's problem.

I opened this issue to recommend changing the phrasing for the explanation around why the workaround is necessary. I'll make a PR when I get the time to update the docs.