# Created 2020-11-09 Mon 16:07
#+TITLE: Work Time Tracker
#+AUTHOR: Yann Esposito
* IN-PROGRESS Write issue about config.edn simplification            :work:
[2020-11-09 Mon 15:24]


** Problem

we have a lot of duplication between our =config.edn= files in
=tenzin-config=.
This make it easy to be wrong while we copy (5 times) similar values.
We would like to make those configuration file a lot shorter and easier to
manage by only puting into them the field that are really different between
all environment.

** A little bit of short-time history.

At first we relied on a single template that used different =.json=
entries.
Which is kind of duplicating the =config.edn= by adding a layer of logic in
the middle.
After lot of confusions and bugs, we decided (both devs and ops) to duplicate
the config.edn and minimize the amount of templating in the process.

** Abstract of previous discussions

If you look at the problem, there are plenty of different solutions to
handle that.
Here are what we thought about:

1. Use a better templating system than jinja. The best in class in my
   opinion (and by far) is dhall.
   The issue with dhall is that it is still put a limitation about how we
   generate the =config.edn= and this is also another new language to
   learn.
2. Use a Clojure project to handle =config.edn= templating/generation.
   Mainly re-write a dhall-like project in Clojure better suited for our need.
3. Use another =ConfigService= that would take care of some logic in Clojure.
4. Make every service to use better default values.
   So typically some service will depend on some `tk-store` that are named
   by something and generally the service is also responsible to know the
   intended store to be used. Typically the service know that it should
   rely on postgres and not redis nor RAM-only store.
   The issue is the service will need to declare its expected configuration
   to other services (typically to tk-stores). So we should move from
   =init= to =start=. Every of our service should declare its default
   config to a centralized =IROHConfigService= during =init= phase.
   Then every service should initialize its context during the
   =start= phase during which we should use a function similar to
   =get-in-config= but from =IROHConfigService= and not TK =ConfigService=.

From an architecture standpoint the conclusion was to prefer the 4th choice
(which would not be incompatible with the 3rd.)
Mainly the service start to take responsibility from some dependencies.
Also the =IROHConfigService= could take care of potential configuration
conflict.
If two services want different value for the same field we should make the
configuration fail (unless the configuration is overwritten in =config.edn=)

** High level technical spec

So technically most (all?) our services have a =init-context= function.
Generally this function use =get-in-config= from TK =ConfigService=.
Also the =init-context= is called in the =init= phase.

We should instead make all our service dependant on the new =IROHConfigService=.
And during the =init= phase the service should send its default
configuration to the =IROHConfigService=.

Example:

#+begin_src clojure
(ns iroh.my-service
   (:require [iroh.my-service.core :as core],,,))
(defprotocol MyServiceProt ,,,)
(tk/defservice
   [[:IROHConfigService declare-default-conf get-in-conf]]
   (init [this context]
      (core/init-conf declare-default-conf))
   (start [this context]
    (into context
     (core/start-context get-in-conf ,,,)))
   ,,,)

(ns iroh.my-service.core ,,,)

(def default-config
  {:stores {"foo" {:type :postgres :conf {:table-name "foo"}}
            "my-service-cache" {:type :redis :conf {:db 1}}}
   :my-service {:default-timeout 3000}})

(defn init-conf
  [declare-default-conf]
  (declare-default-conf default-config))

(defn get-in-conf
  [set-default-config
   get-in-config path]
  (or (get-in-config path) (get-in default-config path)))

(defn ^:always-validate start-context :- MyServiceContext
  [get-in-conf ,,,]
  (let [foo (get-in-conf [:my-service :default-timeout])]
       ))
#+end_src

So the =IROHConfigService= should take care of doing a =deep-merge-with
concat= on all default configurations and the config from =ConfigService=.
It should also throw an exception in case of configuration conflict.
If two services do not agree on the value of some inner field.

Doing so, we should better separate the developer concerns from the ops
concerns.
Currently the ops are responsible to select the type of database, the
default routes.
While this should be configurable and at the same time not the matter of
the ops.

** Last word

This Epic is just here to keep track of the discussions about that
recurring subject and make a proposal of solution.
The objective would be to reach a consensus on a the best way to handle
config.edn simplification and prevent duplication.