samrose / ex_integrate

A continuous integration library implemented in Elixir.
Apache License 2.0
4 stars 0 forks source link

Add validation and fault tolerance to user config parsing #38

Open garrettmichaelgeorge opened 2 years ago

garrettmichaelgeorge commented 2 years ago

Giving some attention to user input, we should address validation and fault tolerance for the user config file.

Validation

We need to validate the user input in the CI config file (i.e. ei.json).

We seem to have two options for this – Ecto, or rolling our own. Since we are considering using Ecto anyway for persistence, I would personally prefer to lean on it for all user input validation – why take the time to build and test a validation library when one exists already?

But of course different teams take different approaches with this. Tate and Gray roll their own validation in Designing Elixir Systems and encourage it freely.

Fault tolerance

Right now, the config file is parsed by ConfigParser using bang functions that raise on error:

https://github.com/samrose/ex_integrate/blob/df58603449a5a443c7371cc4c1c2340154534a1e/lib/ex_integrate/config_parser.ex#L1-L7

There are two main possible points of failure here:

  1. Reading the file
  2. Decoding the JSON

(This is of course also coupled to the JSON format, which I would love to change over time).

Right now, if one of the above points fail, then it crashes the system. In order to build in fault tolerance here, we should probably isolate ConfigParser into a separate process.

garrettmichaelgeorge commented 2 years ago

@samrose what do you think?