gjcarneiro / yacron

A modern Cron replacement that is Docker-friendly
MIT License
449 stars 38 forks source link

Maybe switch to strictyaml? #4

Closed gjcarneiro closed 6 years ago

gjcarneiro commented 6 years ago

https://github.com/crdoconnor/strictyaml

According to the author, it gives you much better error messages when validation fails.

gjcarneiro commented 6 years ago

I created the stricyaml branch with work on this, but I couldn't get it to work in the end. I am not impressed. I think I'll stick to normal yaml and jsonschema.

crdoconnor commented 6 years ago

Thanks for trying. I'll take a look a bit later this evening and see if I can figure out what went wrong and issue a pull request.

From a cursory glance it looks pretty much exactly as I'd expect.

crdoconnor commented 6 years ago

You simply missed out the Seq. Instead of::

jobs:
  - name: test-01
    command: echo "foobar"  # runs with /bin/bash as shell
    schedule: "*/5 * * * *"
  - name: test-02  # runs with /bin/sh as shell
    command: echo "zbr"
    schedule: "*/5 * * * *"

It was expecting::

jobs:
  name: test-01
  command: echo "foobar"  # runs with /bin/bash as shell
  schedule: "*/5 * * * *"
gjcarneiro commented 6 years ago

Oh, wow, thanks for debugging! I was just confused I guess. I'll play around with it some more, then.

gjcarneiro commented 6 years ago

Btw, I would like instead of

strictyaml.exceptions.YAMLValidationError: while parsing a mapping
unexpected key not in schema 'killTimeoutx'
  in "<unicode string>", line 73, column 1:
      killTimeoutx: '0.5'
    ^ (line: 73)

to display the correct file name instead of <unicode string>. If you have any idea, would appreciate the tip. I'm doing this hack, but it feels dirty:

    try:
        doc = strictyaml.load(data, CONFIG_SCHEMA).data
    except YAMLValidationError as ex:
        if ex.context_mark is not None:
            ex.context_mark.name = path
        if ex.problem_mark is not None:
            ex.problem_mark.name = path
        raise
crdoconnor commented 6 years ago

Fair point. I'm mulling over doing this::

try:
    doc = strictyaml.load(data, CONFIG_SCHEMA, label=path).data
except YAMLValidationError as ex:
    print(ex)

strictyaml.exceptions.YAMLValidationError: while parsing a mapping
unexpected key not in schema 'killTimeoutx'
  in "/path/to/whatever.yaml", line 73, column 1:
      killTimeoutx: '0.5'
    ^ (line: 73)
gjcarneiro commented 6 years ago

That could work, though perhaps it would be simpler to just give it a file name and let strictyaml open the file.

Are you designing this as a new API for strictyaml? Should we discuss in a new strictyaml issue, instead of this (closed) issue?

crdoconnor commented 6 years ago

Raised: https://github.com/crdoconnor/strictyaml/issues/26

Re: opening the file - no, I don't really want to do that. It opens up a whole new can of worms (encoding errors, various file system related exceptions, etc.) that I don't really want to become strictyaml's problem.