tomekw/hikari-cp

Conforming configuration options (types coercion)?

pilosus opened this issue · 5 comments

pilosus commented

For now configuration options are validated with s/valid?.

The problem is that quite often DB connection options are coming from the environment variables that are always strings. That breaks validation as many hikari-cp option specs are expecting integers. Basically, that requires extra work in the app: first you coerce types from the envs, and only then pass them to hikari-cp specs.

Why not using s/conform to coerce types in hikari-cp's specs in the first place?

(require '[clojure.spec.alpha :as s])

(defmacro with-conformer
  [[bind] & body]
  `(s/conformer
    (fn [~bind]
      (try
        ~@body
        (catch Exception e#
          ::s/invalid)))))

(def ->int
  (with-conformer [val]
    (Integer/parseInt val)))

(s/def ::->int ->int)

(s/conform ::->int "5432") ;; => 5432

Once data is coerced, all necessary validations can be applied just like they are now (e.g. (s/and integer? gte-1000?)).

tomekw commented

Interesting idea @pilosus! Would you be able to sketch a PR for this?

EDIT: I didn't reach out earlier as I was battling with COVID.

pilosus commented

I probably would! If there're no hard deadlines for volunteers (as I only can work on the weekends), then feel free to assign the issue to me.

tomekw commented

There is no deadlines in OSS ;)

Thanks!

  • 🚀1
pilosus commented

@tomekw sorry for the late response.

Actually, I've started working on the issue, but then decided to double check my tech design with a couple of peers first.
My reviewers say that since hikari-cp is simply a wrapper over another library and it doesn't have its own configuration management (loading config from envs, files, cli), you don't have to introduce types coercion as it will break single responsibility principle. That means an application using hikari-cp must be responsible for config management and type coercion, so that hikari itself only validates the data coming from the config managements layer.

Also, I am still new to Clojure and I am not sure why specs don't do both coercion and validation. This is very different from what pydantic or marshmallow from my "native" Python do. But I guess introducing a new dependency like Exoscale's coax is a bit of overkill for hikari-cp.

All in all, my reviewers convinced me I am wrong wanting to add type coercion to hikari-cp :-) Sorry for opening the issue.
If you still think the original idea was ok, I can try to draft a PR. If not, let's close the issue.