This page is not created by, affiliated with, or supported by Slack Technologies, Inc.
2021-08-11
Channels
- # aws (2)
- # babashka (11)
- # beginners (107)
- # calva (6)
- # cljsrn (18)
- # clojure (180)
- # clojure-australia (6)
- # clojure-europe (54)
- # clojure-losangeles (9)
- # clojure-nl (4)
- # clojure-uk (13)
- # clojureladies (1)
- # clojurescript (57)
- # clojureverse-ops (1)
- # consultantsdirectory (1)
- # cursive (48)
- # datomic (11)
- # defnpodcast (3)
- # degree9 (1)
- # deps-new (5)
- # depstar (21)
- # docker (2)
- # fulcro (15)
- # helix (32)
- # kaocha (1)
- # lsp (21)
- # malli (15)
- # meander (15)
- # news-and-articles (2)
- # nextjournal (1)
- # off-topic (42)
- # pathom (3)
- # podcasts-discuss (1)
- # polylith (73)
- # protojure (1)
- # re-frame (43)
- # reagent (1)
- # releases (1)
- # restql (1)
- # schema (1)
- # sci (1)
- # shadow-cljs (23)
- # spacemacs (7)
- # sql (5)
- # tools-deps (42)
- # vim (15)
- # xtdb (3)
(ns dev.mccue.config.interface
(:import
(com.typesafe.config Config ConfigFactory)
(java.util LinkedHashMap)
( Writer))
(:refer-clojure :exclude [int boolean]))
(set! *warn-on-reflection* true)
;; ----------------------------------------------------------------------------
(defn load-from-classpath
"Loads the default config from the classpath"
[]
(ConfigFactory/invalidateCaches)
(ConfigFactory/load))
;; ----------------------------------------------------------------------------
(defmethod print-method
Config
[^Config x ^Writer writer]
(.write writer "#com.typesafe.config.Config")
(.write writer (let [m (LinkedHashMap.)]
(doseq [[k v] (sort-by key (.entrySet x))]
(.put m k v))
(str m))))
;; ============================================================================
;; Helpers
;; ============================================================================
;; ----------------------------------------------------------------------------
(defn- opt-string
"Helper for optionally extracting a string from the config"
[^Config config key]
(when (.hasPath config key)
(.getString config key)))
;; ----------------------------------------------------------------------------
(defn- int
"Helper for extracting an int from the config"
[^Config config key]
(.getInt config key))
;; ----------------------------------------------------------------------------
(defn- boolean
"Helper for extracting a boolean from the config"
[^Config config key]
(.getBoolean config key))
;; ----------------------------------------------------------------------------
(defn- string
"Helper for extracting a string from the config"
[^Config config key]
(.getString config key))
;; ============================================================================
;; Config Values
;; ============================================================================
;; ----------------------------------------------------------------------------
(defn server-port
"The port on which to listen for http requests."
[config]
(int config "server.port"))
;; ----------------------------------------------------------------------------
(defn db-pool-connections?
"Whether or not to pool connections to the database."
[config]
(boolean config "db.pool_connections"))
;; ----------------------------------------------------------------------------
(defn db-log-queries?
"Whether or not to pool connections to the database."
[config]
(boolean config "db.log_queries"))
;; ----------------------------------------------------------------------------
(defn db-url
"The name of the database to use for primary storage"
[config]
(string config "db.url"))
;; ----------------------------------------------------------------------------
(defn db-username
"The username for the database"
[config]
(opt-string config "db.username"))
;; ----------------------------------------------------------------------------
(defn db-password
"The password for the database"
[config]
(opt-string config "db.password"))
;; ----------------------------------------------------------------------------
(defn redis-url
"The url for connecting to the redis cluster."
[config]
(string config "redis.url"))
;; ----------------------------------------------------------------------------
(defn production?
"Whether the code is running in a \"production\" environment"
[config]
(= (or (opt-string config "env") "production")
"production"))
;; ----------------------------------------------------------------------------
(defn non-production?
"Whether the code is running in a \"non-production\" environment"
[^Config config]
(not (production? config)))
;; ----------------------------------------------------------------------------
(defn photos-s3-bucket
"The s3 bucket to store photos into. nil if undefined."
[config]
(opt-string config "photos_s3_bucket"))
;; ----------------------------------------------------------------------------
(defn support-email
"The s3 bucket to store photos into. nil if undefined."
[config]
(string config "support_email"))
;; ----------------------------------------------------------------------------
(defn jwt-secret
"The secret key to use when signing a JWT"
[config]
(string config "jwt_secret"))
;; ----------------------------------------------------------------------------
(defn stripe-secret
"The secret key to use when signing a JWT"
[config]
(string config "stripe_secret"))
this is an example namespace from my toy project - it has a defmethod which exists as a side effect, a "constructor", and "instance methods"
is the recommendation to move all the impl here into another namespace and (def server-port impl/server-port)
i'm always fine biting repetition/verbosity for a reason, but I don't see the "contract" benefit of the interface being an explicit thing
@emccue Yeah, that's something that seems really "off" about Polylith when you first start -- so much apparent boilerplate and repetition. In a namespace like this, it seems particularly hard to justify since all your functions are one-liners and maybe I'd leave all the public one-liners in place but move the private stuff (and the defmethod
which is purely a side-effecting def) into impl.clj
and reference it like that.
Over time, if any functions become more complex, move their implementation to impl.clj
and delegate to them.
But I have to say, that seems like a very ad hoc set of functions to all have in one namespace and it seems to defy Clojure's data-first principles...
I think I would be inclined to reify the whole config at load time into a Clojure data structure, including spotting "true"
/`"false"` and any parsable integeres.
(since you can get .entrySet
of Config
, that seems easy to do -- does the config auto-reload behind the scenes or otherwise mutate from time to time?)
We've gotten used to the delegation because it provides a nice commonality of approach, and it means that interface.clj
can always be in alphabetical order if you want, regardless of any impl.clj
interdependencies. Also, some of our components are very small, e.g.,
;; copyright (c) 2012-2021 world singles networks llc
(ns ws.file-system.interface
(:require [ws.file-system.impl :as impl]))
(set! *warn-on-reflection* true)
(defn directory-list
"Given a directory and a regex, return a sorted seq of matching filenames."
[dir re]
(impl/directory-list dir re))
and
;; copyright (c) 2012-2021 world singles networks llc
(ns ws.file-system.impl
(:require [ :as io]))
(set! *warn-on-reflection* true)
(defn- wildcard-filter
"Given a regex, return a FilenameFilter that matches."
[re]
(reify java.io.FilenameFilter
(accept [_ dir name] (not (nil? (re-find re name))))))
(defn directory-list
"Given a directory and a regex, return a sorted seq of matching filenames."
[dir re]
(sort (.list (io/file dir) (wildcard-filter re))))
but i wouldn't take this that seriously - its just my toy project to try out libraries and other stuff
(over time I expect that to accrue other functions as we refactor -- but we've being very careful to split code into very specific named components)
We have quite a few components that just have a single function in their interface.clj
Focusing on naming and modularity like that can also lead you to design a narrower interface (which is why I'd be tempted, in your case, not to have a function for every property but to make it more generic somehow, even if it's just reify
ing ILookup
over the underlying Config
object -- which would lead to a nice, narrow interface and everything else hidden away in impl.
There are 2 things I don't really get wrt interface namespaces.
1 - why my.component.interface
and my.component.impl
as opposed to my.component
and my.component.impl
?
2 - if the interface ns has a require .impl
at the top, there is a direct source code dependency from the interface to the implementation, making consumers of the interface transitively but still at compile time depend on the impl. Doesn't that defeat the purpose of an interface somewhat? And if it isn't a goal of these interfaces to serve as means of dependency inversion, I'll double down on my 1st point.
But I've yet to fully read through the documentation so the answer to these could be in there
@U08BJGV6E This is probably the hardest part of Polylith to get used to. The naming is "just" a convention but the consistency pays off in several areas. The swappability of the components comes in assembly as each project within the workspace can declare which components it depends on -- and two different components can implement the same interface, so when a project is assembled, the specific component implementing the shared interface can be selected. There's an example in the docs with user
and user-remote
having the same interface and different implementations.
I advocated for my.component
vs my.component.interface
but there's a lot of existing Polylith code and supporting both formats would be a lot of extra complexity in the tooling and would require a lot of additional testing and documentation. I've talked with @U1G0HH87L several times about this and, now that we're well into our migration journey, we've gotten used to .interface
and it actually helps us keep track of which code is coming from a refactored Polylith component and which is still legacy. Seeing my.component.interface
in a :require
clause is a nice visual clue (and we alias away the .interface
part anyway).
Thanks for that, Sean! Do you by any chance remember where those user/remote components are a little more precisely? I can't seem to find them so far
The consistency comes in always having components/<name>/src/<top-ns>/<name>/
being a folder, with all code inside it -- and the tooling also uses the .interface
on the ns as a "flag" when checking dependencies between various pieces of code, so it can easily tell you if code violates the "interface" by depending on non-interface code instead.
I think the user
/`user-remote` example is in the poly
tool documentation (but it might be in the high-level docs?). Let me find it for you...
In the poly
tool docs: https://github.com/polyfy/polylith#profile
You get used to the .interface
convention pretty quickly 🙂
You have the final state of the example application https://github.com/polyfy/polylith/tree/master/examples/doc-example also.
Hmmm.... now there's another itch I have: se.example.user.core
meaning two different things, with the interface ns duplicated.
I suppose I have to spend some more time coding through the examples to fully understand the benefits
Yeah, you won't really "get it" until you've used it for a while, in my experience.
Example from the file I'm working in right now:
(ns api.handlers.membership
(:require [clojure.tools.logging :as logger]
[ring.util.response :as resp]
[worldsingles.i18n :as i18n]
[worldsingles.libraries.oauth2 :as oauth2]
[worldsingles.membership :as membership]
[worldsingles.money :as money]
[ :as site]
[worldsingles.user :as u]
[ws.billing-sdk.interface :as billing]
[ws.configuration.interface.protocols :as cfg-p]
[ws.web.request :as req]))
spot the two migrated components compared to the legacy code 🙂Since our legacy subprojects tended to be large and a bit amorphous, figuring out where those other namespaces are is non-trivial unless you work with them a lot.
Yeah the migrated ones stand out. But what do you mean by "figuring out where those other namespaces are is non-trivial"?
@U08BJGV6E In our legacy codebase, our subprojects are much coarser-grained than components so there's no obvious clue in the namespace name alone as to which subproject the code is from. Without the discipline/focus of something like Polylith, our codebase has grown somewhat ad hoc over the last decade.
In that snippet shown above, the worldsingles
and ws
nses that are not obviously components
(with .interface
) come from three different subprojects.
A handrolled monorepo of utility libraries where namespace structure is not really well maintained
When we started using Clojure, we thought we'd have a fairly large, flat project so everything was worldsingles.*
. That got hard to manage so we switched new code to ws.<concept>.something
but <concept>
did not necessarily mean subproject -- we were just adding another level to provide grouping/organization. With Polylith, as soon as we see ws.<something>.interface
, we *know* it's in components/<something>
, guaranteed!
What does s & t means?
While migrating legacy code, I have discovered - the hard way - that poly test
won't run tests in files outside the top-level namespace; is this intentional?
Are you saying you have code inside components
or bases
that doesn't follow the src/<brick-name>/<top-ns>/<stuff>/.clj
pattern?
Yeah...
components
|- src
|- top-level
|- brick
|- legacy.namespace
|- test
|- top-level
|- brick.test
|- legacy.namespace.test
And the tests in legacy.namespace.test
is being ignored by poly test
.I'm not very surprised it ignores them -- but it also would never have occurred to me to migrate code/tests in without renaming the namespaces... 🙂
The "worst" part of our migration to Polylith is all the namespace renaming 😐
Of course, now I have a number of tests which I thought were passing, but which were in fact silently failing - or rather, being silently ignored - and probably have been for some time. And I am somewhat upset by this state of affairs.
@U0295UQ75FG Out of curiosity, does clojure -M:poly check
(or poly check
) complain about those legacy tests being there?
(I added a "bogus" test to a component and ran check
and it complained about it)
Hi. I guess your legacy code is imported as :local/root
@U0295UQ75FG? Yes, the test runner only runs migrated tests (living in bricks). We could add support for running tests in code imported with :local/root
if that’s the best way to go. Then I think we should also force people to specify which :local/root
tests we should run. Some of them could be “plain libraries” also, and we should not run tests for these (that’s how I think tools.deps https://github.com/cognitect-labs/test-runner treats all :local/root
imports, by never including their tests). We could show a warning if any of them are not specified. It’s never good if people think a tool behaves in one way, but turn out to behave in another way, so you have a valid point here, and it could also improve the migration experience. I don’t know if some people would start abusing this by adding code this way instead of putting it in bricks, in greenfield workspaces, but I hope not.
I don't think poly
can be expected to run non-brick tests that are elsewhere in the workspace -- there's no way to know how the deps are built for those tests, nor how to actually run those tests.
One thing I do miss right now with poly test
is how to run some setup per test run (not per namespace nor per test). For Cognitect's test-runner
we have our own wrapper that handles per-test-run fixtures -- it would be nice to have a hook in poly test
per-project to have an extra fixture.
@U1G0HH87L Matt showed his component structure above with some legacy tests inside the component -- that just don't match the top-ns.brick.*
naming convention: it's not imported test code.
I added https://github.com/polyfy/polylith/issues/109 issue, which I closed.
@U1G0HH87L It's not about :local/root
imported code.
Yes, I realised that too late!
We have a warning (as you also wrote) that is triggered if you put code that does not live under the top namespace within a brick, and I think that should be enough.
I don't see the warning with poly test
tho', only with poly check
Here's a repro:
seanc@Sean-win-11-laptop:/Developer/workspace/wsmain/clojure$ clojure -M:poly check
Warning 205: Non top namespace legacy was found in preview-web.
seanc@Sean-win-11-laptop:/Developer/workspace/wsmain/clojure$ clojure -M:poly test project:preview
Projects to run tests from: preview
Running tests from the preview project, including 2 bricks: seo-json-renderer, preview-web
2021-08-11 21:40:15.748:INFO::main: Logging initialized @16102ms to org.eclipse.jetty.util.log.StdErrLog
Testing ws.preview-web.main-test
Ran 0 tests containing 0 assertions.
0 failures, 0 errors.
Test results: 0 passes, 0 failures, 0 errors.
Testing ws.seo-json-renderer.interface-test
Ran 2 tests containing 6 assertions.
0 failures, 0 errors.
Test results: 6 passes, 0 failures, 0 errors.
Testing ws.seo-json-renderer.impl-test
Ran 0 tests containing 0 assertions.
0 failures, 0 errors.
Test results: 0 passes, 0 failures, 0 errors.
Execution time: 3 seconds
seanc@Sean-win-11-laptop:/Developer/workspace/wsmain/clojure$ cat bases/preview-web/test/legacy/old_test.clj
(ns legacy.old-test
(:require [clojure.test :refer [deftest is]]))
(deftest bad-test
(is (= 1 2) "Broken!"))
poly check
correctly warns that legacy
is non-conforming but poly test
just quietly ignores it. So a failing test "doesn't fail" and you would only know if you ran check
.
Today the test
command will refuse to execute if there are any errors, but it doesn’t show warnings. One idea could be to show a warning at the end of the test run, that these tests have been ignored.
Or always show all warnings, that would be even simpler.
Ah, OK. Yeah, that would be a good idea I think (depending on what other warnings it would otherwise suppress 🙂 )
I will include that the next time I have something to push.
A bit late but...
poly check
was warning about the namespace problem... but it wasn't an error so I didn't think it was a huge problem. Whoops, I guess.
And I've been working on updating the namespaces, but my codebase uses a lot of namespaced keys and I'm wary of moving them.
No problem. The warning can maybe be explicit about that the tests are ignored + to show warnings at the end of the test commands (if any). I can put that on my todo list.
@U0295UQ75FG I know this probably doesn't help (and you might not enjoy hearing it), but this is why it's better to have qualified keys that map to the business domain, instead of implementation details (actual code namespaces). ::
style keywords should really be private to the code that uses them, except for keywords that you expand via aliases (and when you rename namespaces, you would have to update the :require
/`:as` anyway so ::
uses will continue to work unchanged).
We use qualified keys a lot, but most of ours don't match actual code namespaces -- except for Specs which are typically private to a namespace (or accessed solely via ::alias/key
names elsewhere).
My code uses Integrant, and Integrant depends on multimethods with ns-qualified keywords, like:
(defmethod integrant.core/init-key ::something [_ config] (...))
If the key isn't ::
style then Integrant has trouble finding the correct multimethod. And then that key has to go in .edn
files to get configured.
And then I guess somebody eventually writes something like
(when (:legacy.namespace.file/something some-map) (...))
And then I move legacy.namespace.file
and a test breaks.makes notes... adds this to the list of reasons why he wouldn't use Integrant... Sorry @U0295UQ75FG that does sound pretty painful 😞
afaik integrant's keyword->namespace finding algo is extensible so it might be a matter of tweaking that
(similarly for test/<brick-name>/...
)
One thing I do miss right now with poly test
is how to run some setup per test run (not per namespace nor per test). For Cognitect's test-runner
we have our own wrapper that handles per-test-run fixtures -- it would be nice to have a hook in poly test
per-project to have an extra fixture.