search

arichiardi / fonda

An async pipeline approach to functional core - imperative shell.
Apache License 2.0
22 stars 1 forks source link

not providing a name in a step still results in an error #35

Closed ElChache closed 5 years ago

ElChache commented 5 years ago

Because we are using records for the steps, the spec for the name in the step must accept a nil name. The spec right now just requires the name to be optional:

(s/def ::name (s/nilable string?))
(s/def ::step-common
  (s/keys :opt-un [::name]))

That would work for a map, but not for a record, where name is actually nil. So spec should be:

(s/def ::name (s/nilable string?))
(s/def ::step-common
  (s/keys :opt-un [::name]))
arichiardi commented 5 years ago

Good catch

arichiardi commented 5 years ago

I saw that we have all the keys req-un for FondaContext. Is this required in any record?

I guess it is for the steps as well?

ElChache commented 5 years ago

I guess that's because it's a record and the specs are all nilable... but I think it would be more correct to have the ones that are optional as :opt-un.

One more thing, I think the ::fonda-context-sync spec in fonda.execute.specs is wrong, as it uses namespaced keywords like ::anomaly?, ::ctx and so on that are now defined on a different namespace.

I wonder why the tests didn't error with that

arichiardi commented 5 years ago

Uhm maybe we are never using that spec in any function? I will fix it.

Opt sounds better definitely

arichiardi commented 5 years ago

Should be fixed in #39