-
Notifications
You must be signed in to change notification settings - Fork 51
Add connection object as metadata to query-fn #136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Yeah that looks handy, let's add some docs and can merge it in. :) |
|
I noticed this change is particularly handy since if I try to import the |
|
I tend to reference the system in the (defn api-ctx []
{:query-fn (:db.sql/query-fn state/system)
:http-client (:http/hato state/system)})Then I can call the handler with In terms of app code structure though, the system should be the entry point since Integrant is basically a DI framework. If you need to reach into the system from the scope of a component that probably means you should just be passing the components it needs explicitly via a reference in the system. |
|
@yogthos OK so that was my conclusion and the point of this PR. The current transaction example in docs does reference the system atom explicitly so not practical for a real app then. |
|
Yeah, for this sort of use case metadata is not the "proper" way to do it since metadata shouldn't really be used bypass Integrant. I think we should just update the docs to explain the connection should be passed in for transactions. |
|
@yogthos I agree with you in principle. I think part of my problem is that I have structured my application more like a N-Layered Architecture with entities interacting directly with the database, and not a more "Clean" style with DB I/O is performed at the edge and data pulled out at the same level as the the routing handlers. A more sizeable "real-world" Kit/Clj example would be beneficial I think. The guestbook and musicbrainz examples are just toy-size. Any plan to update your "Web Development with Clojure" book to make use of Kit? I have 2nd and 3rd edition of it and have found them very useful, and it does sport a somewhat more extensive application I think. |
|
I find clean approach is a good default, but in a lot of cases it can become awkward. I find that it's a good approach at small scale, but larger apps end up needing to load data progressively because you either don't know what's all that you need up front, or it's not efficient to fetch all the data eagerly. I settled on reasoning about large apps from the perspective of state machines. There is a high level flow for each workflow in the application, and this flow consists of a number of states that the workflow can end up in. Handling each state can be done in a pure way where you do all the IO at the edges, do some computation, and then produce a new state. The output can then be expected and the next handler can be dispatched based on the state of the data. This is a really good blog post on the subject https://shopify.engineering/17488160-why-developers-should-be-force-fed-state-machines I wrote a bit about how you can use multimethods to manage high level flow here https://yogthos.net/posts/2022-12-18-StructuringClojureApplications.html I also gave a talk where I fleshed out this a idea a bit more https://www.youtube.com/watch?v=y2zvQDpgMak I mention this library I wrote in the talk, and it's based on how I structured a larger app which ended up working out pretty well for me https://github.com/yogthos/maestro/ The library expresses the state machine as a graph, and then each node in the graph can be treated as a tiny clean architecture style app, with the graph managing how the data flows between them. |
Since we already added some metadata to the query-fn I have found it useful to also have the db connection there as well, in particular when working with transactions (such as when using
next.jdbc/with-transaction).Prior to this change I need to pass the connection component ref to each component that uses transactions in addition to the (already somewhat onerous) query-fn ref. Passing one db-related object should be enough I think, and that the connection fits quite well in the query-fn metadata.
If you think this looks good I can also add some docs mentioning this and how to access it.