muesli / beehive

A flexible event/agent & automation system with lots of bees 🐝
GNU Affero General Public License v3.0
6.32k stars 324 forks source link

YAML support for the file backend #339

Closed orsinium closed 4 years ago

orsinium commented 4 years ago

If the path to config explicitly specified and it has .yaml (or .yml) extension, use YAML format instead of JSON.

Motivation

JSON is human-readable but not human-editable. Beehive WebUI is old and limited. For example, I don't see a way to edit an existing chain through the WebUI. So, let's make the configuration file nice and editable.

muesli commented 4 years ago

There's also a TOML config backend in the works by @penguwin. Theoretically we could support all three formats, I'm just not sure if that's a good idea. Example configs and documentation would then often diverge and confront the user with a different format, and that's probably not helpful. We decided to go with JSON originally because it was user-readable "enough", an established standard and the idea really was to get the web interface to a point where editing the config file was the exception, not the norm.

I'm still a bit torn here. Open for discussion!

rubiojr commented 4 years ago

☝️same! my experience is that while I'd appreciate having yaml over json when I have to manually change the config what I'd really want is powerful GUIs and CLIs that do the necessary validation for me so I don't risk breaking it. I also found that as the config file becomes larger with many bees and chains, the language isn't that relevant anymore, they become really tricky to edit without some visual aids.

orsinium commented 4 years ago

I also like TOML much more (Luigi supports TOML because of me, bandit is almost there) but YAML (unfortunately?) works much better for BeeHive:

  1. All fields are text: Name, Class, Description, ID, Bee, and so on. Only some rare options are numbers, and the validation won't happen even for these cases because beehive accepts interface{} as options value.
  2. TOML is good for flat configs but is not so good for deeply nested arrays:
    1. It's hard to read
    2. It's hard to write
  3. More people are familiar with YAML, at least because of docker-compose. In the DevOps world, everything is configured by YAML.
  4. 341 brings starlark support, also bash scripts can come soon. It requires multiline strings, and the indentation is important. And YAML is better (and more tricky sometimes, yes) in multiline strings, supporting different scalars and ways to fix indentation. However, TOML always preserves all spaces inside of multiline strings.

Let's see an example.

For YAML, it is clear what is related to what, nesting comes naturally:

bees:
  - name: something
    options:
      - name: hello
        value: world
      - name: another
        value: one bite
  - name: something else
    options:
      - name: check
        value: |
          def main(**kwargs):
              return True

For TOML, it is strange non-intuitive [[..]] syntax where options specified as [[bees.options]] but this is not a bees.options field but a field in one of the bees:

[[bees]]
name = "something"

  [[bees.options]]
  name = "hello"
  value = "world"

  [[bees.options]]
  name = "another"
  value = "one bite"

[[bees]]
name = "something else"

  [[bees.options]]
  name = "check"
  value = """
def main(**kwargs):
    return True
"""

So, if you don't want to support both formats at once, I advise you to choose YAML. Sometimes, YAML is tricky and don't strict enough but this shouldn't be an issue with BeeHive, and TOML would introduce more issues.

muesli commented 4 years ago

Ok, some good points there, I'm almost convinced :smile:

We should still only support one designated "config language", so I agree with @penguwin: how about we try to parse the old JSON configs and migrate them to YAML when we first encounter them?

orsinium commented 4 years ago

Thank you for your reviews!

The PR introduces support for both formats at once, without breaking the existing installations. So, it's safe to take it as is. The migration logic can be done later, in a separate PR. Meanwhile, we can try how the new format works without forcing anyone to migrate :)

muesli commented 4 years ago

Fair enough, let's give this a chance and see how it feels.

muesli commented 4 years ago

Thank you @orsinium, loving all the contributions!