Fork me on GitHub
#untangled
<
2017-04-06
>
mitchelkuijpers15:04:39

I am running into an issue for which I know a solution, but i am curious if this was such a wrong approach. I get a callback from a user action which returns 0 or more things that I need to save let's call that action foo/save And after that is done I load the thing by doing a df/load. No when I run this only the last foo will get loaded because my tx looks lik this [(foo/save {:id tempid1}) (foo/save {:id tempid2})] And now the server will only return the mutation tempid mapping for the last action. No I can make a foo/save-bulk but that does not give me anything expect for more code

tony.kay16:04:15

@mitchelkuijpers So, this is a constraint of Om. The return from the server is a map, keyed by mutation name. I thought we just put in a patch that would make those go as separate network requests

tony.kay16:04:35

though, I do not recommend triggering load from within a mutation

tony.kay16:04:44

that is what load-action and remote-load are for

tony.kay16:04:15

and, for that matter, direct access to the untangled/load mutation within your top-level tx

tony.kay17:04:00

so: 1. Yes, you might want a save-bulk, just because that would reduce network traffic 2. What you are doing should work in the latest version (the mutations should cause two net requests) 3. If you're CALLING load from within a mutation, that is not a supported thing. Use composition (`load-action` + remote-load) or directly use the untangled/load mutation in your tx.

tony.kay17:04:27

On (3), it sounds since you already have a remote mutation, you'd need an addition to the tx for the loads...which you could write as a mutation and use load-action, or just directly use untangled/load: (transact! this [(foo/save {}) (untangled/load {}) (foo/save {}) (untangled/load {})])

tony.kay17:04:47

Not sure why you need the loads at all, since you knew all of the novelty on the client to begin with...I guess the server is adding novelty as well? If that is the case you could also use the server's mutation return values by adding a mutation-merge handler, which would eliminate the need for the load

mitchelkuijpers17:04:39

@tony.kay on 2 should that work in 0.8.1? Because I am running that

tony.kay17:04:25

Yes...let me look up the patch

tony.kay17:04:59

OK. The patch I was thinking of was by jasonjckn

tony.kay17:04:08

and it covered reads with this problem

tony.kay17:04:15

so, perhaps it isn't fixed for mutations

mitchelkuijpers17:04:25

Btw I am doing a (om/transact!) and a (df/load) from where I get the callback with the id of id's of which I only know that. I thought about using untangled/load directly but was not sure that was a valid approach

mitchelkuijpers17:04:25

But I think I change it to a bulk mutation and then run the reads, that seems like the most simple solution

tony.kay17:04:31

ok, remote mutations will be queued in front of reads, so that should be ok...it's mainly the same-named mutations that are causing your issue, I think

tony.kay17:04:39

yes, what you said

tony.kay17:04:15

I think I will put in an issue, though. Untangled can solve Om's problem here. We have a sequential queue...we can separate out the mutations

mitchelkuijpers17:04:02

That is a good point, it would be nice to just work. but do you want to run all mutations and then the reads?

tony.kay17:04:10

Yes. All mutations should go before reads that were queued at the same time

tony.kay17:04:32

normally, that means we order the mutations first and reads seconds and send one net request

tony.kay17:04:44

but in cases where there are collisions on name, we have to split it up

mitchelkuijpers17:04:49

Might also be nice to do some magic on the server and still send one mutation

tony.kay17:04:06

the one mutation thing is up to you

tony.kay17:04:11

I cannot morph that for you

mitchelkuijpers17:04:17

That is a good point

tony.kay17:04:54

but if you say: transact w/mutation, load, load, transact w/mutation (on one thread sequence), then we do send: mutation mutation load load

tony.kay17:04:00

as one network request

tony.kay17:04:08

which is also a problem

tony.kay17:04:11

same problem really

tony.kay17:04:16

if the mutations collide in name

tony.kay17:04:21

or the loads for that matter

tony.kay17:04:27

but the loads are already fixed

tony.kay17:04:32

we just didn't do it for mutations

mitchelkuijpers17:04:47

Maybe you want to just log a warning?

tony.kay17:04:48

cause we didn't think ppl would need to send more than one of the same

tony.kay17:04:58

nah, we can split them. That is the general purpose solution

tony.kay17:04:21

we could log a warning that we have to split it...so you could optimize

tony.kay17:04:35

but the optimization would be what you're doing...writing a single bulk mutation

tony.kay17:04:02

so we could say you're optimizing your code, and avoiding a temporary bug as a side-effect 😉

mitchelkuijpers17:04:59

Btw we are thinking about moving to untangled/om-css we used to use the other ladderlife/om-css one but we have some issues with it since we are moving to server-side rendering. Composing css to root is actually insanely cool for getting something on screen as fast as possible. Because you can just inject a style tag with the absolute minimal css needed to show what is on screen

mitchelkuijpers17:04:40

It was not written to support that but the idea pretty much fixed SSR

tony.kay17:04:46

nice. It's a super-small lib...I put it out there as more of a proof-of-concept. I'd love untangled-ui to use it, though...would make themeing a pure cljs task

mitchelkuijpers17:04:50

My intern is fixing that pull request

tony.kay17:04:53

NO CSS tooling would be soooooo sweet

tony.kay17:04:01

Oh, that's you guys. cool!

mitchelkuijpers17:04:15

No, that is someone else

mitchelkuijpers17:04:24

We saw the almost done pull request

mitchelkuijpers17:04:26

And we need it 😛

tony.kay17:04:37

it shouldn't take much

mitchelkuijpers17:04:02

No and it is great fun for intern

tony.kay18:04:07

@mitchelkuijpers Updated 0.8.2-SNAPSHOT on clojars. It now does tx splitting to fix what we discussed. Repeat calls to a remote mutation will now get split into separate network requests. Order of mutation1 remote-read1 mutation2 remote-read2 in a single tx should end up as 4 network requests (assuming you have a dupe key on reads as well): mutation1, mutation 2, remoter1, remoter2 If nothing is duplicate, it will be one net request. Basically, does the "right thing" to make sure that Om's map return value won't cause loss of data when a tx has duplicate keys/mutation names

wilkerlucio19:04:44

@tony.kay do you think it's feasible to have a dynamic endpoint on the network handler? I found myself in a situation where having it dynamic would be handy, because the UI is working on a service that is sharded at our company, so I would like the user to be able to switch the shard he is accessing during runtime, and so each shard needs to target a different URL to do the calls, do you have another idea on how to handle this?

wilkerlucio19:04:06

the best I can think of as now is to create a new network handler that uses some external atom to select the shard and then internally re-uses the standard network one, not super ideal, so please let me know if you have a different idea

cjmurphy20:04:30

@wilkerlucio: Is this what you mean: https://github.com/untangled-web/untangled-server/pull/11 - however it is not so much dynamic as multiple - but the client can change its mind, so dynamic in that respect.

wilkerlucio20:04:42

I see this PR is on the server side, the server is not the problem, but the client

wilkerlucio20:04:51

because we have the same service deployed in multiple shards

wilkerlucio20:04:13

so each it's like it's own island, what I want is to be able, on the client, to change to which URL I'm going to point to

wilkerlucio20:04:31

currently you can only setup the network once at the startup, then there is no way to change it in any manner

cjmurphy20:04:07

Right, I understand the issue.

wilkerlucio20:04:55

but you just made me think, maybe there is a way to change, after all the networking is a record in a map, maybe there is a way to change it, I'm going to look on that, thanks 🙂

cjmurphy20:04:23

:networking can only be set, not changed.

cjmurphy20:04:53

- that's what you are saying.

wilkerlucio20:04:04

but this networking goes into the @app atom right? what if I just swap the atom and update the record?

cjmurphy20:04:32

Yes perhaps it can be changed on the client...

tony.kay20:04:18

@wilkerlucio So, this is something the user would actually be able to choose?

wilkerlucio20:04:45

yes, the user should be able to choose on which shard he wants to use the service

wilkerlucio20:04:07

it's related to managing dead messages on the kafka processes, so the user knows in which shard he wants to operate

tony.kay20:04:08

At start, or at any time?

wilkerlucio20:04:19

any time preferably

tony.kay20:04:11

So, I'd probably do it as a custom networking on the client, with an atom that you can globally change (from within a mutation). The networking object would then select the remote

tony.kay20:04:35

of course, you have to make sure the browser is going to allow it...e.g. no cross domain

wilkerlucio20:04:08

that's fine, the service has a graph api open for CORS (it only runs in internal networks, so it's safe to open CORS)

tony.kay20:04:58

yeah...you could also make a special mutation, like switch-remotes, that you set remote=true to. Hide the atom in your networking. When then networking sees a request to send switch-remotes, it handles thta instead of sending it

tony.kay20:04:23

that's how we handle loads....the mutation comes in as a (remote) load mutation, and the processing layer transforms it to a query

wilkerlucio20:04:42

yeah, I think I'll try with that, I just tried (swap! app assoc-in [:networking :remote :url] "") but that didn't worked

tony.kay20:04:29

not sure why you expected it to...app-state has nothing to do with networking

tony.kay20:04:03

Yeah, the running app is mounted...you just made a new data structure that nothing is using, and stored it in app

tony.kay20:04:13

the existing code already closed over the networking

wilkerlucio20:04:14

(defrecord DynamicNetwork [url-atom network]
  net/NetworkBehavior
  (serialize-requests? [this] (net/serialize-requests? network))

  net/IXhrIOCallbacks
  (response-ok [this xhr-io valid-data-callback]
    (net/response-ok network xhr-io valid-data-callback))
  (response-error [this xhr-io error-callback]
    (net/response-error network xhr-io error-callback))

  net/UntangledNetwork
  (send [this edn ok error]
    (-> network
        (assoc :url @url-atom)
        (net/send edn ok error)))
  (start [this app]
    (net/start network app)))

wilkerlucio20:04:42

@tony.kay @cjmurphy worked as a charm 🙂 ^^^

tony.kay22:04:02

yeah, that looks good