Skip to content
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

Processor catalog #19

Open
arichiardi opened this issue Dec 1, 2018 · 6 comments
Open

Processor catalog #19

arichiardi opened this issue Dec 1, 2018 · 6 comments

Comments

@arichiardi
Copy link
Owner

I would like to proposal a quite radical, major version bump to Fonda.

The current layout of steps has two main problems IMHO:

  • It is not obvious to identify where the function is when scanning the step map.
  • Most importantly, we need an index in the vector if we want to replace a step

It seems like we could find a better way to decouple step declaration with the flow. This, of course, have been already thought out by the immensely good folks that developed onyx and I will just post the link to their docs:

http://www.onyxplatform.org/docs/user-guide/latest/#_workflow

The Catalog declares the steps. They get then combined in a workflow.

Now, we do not have the workflow requirement (yet) so I would basically just use just a :steps key - a list of keywords - and a map from keys names to processor or tap maps.
This would make replacing implementations for testing a mere assoc-in away.

@mattbishop
Copy link
Contributor

Interesting, but why would you want to replace a step? I like the rigidity of the steps as they are now.

@arichiardi
Copy link
Owner Author

Oops I did not write it explicitly up there, it mainly would be for testing so that you can swap your "mock" implementation.

At the moment we need a function that does that (scans through all the step vector and replaces one by name). If we move the step declaration to a map we can just use assoc to replace a step for testing.

@ElChache
Copy link
Collaborator

ElChache commented Dec 4, 2018

I support this change, it would make testing much easier as it would be very trivial to replace the side effect steps

@arichiardi
Copy link
Owner Author

arichiardi commented Dec 4, 2018

To summarize this is what I am proposing.

Only 1 parameter for the catalog:

{:get-all-things {:fonda/type :processor
                  :fonda/path [:github-response]
                  :fonda/fn (fn [ctx]
                              {:status 200
                               :headers {"Content-Type" "application/json"}
                               :body {:data :loads-of}})}
 
 :tap-all-things {:fonda/type :tap
                  :fonda/fn (fn [{:keys [github-response]}]
                              (println "the remote thing response was:" remote-thing-response))}}

so that one could do (assoc-in catalog [:get-all-things :fonda/fn] your-other-fn).

Then on the runtime side, a seconda parameter would then combine the steps with something like:

{:steps [:get-all-things :tap-all-things]
 :runtime-ctx {:the :parameters}}

This opens the door for the future - not a thing we need now:

;;;        processor-1
;;;             /\
;;; :processor-3 processor-4
;;;         \      /
;;;          processor-5

{:workflow [[:processor-1 :processor-3]
            [:processor-1 :processor-4]
            [:processor-3 :processor-5]
            [:processor-4 :processor-5]]
 :runtime-ctx {:the :parameters}}

Basically we would be decoupling the declaration from the runtime behavior. It feels like a good change. It might be complicating a bit the UX but not that much actually.


EDIT: the catalog could actually go in the :config because it is static.

To summarize the call:

(execute config runtime-map on-success on-exception on-anomaly)

Where the config is:

{:catalog *the map above with declarations*
 :initial-ctx ...}

@ElChache
Copy link
Collaborator

ElChache commented Dec 6, 2018

I am a little confused in what would go in the config and what would go in the runtime map. runtime-map IMHO would be the map that is passed to the steps. I think I would add the :steps and :catalog in the config

@arichiardi
Copy link
Owner Author

So let's call the input to execute in a different way, param for now.

That is for sure a runtime concern - the :catalog is static so I would say can go in config.

I am not sure about the wiring - :steps - seems like you are right, it feels static.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants