Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Yaml Validation Feature #397

Merged
merged 5 commits into from
May 16, 2021
Merged

Add Yaml Validation Feature #397

merged 5 commits into from
May 16, 2021

Conversation

krkeegan
Copy link
Collaborator

@krkeegan krkeegan commented May 11, 2021

Breaking change

  1. Cerberus is now a required package. If you are running from raw source code without installing, you will need to run pip3 install -r requirements.txt again.
  2. There is a potential (rather unlikely), that your current yaml configuration file has an error you have not realized. If that is the case, InsteonMQTT will fail to start after upgrading. It will output a clear message describing the error in your yaml file that needs to be fixed.

Proposed change

Adds code to validate the configuration yaml against an included schema on startup. It will also validate the the scenes yaml against an included schema if scenes are enabled.

If validation fails it will print a nice clear message to the user and exit the program.

While a harsh result, it will prevent configuration errors may become latent errors that are hard to diagnose.

The validation is moderately permissive. That is, I have allowed unknown keys to be present in a few places where they may exist in old configuration files.

I believe, for 99.9% of users, if your installation is working currently. The validation will pass. However, there may be some odd situations that are being tolerated by the code that may cause a validation error.

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass.
  • Tests have been added to verify that the new code works. (I am going to add some example bad yaml files that contain what I suspect are common mistakes).
  • Code documentation was added where necessary
  • User Documentation added/updated (Still need to add some light user documentation)

This adds Cerberus as a required package.

Cerberus is used to validate the configuration yaml against an
included schema.  It will also validate the the scenes yaml
against an included schema if scenes are enabled.

The validation is done first thing on startup, and if it fails
it will print a nice clear message to the user and exit the
program.

I realize that exiting is a harsh result, but I strongly
believe that it is better for users this way.  Otherwise
configuration errors may become latent errors that are hard
to diagnose.

The validation is moderately permissive.  That is, I have
allowed unkown keys to be present in a few places.  Notably, I
did this in the devices sections of the mqtt portion.  This is
because in the past we had named a few of the topic keys
differently or allowed their use in places where they are no
longer used.

I believe, for 99.9% of users, if your installation is working
currently.  The validation will pass.  However, there may be some
odd situations that are being tolerated by the code that may
cause a validation error.
@krkeegan
Copy link
Collaborator Author

I should also note that the schema was successfully run against the config-example.yaml and scenes.yaml files as well. So our recommended configuration formats are valid. When a configuration setting has multiple formats, we generally have all examples of these formats in these files, so this is a good test that the schema is tolerating all permitted variations.

Some basic tests to check validation.  I am not going to test
all possible bad configurations, just the basic concept

Also check config-example.yaml so that it is always correct.

Test the address validation and fix error in regex

Improve validation messages
krkeegan added 3 commits May 16, 2021 10:16
Make the Schema less strict. Less ideal as typos will no pass
unnoticed, but it won't halt on startup for typos either.
@krkeegan krkeegan merged commit 6d6c4a5 into TD22057:dev May 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant