@dominicm I should also point out that you could have a separate state object for each client if you want by just attaching it to the environment where all the child parsers will have access to it (but I'm not sure I'd recommend it, just having a unique ID for each client and storing the data for all the clients in a single place is what I'd advocate for)
@drcode I was thinking more in terms of "api client" e.g. a platform, which hosts several services.
@dominicm OK, I can see how the current design could get in the way of maintaining multiple independent services on the server from the same java process... I'll spend some time thinking about that. I guess the question is how many people now rely on dockerization & microservices, which usually leads to a pattern where different services use use their own java processes.
@drcode The trade-offs for microservices have not been suitable for us at all, so we've not gone anywhere near them. The state management in qlkit/clj is very different to how I see state managed in other libraries. Usually I'd expect to be given a map/record/type/object back, and be told "call me with this whenever you need something".
@dominicm Are you also comparing this to state management in OmNext? What qlkit does isn't that different from OmNext.
(just trying to understand the issue better)
@drcode No, not to OmNext. Comparing it to things like database connections, or web servers.
I think what qlkit does is conventional in clojurescript (and I can't decide if that is a good thing!), but unconventional for clojure/jvm.
@dominicm OK, I think maybe the confusion is that parsers on the server are passed a "state" argument- You are completely free to ignore that argument (in fact, it will be nil
if you don't explicitly set the state during mount
) and then what I would do is attach links to the database and stuff in the root component into the environment via (parse-children query-term (assoc env :database DATABASEACCESSOBJECT))
so that all the children can access the database by pulling it from the environment. (and different parsers can hit different databases)
The "state" argument on server parsers exists mostly to have symmetry between client and server parsing
@drcode: No, it's more that these lines happen top level: https://github.com/forward-blockchain/qlkit-todo-demo-raw/blob/master/src/qlkit_todo/server.clj#L13-L14 and therefore they must be having a side-effect somewhere (e.g. updating an atom — https://github.com/forward-blockchain/qlkit/blob/master/src/qlkit/core.cljc#L209). The state in that atom might be not be suited to being a global singleton, e.g. in the case of a multi-tenanted application. It also makes more difficult running parallel tests, as they will mutate the state of that atom. There are tools like eftest which encourage the use of parallel tests to massively speed up test runs, but even for the common case of running tests in the same JVM as your main application it's a useful property.
Agreed, having that global mount can constrain some complex use cases, which we did as a trade-off that makes easy use cases easier. I understand the concern, we may change that approach if more folks run into issues with it.
Keep in mind though that best practice is to use query terms with namespaced keywords so you can have a query term like [foo.bar.todo/delete! {...}]
and so it's not really that restrictive to have a single global read
and mutate
parser, since you can compose a global parser from multiple specific parsers through composition, as keyword name clashes are not an issue.
Yeah, I think that covers multi-tenanting fairly well, unless someone chooses names poorly 🙂. Not as useful for the tests though, a great reason to want to change read/mutate is for testing. We often mock our state out (e.g. in-memory datomic) in order to make testing easier. I could see a need to mock these out to hard-coded values for certain unit tests.
So in our own unit tests we just re-`mount` at the beginning of every test (though that of course will fail if you parallelize your tests)
Ah, makes sense that you're using I completely misread what you said 😂mount
with this. That is more favourable to global singletons. With component
qlkit's approach would be an unnatural fit.
@drcode A pattern I do a lot is to run the tests inside my already running repl, which is already running my dev system. So the tests would end up clobbering the state set by my dev system, which would cause problems I suspect.
The main challenge is that complex parsers involve a lot of recursion, and therefore we had to decide "does the parent pass the parsing function context info to the children, or are those a global singleton?" The first approach is what you want (and essentially what OmNext does, by the way) but the second is easier to read and has less busy work.
I'll definitely think about this- Maybe we can find a "third way" that keeps the best of both approaches
If qlkit never does parsing in a parallel way, then a thread-local dynamic var would work as a medium, where the top-level does:
(binding [*state* passed-param]
(parse …))
I accept that top-level vars are certainly more convenient than passing around parameters in this way though.I entered an issue for this: https://github.com/forward-blockchain/qlkit/issues/4
When I have time to experiment with the best api for this I'll try to support this