Fork me on GitHub
#yada
<
2015-08-28
>
mccraigmccraig17:08:31

@malcolmsparks: did you manage to replicate the problem i was having ?

malcolmsparks17:08:56

no, I haven't: this test passes on -5 😞

malcolmsparks17:08:32

all I changed was (generate-string) to (str) , because I didn't have the source to (generate-string)

malcolmsparks17:08:59

ah, now I've replicated it:

malcolmsparks17:08:47

@mccraigmccraig: hi there, I think I've got to the bottom of the issue

malcolmsparks17:08:28

I've changed your chain-delay-deferred to the following:

clojure
(defn chain-delay-deferred
  [n t]
  (let [f (d/deferred)]
    (future (Thread/sleep t) (d/success! f 100))
    (let [sum (reduce (fn [d i] (d/chain d (fn [v] (+ v i))))
                      f
                      (range 0 n))]
      (d/chain sum
               (fn [v]
                 @(d/future (Thread/sleep 10) (str v)))))))

malcolmsparks17:08:43

note the deref on the last line

mccraigmccraig17:08:33

yep... hmm. i'll have to have another look... presumably that last deref is just adding in a delay, so maybe you can get rid of most of chain-delay-deferred and replace it with (d/chain (d/success-deferred 10) (fn [v] (future (Thread/sleep 10) (str v))))

malcolmsparks18:08:09

i'm getting deeper into this

malcolmsparks18:08:13

it's a bit weird

malcolmsparks18:08:43

I'm on working on tag -5

malcolmsparks18:08:06

it works if I remove all but the invoke-method interceptor

malcolmsparks18:08:16

it only hangs if there's at least one interceptor before the invoke-method interceptor AND (at least) one after it. I'm trying to reduce down to a hanging test using only manifold

malcolmsparks18:08:05

I was going to suggest that I thought the problem is that your d/chain (itself a deferred) was returning a deferred in the last link of the chain

malcolmsparks18:08:18

but after some testing, I've discovered that's perfectly ok with manifold

malcolmsparks18:08:51

but just not in the context of yada, which is a rather contorted manifold structure - they'll be a reason, I just can't find it yet! simple_smile

mccraigmccraig18:08:20

well, i guess i'm happy you've managed to replicate !

malcolmsparks18:08:20

despite this issue, I am gaining further respect for manifold, it really is flexible and lets you exploit async without thinking too hard - @ztellman did a great job

mccraigmccraig18:08:24

yeah, i've been really happy with it - in combo with a cats monad on top of it i've been building what would otherwise be insanely complex chains, and they are very straightforward to grok and work with

malcolmsparks18:08:48

have you any links to the cats monad stuff I can read up on?

mccraigmccraig18:08:29

there's this, which is out of date and referring to core.async rather than manifold - https://github.com/funcool/canal

mccraigmccraig18:08:35

but the idea is the same

mccraigmccraig18:08:44

there's probably not that much difference between using the d/chain and monad approach ... i was already using the monad for error-handling elsewhere though, so i went with that

malcolmsparks18:08:05

in other news, if you forgive the aside, I've been playing with yada a bit - I've noticed that the result of yada/handler (which is aliased as yada/yada now), is a record, that itself can be fed into yada - if you want to expose the metadata of the handler as a REST service

malcolmsparks18:08:14

so you could do this

malcolmsparks18:08:23

(-> "Hello World!" yada yada)

malcolmsparks18:08:48

(-> "Hello World!" yada yada yada)

mccraigmccraig18:08:42

'cos i knew what you were going to type for the second example there

mccraigmccraig18:08:05

i've been having a pretty good time with yada and re-frame... it all feels like webapps done, if not completely right, then pretty close

malcolmsparks18:08:00

yes, i've been having a lot of fun with re-frame too, it's a wonderful thing -

malcolmsparks18:08:27

I'm building a yada console with material design classes and re-frame to inspect and analyse/debug yada calls

malcolmsparks18:08:41

It's also nice to be able to use yada in a real system

malcolmsparks18:08:50

and experiment with the possibilities

mccraigmccraig18:08:55

that sounds pretty cool !

malcolmsparks18:08:54

it's early days, but I think if I'm asking people to trade in their Ring middleware for yada, the least I can do is to provide a decent debug experience when things go awry

mccraigmccraig18:08:48

it won't do any harm... but swagger, extraction and coercion of params, async, representations, auth and spec conformance are killer features too

malcolmsparks18:08:34

well, I'm sure it's possible to 'lift' Ring middleware into a manifold chain, so it might not be a case of yada and Ring middleware being mutually exclusive -

malcolmsparks18:08:54

I've got a todo item somewhere to investigate that

malcolmsparks18:08:42

I'm thinking of things like prone, rather than the Ring middleware that tries to do http-like things like wrap-head and wrap-last-modified, which are pretty broken because they don't reduce any load on the server in their implementations

mccraigmccraig18:08:47

oh, i hadn't seen prone - that looks awesome

malcolmsparks18:08:52

thx for the kind feedback too btw

malcolmsparks18:08:59

yeah prone is very nice

malcolmsparks18:08:32

it's partly what influenced me to build this yada console thing

malcolmsparks18:08:43

prone has the request context, the response, any errors, etc. but a yada console gets to show available methods/representations, content-negotiation scores, interceptors invoked, and a whole lot more besides

malcolmsparks18:08:44

not forgetting of course that a yada resource is itself a rich data structure, it's not just a Ring handler

malcolmsparks18:08:19

ok, i'm going to have another stab at your bug simple_smile

malcolmsparks19:08:11

the problem is due to yada's construction of a manifold chain via clojure.core/apply

malcolmsparks19:08:31

I'd like to invite @ztellman here to see if he has any thoughts

malcolmsparks19:08:15

(I don't know what the netiquette of summoning people to slack channels is, I hope Zach doesn't mind)

malcolmsparks19:08:40

going back to simplify further

mccraigmccraig19:08:49

ah, i see - perhaps use reduce rather than apply then : https://www.refheap.com/108923

malcolmsparks19:08:23

you're a genius, that works!

ztellman19:08:10

what’s up?

ztellman19:08:21

sorry, accepted the invite and then spaced out

malcolmsparks19:08:44

hi zach! just been puzzled by a little issue in our use of manifold

malcolmsparks19:08:51

i've raised #50 to show some examples

ztellman19:08:58

ok, taking a look now

malcolmsparks19:08:08

we have a workaround (using reduce rather than apply)

malcolmsparks19:08:29

but from something you warned me about before, I'm concerned reduce might have a performance impact - my chain is about a dozen long

ztellman19:08:52

what version, btw?

malcolmsparks19:08:10

[manifold "0.1.0"]

mccraigmccraig19:08:38

i encountered the problem on 1.1.0-alpha3, since i overrode yada's dep

malcolmsparks19:08:50

we can definitely upgrade manifold if that helps

mccraigmccraig19:08:11

oops 0.1.1-alpha3

malcolmsparks19:08:14

ah, sorry @mccraigmccraig you tried that already

ztellman19:08:59

so if you just call (chain a b c) that does an inline expansion

malcolmsparks19:08:02

i've been trying to simplify the examples as far as possible, the bug only shows itself in certain configurations

ztellman19:08:09

if you do (apply chain [a b c]) that calls the non-inlined form

ztellman19:08:16

this suggests a bug in my non-inlined form

ztellman19:08:24

which I will now look into

malcolmsparks19:08:39

potentially I don't know upfront the interceptor chain, I need the dynamic version...

ztellman19:08:13

right, and that should work perfectly fine

ztellman19:08:27

nothing wrong with your approach, just (apparently) with my code

malcolmsparks19:08:59

if you look at #50, you'll see that tinkering around with the inclusion of identity in the chain makes a difference -it's not just a problem with apply in isolation

ztellman19:08:19

ok, replacing chain with chain’ on the hanging example also fixes it

malcolmsparks19:08:22

oh, the arg count might make a difference here, I didn't spot that

ztellman19:08:48

ok, just pushed a fix, now to cut a version

ztellman19:08:11

I’m going to try to figure out why my existing tests didn’t catch this

ztellman19:08:17

that’s still a bit mysterious

malcolmsparks19:08:40

what is being coerced here? what's the difference between chain and chain' - I'm struggling a bit

ztellman19:08:39

so chain’ is the faster cousin of chain, because instead of checking if a value is either a deferred or can be coerced to a deferred, it only checks if it’s a deferred

ztellman19:08:46

they have entirely disjoint implementations

ztellman19:08:06

chain’ didn’t have the implementation error

malcolmsparks19:08:23

so clojure.core/future is not a deferred but can be coerced to one?

ztellman19:08:38

but you were using d/future, which does return a deferred

ztellman19:08:48

so chain’ would have been the “optimal” thing to use

malcolmsparks19:08:00

ok, i'll need the slow version I think - I think I need to let my users return clojure.core futures and promises

ztellman19:08:10

yeah, again, nothing wrong with anything you’re doing

ztellman19:08:18

this is all on me

malcolmsparks19:08:41

unless I can coerce on the user boundary and internally use manifold defereds exclusively, then I'll get some boost with chain' right?

malcolmsparks20:08:42

great - thanks Zach - I'll switch to the bugfix release when it's ready

ztellman20:08:02

yeah, you’ll get some small boost

ztellman20:08:07

I wouldn’t worry about it

malcolmsparks20:08:53

ok, I'll bear it in mind when I start profiling

ztellman20:08:23

great, ok, figured out why the test didn’t catch it, either

ztellman20:08:31

one sec while I cut a release

malcolmsparks20:08:41

oh great - glad you found that

malcolmsparks20:08:39

glad we've found the issue because otherwise manifold has been working out really well - extremely easy to give to users

ztellman20:08:20

sorry for this, pretty silly oversight on my part

malcolmsparks20:08:43

absolutely no problem - thanks so much for helping out and fixing

ztellman20:08:56

0.1.1-alpha4 is deployed

malcolmsparks20:08:06

cool - I'll cut a new yada release now

malcolmsparks20:08:24

Ran 39 tests containing 229 assertions. 0 failures, 0 errors.

malcolmsparks20:08:35

yada likes the new manifold - am going to release

malcolmsparks20:08:25

yada 1.0.0-20150828.201157-7 is deployed

mccraigmccraig20:08:38

w00t ... i'll try it out next time i'm allowed near a keyboard

malcolmsparks21:08:26

try the 1.0.0-20150828.203727-8 release, I messed up

mccraigmccraig21:08:21

just tried the -8 release - no hangs 😄