Skip to content

WIP - Add previous result in step functions, mocked functions#43

Open
arichiardi wants to merge 9 commits into
masterfrom
elchache-new-features
Open

WIP - Add previous result in step functions, mocked functions#43
arichiardi wants to merge 9 commits into
masterfrom
elchache-new-features

Conversation

@arichiardi

@arichiardi arichiardi commented Apr 7, 2020

Copy link
Copy Markdown
Owner

In a nutshell, the main new features are:

  • Callbacks on each step: on-success, on-error, on-start, on-complete for status reporting.
  • Bug fix - inject couldn't return nil.
  • We might go for multimethods like this article instead of:

    Wrapper function for the callbacks - that way the callbacks value can be data that will be passed to the wrapper, very handy for re-frame as the callbacks can be events and the wrapper function dispatches the event

  • Steps can access the previous step result as the second argument of the function. Also the on-success function receives the last step result as as second parameter. This allows to have a last step that returns a clean output instead of using the whole context as an output, or an arbitrary key in the context. David you might want to revert this?
  • :is-anomaly-error? attribute in the steps. The value is a function that would be called if the step returns an anomaly. If this function returns false, then the flow continues. This allows to break the circuit only with some anomalies.

@arichiardi arichiardi force-pushed the elchache-new-features branch 3 times, most recently from 988d5bf to fc859cc Compare April 12, 2020 01:15
Comment thread src/fonda/execute.cljs Outdated
Comment thread src/fonda/execute.cljs Outdated
Comment thread src/fonda/execute.cljs Outdated
inject (inject ctx))
assoc-result-fn (cond
(let [
;; First step only gets the ctx, next ones receive last-result,ctx

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this comment still apply?

Comment thread src/fonda/execute.cljs
Comment thread README.md

;; on-exception
(fn [exception]
(fn [ctx exception]

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going through all the breaking changes, this seems one.

Do we want to keep it? Is the context useful in case of exceptions? I remember back in the days we never needed it that's why it was not included in the signature.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this signature more than anything because is consistent with all the callback functions signatures - the steps callbacks, and also the on-success callback, all have the same signature.
I also found very useful to be able to print the context when an exception happens.
And I can see it used when trying to recover from an error, maybe to repeat the flow from a certain step

Comment thread README.md
;; on-success
(fn [ctx]
(handle-success (:github-things ctx))))
(fn [ctx last-step-res]

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another but we discussed it already and it seemed useful.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes. Not breaking in Clojurescript, but would be breaking in Clojure

Comment thread README.md
- [Optional] **on-anomaly** Function called in case of anomaly with the anomaly data itself.
- **on-exception** Function with the signature `(fn [ctx exception])` called with the context and an exception when any of the steps throws one.
- **on-success** Function with the signature `(fn [ctx last-step-result])` called with the context if all steps succeed, and the last step result.
- [Optional] **on-anomaly** Function with the signature `(fn [ctx anomaly])` called in case of anomaly with the context and the anomaly data itself.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I see we added the context also to this one. Sorry don't want to be a pain but just want to understand if really necessary 😄

@arichiardi arichiardi force-pushed the elchache-new-features branch from cc9d4a8 to d0c6e41 Compare April 13, 2020 02:33
Comment thread examples/fonda/simple.cljs
@arichiardi arichiardi force-pushed the elchache-new-features branch from 1a87d13 to 8d71131 Compare April 13, 2020 02:44
@arichiardi arichiardi force-pushed the elchache-new-features branch 2 times, most recently from af6724a to 54df2b1 Compare April 15, 2020 00:55
It can now be executed with yarn shadow-cljs compile simple-example and node
dist/simple-example.js.
A couple of changes here. First of all the name specs in execute and core are
now the same and both allow keyword and string for :name keys.
Secondly, we refactor the specs so that including fonda.core.specs actually
adds everything to the registry.
Finally we fix the tests and issues that were detected with the change.
@arichiardi arichiardi force-pushed the elchache-new-features branch from edc837c to f6afbf4 Compare April 15, 2020 01:36
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

Successfully merging this pull request may close these issues.

2 participants