hikari-cp icon indicating copy to clipboard operation
hikari-cp copied to clipboard

Conforming configuration options (types coercion)?

Open pilosus opened this issue 2 years ago • 5 comments

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?)).

pilosus avatar Jan 09 '22 18:01 pilosus

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.

tomekw avatar Jan 25 '22 11:01 tomekw

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.

pilosus avatar Jan 25 '22 13:01 pilosus

There is no deadlines in OSS ;)

Thanks!

tomekw avatar Jan 25 '22 13:01 tomekw

@pilosus like this? :)

https://github.com/exoscale/coax

tomekw avatar Apr 07 '22 11:04 tomekw

@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.

pilosus avatar Apr 16 '22 14:04 pilosus