These are chat archives for lift/framework

7th
May 2016
Matt Farmer
@farmdawgnation
May 07 2016 13:06
Mornin’ folks.
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:04
'Morning!
Matt Farmer
@farmdawgnation
May 07 2016 14:04
How goes it?
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:05
Not too bad.
Been dealing with some ankle sprain annoyance this past week -.-
Matt Farmer
@farmdawgnation
May 07 2016 14:05
Oof.
That’s the second or third time that’s happened right?
Soccer?
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:11
No, this was a misstep at the wedding that loosened things up enough to lead to several additional missteps.
Matt Farmer
@farmdawgnation
May 07 2016 14:11
Womp
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:12
Yeah, it's the once-badly-sprained ankle that now has seemingly unfathomable accidents every once in a while.
Like, things that would just look like, you know, walking, but lead to a sprain instead for no obvious reason :/
Anyway, going to go see a doctor sometime in the next few weeks because I'm due.
And I'll probably be like “heey, so this ankle is the worst, what can I do about it”.
I think there's some PT that can be done to at least reduce the future chances of silliness.
Matt Farmer
@farmdawgnation
May 07 2016 14:18
Oof.
Matt Farmer
@farmdawgnation
May 07 2016 14:27
Hey codacy keeps failing things heh
And… I’m not entirely sure of the validity of those failures
Especially because I inspect a PR that claims “7 new issues” and the Codacy UI doesn’t show any
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:30
?
Which one?
Matt Farmer
@farmdawgnation
May 07 2016 14:30
Joe’s future with session
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:33
I kicked off a reanalyze, I think this may be because they did an overhaul a couple of months ago and that PR is ancient at this point.
(In code years :laughing:)
Matt Farmer
@farmdawgnation
May 07 2016 14:33
lol
Matt Farmer
@farmdawgnation
May 07 2016 14:41
Hey @Shadowfiend - I’m looking at an old ticket you filed
lift/framework#1146
This is actually super easy to implement so I’m just going to do it.
But I have a question.
The existing sendCometActorMessage creates a matching comet if one doesn’t already exist.
Should the “Send to all regardless of name” kind do the same?
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:43
Oh man inline expansion of GH PR description :heart_eyes:
Matt Farmer
@farmdawgnation
May 07 2016 14:44
?
Oh if you hover
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:44
Or click, actually. At least for me.
Matt Farmer
@farmdawgnation
May 07 2016 14:44
Word
WDYT?
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:45
The existing sendCometActorMessage creates a matching comet if one doesn’t already exist.
Wat.
Matt Farmer
@farmdawgnation
May 07 2016 14:45
Lol yeah
Lacking a matching comet it invokes setupComet
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:45
Yeah, but that doesn't create the comet.
Matt Farmer
@farmdawgnation
May 07 2016 14:45
Oh
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:45
Just queues a message up to be sent to it.
Matt Farmer
@farmdawgnation
May 07 2016 14:46
Well that’s a misleading name
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:46
Nahhhhhh.
(SO VERY YES)
Wouldn't mind renaming that and deprecating the old name, tbh…
Matt Farmer
@farmdawgnation
May 07 2016 14:46
Yeah, food for thought.
Hm
So part of the reason that works is that it’s a concurrent hash map with the type and name as a key.
Would need to rework the looking up of messages somehow.
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:47
findOrBuildComet is where we use it.
Oh, I see what you mean.
Yeah, I see some complications for sure…
Matt Farmer
@farmdawgnation
May 07 2016 14:49
So does it make sense for comets to pick up both messages for their type and name and messages for just their type?
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:50
Well, it would, except you can create a comet with a type and no name :/
(Indeed, that's the default.)
Matt Farmer
@farmdawgnation
May 07 2016 14:50
Right, that’s what I’m wondering.
So, type=foo name=Some(bar) and type=foo name=None would both pick up messages just for type=foo
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:50
So indicating “just for this type” becomes more complicated, because “just for this type and an Empty name” and “just for this type” look kinda similar.
Matt Farmer
@farmdawgnation
May 07 2016 14:51
Right, we’d need a correlary to CometId
That just doesn’t have a name field.
And probably a separate hash map for storing those messages.
Also funny, we can broadcast this message to all comets that already exist, but we would also need to call setupComet for this message.
Because there could be more matching comets that get instantiated.
Oof the cometPreMessages thing is a session var
Hm
So request A can queue up a message for such a comet, request B can create the comet, and the comet will get that message
I feel like we wouldn’t want that behavior if we were just searching by type
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:55
Er… Yeah. Yikes.
Well this is just a bundle of thorns, isn't it.
Matt Farmer
@farmdawgnation
May 07 2016 14:56
It is indeed.
Mature Codebase™
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:56
Tbh, I don't know that you'd expect that to happen with what's already there, either.
Matt Farmer
@farmdawgnation
May 07 2016 14:56
Yeah, that’s what I’m thinking about.
Okay, so I’m going to send a note to the list.
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:56
Imagine I run, in one request, a sendCometActorMessage. Then I spin up a comet. But in the intervening time, someone else has spun up the same comet.
Ah, but then they won't overlap.
Because if it's the same CometInfo, it's actually literally the same comet.
Matt Farmer
@farmdawgnation
May 07 2016 14:56
Different session
Yeah
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:57
I was thinking same session.
But CometInfo is enough in the same session.
Matt Farmer
@farmdawgnation
May 07 2016 14:57
Yeah, right.
But there can be multiples of the type
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:57
So with that in mind, I think doing it with a RequestVar for the type-only sitch makes sense?
Matt Farmer
@farmdawgnation
May 07 2016 14:57
Right
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:57
TransientRequestVar**
Matt Farmer
@farmdawgnation
May 07 2016 14:57
But that’s subtly different behavior
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:57
Which I guess incidentally is what we were doing at OpenStudy, too heh.
It's subtly, but probably not usually materially, different behavior.
Matt Farmer
@farmdawgnation
May 07 2016 14:58
Yeah. Maybe this is a “solve with documentation” thing.
Okay
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:59
Ironically though, sendCometActorMessagehas one thing that's incorrect in its description.
Namely, it doesn't matter if it's created in this request/response cycle.
It's whenever a matching comet is next created in this session that it'll get that message.
Matt Farmer
@farmdawgnation
May 07 2016 14:59
Hah
That is true
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 14:59
Unfortunately fixing that is a breaking behavior change.
So maybe it's something to deal with separately.
Matt Farmer
@farmdawgnation
May 07 2016 15:00
Yeah, I’ll tweak the scaladoc in the meaintime
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 15:00
(Ultimately, I'm basically saying that using a TransientRequestVar for both is probably the right long-term solution.)
Matt Farmer
@farmdawgnation
May 07 2016 15:01
Yep, agreed.
Matt Farmer
@farmdawgnation
May 07 2016 15:32
Alright, got a PR open for that @Shadowfiend
I’m out for now. :)
Antonio Salazar Cardozo
@Shadowfiend
May 07 2016 15:47
👌