Skip to end of metadata
Go to start of metadata

[15:24:25 CDT(-0500)] <Bosmon> Just having a conversation with colinclark and yura1 now about the failure of the LifecycleManager when processing settings handler payloads with no settings

[15:24:44 CDT(-0500)] <Bosmon> We've determined that the part of the infrastructure responsible for breaking its contract is the lifecycleManager

[15:25:30 CDT(-0500)] <Bosmon> In particular, the model transformation action on line 127 of LifecycleManager.js produces a "truncated tree" when asked to transform a settings handler return which ends with settings: {}

[15:26:03 CDT(-0500)] <colinclark> hi Bosmon

[15:26:11 CDT(-0500)] <yura1> Bosmon: that is correct

[15:26:18 CDT(-0500)] <Bosmon> As the comment on line 127 suggests, line 128 is already a hack to work around a deficiency in the model transformation system

[15:26:46 CDT(-0500)] <Bosmon> The very simplest fix would consist of just rewriting line 128 to consist of an action of fluid.get rather than a manual property fetch

[15:26:58 CDT(-0500)] <Bosmon> Which is what the fixed model transformation system would do in any case

[15:27:05 CDT(-0500)] <colinclark> that was yura1's first suggestion, in fact

[15:27:35 CDT(-0500)] <Bosmon> And if it turns out that $.extend on line 143 barfs on eating an "undefined" we can just rewrite it to fluid.merge, as yura1 suggested in his review comment

[15:27:55 CDT(-0500)] <Bosmon> The reason for that being $.extend was largely historical in any case - it was just left that way because an earlier version of the algorithm did a shallow merge

[15:27:55 CDT(-0500)] <colinclark> Bosmon: Is there any reason not to do that proactively?

[15:27:59 CDT(-0500)] <colinclark> switch to fluid.merge, that is

[15:28:12 CDT(-0500)] <colinclark> ah, it sounds like not

[15:28:25 CDT(-0500)] <Bosmon> Not really, no... it was just an early attempt at an optimisation

[15:28:37 CDT(-0500)] <Bosmon> This would make our $ resolution issues go away too - although it would be nice to have a better answer to that

[15:28:56 CDT(-0500)] <Bosmon> Otherwise we just go ahead and advertise that "jquery methods are not available for use by users of Infusion in node"

[15:29:27 CDT(-0500)] <Bosmon> But I think there are still valid use cases where shallow merges are a reasonable thing to want to do

[15:29:59 CDT(-0500)] <Bosmon> I was unable to find a pattern of using $ which was valid both in the browser and in node without removing the "use strict" directive at the head of the file

[15:30:38 CDT(-0500)] <Bosmon> Since we are definitely inside a closure once the file starts, it implies that any attempt to read/write to "$" separately from the closure argument counts as an "unexpected use of an unqualified global variable"

[15:30:41 CDT(-0500)] <yura1> Bosmon: so I guess my question is:

[15:31:44 CDT(-0500)] <Bosmon> I don't really like using fluid.merge when it is possible that $.extend might do the job, since I am painfully aware how expensive fluid.merge is : P

[15:31:52 CDT(-0500)] <Bosmon> But actually we have no evidence that $.extend is any faster

[15:32:25 CDT(-0500)] <Bosmon> That is, when it is doing deep merges

[15:32:31 CDT(-0500)] <yura1> if we do go ahead with changes to fluid.get and fluid.merge, is that just a "fix" to the bug or does it actually carry a conceptual improvement to the lifecycle manager ? e.g. if it returns an undefined snapshot, is that a good representation of settings that are not set ?

[15:32:40 CDT(-0500)] <yura1> or settings that set nothing rather ?

[15:33:02 CDT(-0500)] <Bosmon> yura1 - I think it is a true improvement, yes - but bear in mind this is largely an implementation-private utility function in any case

[15:33:11 CDT(-0500)] <colinclark> I guess there are still issues further up the stack

[15:33:22 CDT(-0500)] <Bosmon> Noone outside the lifecycleManager has any particular business calling gpii.lifecycleManager.responseToSnapshot

[15:33:27 CDT(-0500)] <colinclark> the first being that the matchmaker is barely existent

[15:33:28 CDT(-0500)] <Bosmon> It is just left there as an implementation courtesy

[15:33:50 CDT(-0500)] <colinclark> and presumably it may still make sense to weed out settings handlers that have no effect

[15:33:52 CDT(-0500)] <Bosmon> The only purpose for the return of this function is to be used as an argument within gpii.lifecycleManager.invokeSettingsHandlers

[15:34:03 CDT(-0500)] <Bosmon> For which purpose "undefined" is a perfectly good value

[15:34:19 CDT(-0500)] <Bosmon> colinclark - certainly

[15:34:38 CDT(-0500)] <colinclark> But the bugs are less pressing the further they move up the stack

[15:34:55 CDT(-0500)] <colinclark> yura1: are you going to implement the fix?

[15:34:58 CDT(-0500)] <colinclark> or is Bosmon?

[15:35:02 CDT(-0500)] <yura1> colinclark: sure

[15:35:07 CDT(-0500)] <colinclark> It seems easily testable, too

[15:35:11 CDT(-0500)] <Bosmon> yes

[15:35:22 CDT(-0500)] <Bosmon> Thanks, yura, I'll be happy to review (smile)

[15:35:28 CDT(-0500)] <colinclark> I should apologize for having committed this change to the Solutions Registry that shows this bug

[15:35:31 CDT(-0500)] <yura1> Bosmon: so fluid merge would get rid of an undefined snapshot ?

[15:35:38 CDT(-0500)] <colinclark> I suck, in that I made a change and only tested with Timmy

[15:35:51 CDT(-0500)] <yura1> Timmy is awesome (smile)

[15:35:57 CDT(-0500)] <Bosmon> yura1 - certainly fluid.merge will behave correctly, even if $.extend doesn't

[15:36:04 CDT(-0500)] <colinclark> Kasper wanted to throw Timmy off the ship

[15:36:14 CDT(-0500)] <colinclark> my desperate measures to save him caused everyone else to slip and fall

[15:36:19 CDT(-0500)] <colinclark> or so the metaphor goes

[15:36:42 CDT(-0500)] <yura1> ok colinclark, Bosmon ill have pull request fairly soon

[15:36:56 CDT(-0500)] <colinclark> Bosmon: Once you review it, I guess I'll get my beloved MoonBook Pro back (smile)

[15:37:01 CDT(-0500)] <Bosmon> My "by eye" reading of jquery.extend suggests that it will fail

[15:37:03 CDT(-0500)] <Bosmon> if ( (options = arguments[ i ]) != null ) {

[15:37:13 CDT(-0500)] <Bosmon> But I've found it is very hard to assess jquery code by eye : P

[15:37:39 CDT(-0500)] <colinclark> so yura1, one other question

[15:37:43 CDT(-0500)] <Bosmon> It seems that in general, the jQuery developers have declared war on "undefined"

[15:37:53 CDT(-0500)] <colinclark> If we're doing demos and presentations over the next two weeks, do you think we're in good shape?

[15:37:55 CDT(-0500)] <Bosmon> Since their framework throws all kinds of unpleasant errors when it is used

[15:38:04 CDT(-0500)] <colinclark> I guess if we fix this, and do a bit more testing of the RFID work on Windows, we should be all set

[15:38:17 CDT(-0500)] <yura1> well there's the new issue that we had yesterday with an rfid on windows

[15:38:20 CDT(-0500)] <colinclark> I'd love it if we could get to the point where we had a definitive version of the RFID User Listener working on both platforms

[15:38:33 CDT(-0500)] <yura1> ya that's the goal

[15:38:34 CDT(-0500)] <colinclark> My guess is that you were using the wrong revision of the RFID reader, perhaps?

[15:38:40 CDT(-0500)] <colinclark> Do you remember off the top of your head which version you ran?

[15:38:51 CDT(-0500)] <colinclark> I have certainly experienced flakiness myself

[15:39:04 CDT(-0500)] <colinclark> it failed to fully work when I demoed it to my Dad on Sunday (tongue)

[15:39:19 CDT(-0500)] <yura1> well on windows i did not touch the code at all

[15:39:23 CDT(-0500)] <colinclark> right

[15:39:27 CDT(-0500)] <colinclark> but which executable did you run?

[15:39:30 CDT(-0500)] <colinclark> there were several

[15:39:35 CDT(-0500)] <yura1> oh

[15:39:44 CDT(-0500)] <yura1> the one in release folder in that dir on c drive

[15:39:48 CDT(-0500)] <colinclark> hmm

[15:39:57 CDT(-0500)] <colinclark> that should have worked with the tokens except for Carla

[15:40:14 CDT(-0500)] <colinclark> Version 1.04, if I remember correctly, authenticates with the "A" key and reads the first 16 bytes

[15:40:26 CDT(-0500)] <colinclark> Version 1.05 authenticates with the B key, also reading the first byte

[15:40:54 CDT(-0500)] <colinclark> and the latest, I think version 1.11, actually reads data from some other region, and I'm not really sure which key he authenticates with

[15:40:56 CDT(-0500)] <colinclark> perhaps both

[15:41:28 CDT(-0500)] <yura1> colinclark: well i will try again and see if it works

[15:42:21 CDT(-0500)] <colinclark> worth experimenting with, yes

[15:42:31 CDT(-0500)] <colinclark> I think I forwarded you the email from david mork with the latest revision

[15:42:36 CDT(-0500)] <colinclark> Bosmon: Did you get an RFID reader?

[15:42:41 CDT(-0500)] <colinclark> I asked for one to be shipped to you

[15:45:06 CDT(-0500)] <Bosmon> colinclark - I haven't received on yet

[15:45:07 CDT(-0500)] <Bosmon> one

[15:45:13 CDT(-0500)] <Bosmon> It would be great to get a "faulty USB stick" too

[15:45:28 CDT(-0500)] <colinclark> oh yes

[15:45:38 CDT(-0500)] <colinclark> I'll talk to Pat and Iris about shipping one

[15:45:44 CDT(-0500)] <Bosmon> Actually the USPS van is around just as we speak

[15:45:51 CDT(-0500)] <Bosmon> But I don't see he has anything package-like in his bundle

[15:45:58 CDT(-0500)] <colinclark> It may be that Gregg didn't get the chance

[15:46:06 CDT(-0500)] <colinclark> he seemed rather random in his approach

[15:46:18 CDT(-0500)] <colinclark> should I get him to send you one?

[15:46:21 CDT(-0500)] <Bosmon> I would have expected it would have arrived by now

[15:46:26 CDT(-0500)] <colinclark> yes

[15:46:41 CDT(-0500)] <Bosmon> USPS standard seldom takes more than 5 days

[15:46:55 CDT(-0500)] <Bosmon> And most usually takes 2

[15:47:02 CDT(-0500)] <Bosmon> Sure, it would be helpful to be able to test these things

[15:47:20 CDT(-0500)] <Bosmon> And to see what has become of this prehistoric technology in 12 years

[15:47:33 CDT(-0500)] <Bosmon> I think I still have an old JavaCard compatible smartcard reader in my attic somewhere....

[15:51:15 CDT(-0500)] <yura1> ok so fluid.get needs to resolve a org.gnome.desktop.a11y.magnifier key for example, Bosmon you mentioned there is a utility for that ?

[15:51:21 CDT(-0500)] <Bosmon> So, yura1 has rightly spotted a significant issue with the use of unvarnished "fluid.get" on such a key

[15:51:25 CDT(-0500)] <Bosmon> Yes, there is

[15:51:35 CDT(-0500)] <Bosmon> But I just recalled that this is one of the ones that are not upgraded for dealing with escaped paths

[15:52:28 CDT(-0500)] <Bosmon> This is actually going to be a bit of a mess, and it may well work out quicker just to write a couple of if statements

[15:52:31 CDT(-0500)] <Bosmon> disagreeable though this is

[15:53:54 CDT(-0500)] <colinclark> yes, interesting

[15:54:45 CDT(-0500)] <Bosmon> https://github.com/amb26/infusion/blob/FLUID-4695/src/webapp/framework/core/js/ModelTransformations.js#L585-592

[15:54:52 CDT(-0500)] <Bosmon> yura1 - you can see the required configuration here

[15:54:56 CDT(-0500)] <Bosmon> As you see it is a little bulky

[15:56:14 CDT(-0500)] <Bosmon> It's your choice : P

[15:56:47 CDT(-0500)] <Bosmon> I'm sure colinclark will be happy to let you resolve which better serves the aims of MAKE IT WORK and MAKE IT MAKE SENSE in this case : P

[15:57:23 CDT(-0500)] <colinclark> (smile)

[16:13:43 CDT(-0500)] <Bosmon> colinclark - ayt?

[16:13:46 CDT(-0500)] <Bosmon> You seemed to drop off IM

  • No labels