Flows #25

wkok · 2023-11-22T13:51:12Z

Closes #23

Adding support for Flows

New sample app samples/flow

valerauko · 2023-11-22T14:25:31Z

src/hti/re_dash.cljd

+                (reduce (fn [new-db {:keys [inputs output path]}]
+                          (let [new-db-resolved-inputs (resolve-inputs new-db inputs)
+                                old-db-resolved-inputs (resolve-inputs @app-db inputs)]
+                            (if (not= new-db-resolved-inputs old-db-resolved-inputs)
+                              (assoc-in new-db path (output new-db-resolved-inputs))
+                              new-db)))


looks good to me!

liveness check would run here? (cleanups...?)

https://day8.github.io/re-frame/Flows/#living-and-dying

I think so yes. Still need to consider all the supporting functionality around the Flow lifecycle, while keeping in mind things are still settling in re-frame land around this feature.

valerauko · 2023-11-25T02:01:59Z

src/hti/re_dash.cljd

+  ([flow-id {:keys [inputs output path] :as flow}]
+   (when-not flow-id (throw (ex-info "flow-id is mandatory" flow)))
+
+   (let [flow' (assoc flow :id flow-id)]


Looking at this it occurred to me if it might be worth using records for these object shapes?

(defrecord Flow [id inputs output path]) (and possibly similar with interceptors etc)

In JVM Clojure field access on records is much faster than map lookups, though I don't know if that applies in cljd

It would be interesting to do some performance bench-marking at some point, and compare between using maps vs records.

My view is that it would need to be a significant, measurable, performance gain before we swap the simplicity of maps for records

I don't feel records would make anything that much more complicated though.

In Clojure they're just glorified maps anyway

valerauko · 2023-11-27T02:17:44Z

src/hti/re_dash.cljd

+  (or (flow-id definitions)
+      (throw (Exception. (str "Flow not found: " flow-id)))))
+
+(defn- derived-input-path


maybe worth splitting namespaces at this point? it't getting pretty crowded

valerauko · 2023-11-27T02:18:25Z

src/hti/re_dash.cljd

+  {::flow<- flow-id})
+
+(defn- sort-flow-ids
+  "Sort all Flow definition dependency graphs


all? do we have to worry about cyclic dependency between flows that aren't live?

Yes I think we do, as flows may come alive at any point during flow processing given their live? test passes.

I've started with the flow life-cycle hooks so it will get clearer as we progress.

valerauko · 2023-11-30T01:36:45Z

src/hti/re_dash.cljd

-      (throw (Exception. (str "Subscription not found: " subscription-id))))))
+  (if (flow? query-vec)
+
+    (flows/subscribe query-vec)


What does it mean to subscribe to a flow?

My understanding was that flows are app-db -> app-db processes. They respond to changes in their inputs and generate some output into the app-db in turn. The user would subscribe to the output field in the app-db (or whatever else they use instead of the app-db)

Especially if a flow can have multiple outputs, how would that work with direct subscribe?

From the docs, a Flow can only have one output path.

I suspect the intention is merely to provide a more declarative way to subscribe to the Flow's output path by referencing it by name (:id)

It's also kind of nice that the Flow's logic can therefore be updated later without affecting the UI that subscribes to it necessarily.

valerauko · 2023-11-30T01:46:58Z

src/hti/flows.cljd

+(defn subscribe
+  [query-vec]
+  (let [path (path query-vec)]
+    (f/$ (get-in (f/<! app-db/app-db) path))))


I don't think it's a good idea to hard-code the app-db here.

Subscriptions can use anything subscribable if you give them an according input-signal when building them with reg-sub. I think a similar pattern would be desirable here, so that it would be possible to use alternative backends for the app-db (or multiple app-dbs altogether).

Imagine something like this: using one (atom {}) (let's call it data-db) to store user data and using the app-db to store UI state in parallel. This is already possible with subscription input signals. With flows it would be possible to automatically update the app-db based on changes to the data-db (from the result of an API call or something). That'd be awesome.

On the other hand, using input functions instead of "just paths" would make the current dependency calculation pretty much impossible... Maybe if there was a protocol for app-db-like things?

The philosophy of re-dash is to take lead from re-frame as far as reasonably possible in my opinion.

Anything outside of that, I'd argue should be plugable at some extension point, like using interceptors for example.

These could maybe be published as supplementary libraries like re-dash-my-custom-db or something. Just thinking out loud.

Yes that's exactly what I meant.

By hardcoding app-db here subscribing to Flows becomes wired to the app-db.

It'd be nice if (while the app-db was the obvious and simple default) it'd be possible to provide a different input-signal (in reg-sub terms) at reg-flow time

valerauko · 2023-11-30T01:51:28Z

src/hti/re_dash.cljd

-                :else             (f/<! signals)))
-            query-vec))
-      (throw (Exception. (str "Subscription not found: " subscription-id))))))
+  (if (flow? query-vec)


If subscriptions and flows were records subscribe could be turned into a protocol on top of that (which could also protect against the in my experience all-too-common silly mistake of (subscribe ::id))

I'd be happy to help with implementing that if you want a hand

You're welcome to put a proof of concept together, thank you for offering.

As I'm still struggling a bit to see the benefit records will bring, my two biggest concerns with whichever end result we end up with, would be:

that it significantly, and measurably improve event/subscription performance over the current implementation

that it does not add complexity the code

Also just be aware that things are still changing as I'm iterating through the Flow implementation

valerauko · 2023-12-04T09:29:03Z

samples/flow/src/acme/model.cljd

+      {:clear-flow (:id area-flow)}))
+
+  ;; Initialize db
+  (rd/reg-event-db ::init   (fn [db _]        (-> db


Is the indentation broken here?

valerauko · 2023-12-04T09:31:22Z

src/alandipert/kahn.cljd

+;; the terms of this license.
+;; You must not remove this notice, or any other, from this software.
+
+(ns alandipert.kahn


Should there be a link to the original gist? Not sure

I'll add the link to the gist anyway

valerauko · 2023-12-07T04:06:55Z

doc/03-flows.md

@@ -0,0 +1,41 @@
+# Flows
+
+Status: (alpha)


What does that mean?

re-frame released Flows under alpha, likely for reserving the right to either change the API, or how it works, or bin the whole idea if it doesn't work out.

re-dash would track this

OK so this means "API might change, there be dragons"

Some places call "untested experimental code" alpha, that's why I asked

valerauko · 2023-12-07T04:10:11Z

src/hti/re_dash.cljd

I think this doesn't have to move.

It's a huge breaking change (literally obliterates the whole existing public API) for the sake of folder structure...

The idea was not to break but to deprecate.
But I see your point, it is rather harsh for the sake of folder structure. I'll revert this namespace

imo, either keep it at hti.re-dash, or move it to hti.re-dash.core and add a "new" hti.re-dash that re-exports the (at least current) public API from the new hti.re-dash.core

That's precisely what the hti.re-dash change in my commit proposed - it exports everything from core but with a deprecation notice on the namespace.

Would removing the deprecation notice then be a more acceptable middle ground?

Am I missing something? I can't seem to find the diff you're talking about. The hti.re-dash namespace is just gone

OH never mind. It was just Github being stupid (it showed hti.re-dash as all deleted and hti.re-dash.core as all created)

valerauko · 2023-12-07T09:03:44Z

src/hti/re_dash.cljd

+  {:deprecated "0.9.0"
+   :superseded-by "hti.re-dash.core"}
+  (:require [hti.re-dash.core :as core]))


Looks good to me

wkok added 2 commits November 22, 2023 13:35

Initial, basic Flows implementation

3229aea

Updated label

1eb69e2

valerauko reviewed Nov 22, 2023

View reviewed changes

wkok added 2 commits November 22, 2023 18:18

Improved current Flow tests to span entire data loop

cebed17

Added flow execution on registration and de-duped registrations

c93454c

valerauko reviewed Nov 25, 2023

View reviewed changes

Added support for layered / dependent flows using dependency graph

a487870

valerauko reviewed Nov 27, 2023

View reviewed changes

Added ability so subscribe to a Flow

aa23df8

valerauko reviewed Nov 30, 2023

View reviewed changes

Added Flow lifecycle support and updated samples/flow to demo

0ab718a

wkok marked this pull request as ready for review December 3, 2023 15:04

wkok changed the title ~~Initial proof of concept - Flows~~ Flows Dec 3, 2023

valerauko reviewed Dec 4, 2023

View reviewed changes

wkok added 5 commits December 6, 2023 11:43

Fixed indentation and added link to kahn gist

eac93dd

Added core, alpha namespaces, moved Flows to alpha, added deprecations

d9d04e6

Fixed tests

7fba3be

Added docs

20db949

Updated docs

2f5fb33

valerauko reviewed Dec 7, 2023

View reviewed changes

valerauko approved these changes Dec 7, 2023

View reviewed changes

wkok merged commit c1664af into main Dec 7, 2023
1 check passed

wkok deleted the flows branch December 7, 2023 09:42

valerauko mentioned this pull request Dec 13, 2023

Experiment with using records #29

Open

wkok mentioned this pull request Dec 19, 2023

Database Protocol #30

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flows #25

Flows #25

wkok commented Nov 22, 2023 •

edited

valerauko Nov 22, 2023

wkok Nov 22, 2023

valerauko Nov 25, 2023

wkok Nov 26, 2023

valerauko Nov 27, 2023

valerauko Nov 27, 2023

valerauko Nov 27, 2023

wkok Nov 30, 2023

valerauko Nov 30, 2023 •

edited

wkok Nov 30, 2023

valerauko Nov 30, 2023

wkok Nov 30, 2023

valerauko Nov 30, 2023 •

edited

valerauko Nov 30, 2023

wkok Nov 30, 2023

valerauko Dec 4, 2023

wkok Dec 6, 2023

valerauko Dec 4, 2023

wkok Dec 6, 2023

valerauko Dec 7, 2023

wkok Dec 7, 2023

valerauko Dec 7, 2023

valerauko Dec 7, 2023

wkok Dec 7, 2023

valerauko Dec 7, 2023

wkok Dec 7, 2023

valerauko Dec 7, 2023

valerauko Dec 7, 2023

valerauko Dec 7, 2023

Flows #25

Flows #25

Conversation

wkok commented Nov 22, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valerauko Nov 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

valerauko Nov 30, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wkok commented Nov 22, 2023 •

edited

valerauko Nov 30, 2023 •

edited

valerauko Nov 30, 2023 •

edited