| Time |
Nick |
Message |
| 07:35 |
|
collum joined #evergreen |
| 08:34 |
|
Dyrcona joined #evergreen |
| 08:39 |
|
mmorgan joined #evergreen |
| 08:45 |
|
kmlussier joined #evergreen |
| 08:50 |
kmlussier |
Good morning #evergreen! |
| 08:50 |
kmlussier |
@coffee [someone] |
| 08:50 |
* pinesol |
brews and pours a cup of El Salvador Pacamara Finca Los Alpes Tierra Fertil, and sends it sliding down the bar to abneiman |
| 08:50 |
kmlussier |
@tea [someone] |
| 08:50 |
* pinesol |
brews and pours a pot of Honey Black Tea, and sends it sliding down the bar to jblyberg7 (http://ratetea.com/tea/health-and-tea/honey-black-tea/7529/) |
| 08:55 |
|
terranm joined #evergreen |
| 08:55 |
|
Dyrcona joined #evergreen |
| 08:59 |
mmorgan |
Good morning! |
| 09:06 |
|
terranm58 joined #evergreen |
| 09:07 |
|
dguarrac joined #evergreen |
| 09:26 |
mmorgan |
abneiman++ |
| 09:33 |
|
kmlussier1 joined #evergreen |
| 09:40 |
kmlussier |
abneiman++ Release_Team++ |
| 09:50 |
|
mantis1 joined #evergreen |
| 10:08 |
Dyrcona |
Bmagic++ # For all of his help over the weekend. |
| 10:22 |
|
terranm joined #evergreen |
| 10:33 |
|
BDorsey joined #evergreen |
| 10:52 |
|
jvwoolf joined #evergreen |
| 10:53 |
|
sandbergja joined #evergreen |
| 10:53 |
|
briank joined #evergreen |
| 10:54 |
sandbergja |
abneiman++ # the 3.12 release notes are looking great! |
| 10:58 |
terranm |
abneiman++ |
| 11:00 |
|
mmorgan1 joined #evergreen |
| 11:02 |
|
redavis joined #evergreen |
| 11:32 |
|
jvwoolf joined #evergreen |
| 11:41 |
|
JBoyer joined #evergreen |
| 12:14 |
|
jihpringle joined #evergreen |
| 12:30 |
|
collum joined #evergreen |
| 12:33 |
|
Dyrcona joined #evergreen |
| 12:50 |
|
collum joined #evergreen |
| 12:57 |
|
Dyrcona joined #evergreen |
| 13:15 |
eeevil |
sandbergja: I'm pointing this in your general direction because you've been AWESOME about pushing for more test automation ... do you have thoughts about where we might want to stick the following test? `(cd Open-ILS/examples && xmllint --schema fm_IDL.xsd --noout fm_IDL.xml)` |
| 13:19 |
sandbergja |
eeevil: ooh, good question! Maybe we could lightly wrap it in a perl test? Like run that command in backticks, and confirm that the exit code is 0? |
| 13:20 |
sandbergja |
or something along those lines |
| 13:21 |
eeevil |
maybe as part of `make check` directly? I don't have strong feelings about it, but would prefer it yell at us easily/quickly |
| 13:22 |
sandbergja |
yes, +1 to yelling :-) |
| 13:23 |
sandbergja |
example of the perl approach in the first subtest here: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=commitdiff;h=2f264c6ad779bacef4dd910bc63ece0174f82083 (the ok($output, 'runs without error');) |
| 13:24 |
Dyrcona |
if you only want the return value, system() would be preferable to backticks, and if you care about the output, I'd prefer a pipe open, but that's just my Perl style.... |
| 13:25 |
sandbergja |
Dyrcona++ # thanks! That is good to know. |
| 13:27 |
Dyrcona |
Just running it from 'make check' would also be good. |
| 13:49 |
terranm |
Bmagic or anyone - I'm trying to set one of my test servers to have the Czech option in the staff client and I can't figure out what I'm missing. These are the set up instructions I have so far: https://wiki.evergreen-ils.org/doku.php?id=newdevs:i18n |
| 13:49 |
terranm |
Of course, now it is working |
| 13:50 |
terranm |
No it's not. Argh |
| 13:52 |
terranm |
at https://terran-testbox.gapines.org/eg/staff - Czech is working in the OPAC. On the eg login page the Czech option is appearing in the top menu, but when logged in and on an eg2 page it's not there |
| 13:55 |
|
jvwoolf left #evergreen |
| 14:30 |
|
jvwoolf joined #evergreen |
| 14:33 |
|
jihpringle joined #evergreen |
| 14:43 |
JBoyer |
terranm, I think there's a 1-line change required to turn on Czech translations in the Angular client, because it's not smart enough to enable or disable it based on presence. |
| 14:45 |
JBoyer |
Oh, yeah, it's in Open-ILS/src/eg2/src/environments/environment(.prod).ts, depending on how you build the client, and you just need to add 'cs-CZ' to the locales array |
| 14:59 |
jeff |
added more comments to bug 2044033 |
| 15:00 |
pinesol |
Launchpad bug 2044033 in Evergreen "Make Evergreen resilient in the face of Chrome's prefetching and prerendering" [Undecided,Confirmed] https://launchpad.net/bugs/2044033 |
| 15:00 |
jeff |
found a way to more reliably reproduce. |
| 15:14 |
eeevil |
uses_pooled_read_replica_dbs is breaking ... everything? wheeeeeeee |
| 15:17 |
Dyrcona |
@swill eeevil |
| 15:17 |
* pinesol |
grabs a forty of Black Bull and sends it sliding down the bar to eeevil |
| 15:20 |
eeevil |
Dyrcona: that's not helping my unbreak whatever this crap is, but hopefully will ease the pain, I guess |
| 15:20 |
berick |
eeevil: what are you seeing? |
| 15:21 |
eeevil |
(just seems like this could be done transparently in the register_method wrapper) |
| 15:21 |
eeevil |
an error in the console, complaining that there's no function for looking up the authoriative-available setting |
| 15:21 |
eeevil |
in the browser console |
| 15:22 |
berick |
hm. there's a method in the code. |
| 15:24 |
berick |
btw the client-side sutff behaves that way to avoid the manual begin/rollback's that come directly through pcrud. separate from the .authoritative apis. |
| 15:24 |
eeevil |
but it's an api-breaking change at the angular level ... I regret I wasn't around for the discussion, but this seems like the most-breakage-possible way of implementing that |
| 15:24 |
|
jihpringle joined #evergreen |
| 15:26 |
berick |
er, the API and Angular changes are part of the same commit. |
| 15:33 |
|
mantis1 left #evergreen |
| 15:40 |
eeevil |
"I haven't changed any perl, I'll just grab the commits that make this new UI work" ... splat. anyway, full reinstall of everything it is |
| 15:57 |
eeevil |
berick: so, since it's storing this in the browser via setLocalItem, it can get into a state where it's generally broken for the app, it looks like. can't be sure you can reuse a server without browser content surgery (or blowing everything local away), AFAICT. |
| 15:58 |
Bmagic |
terranm: did you get that worked out (sorry just tuning in) |
| 15:58 |
|
jvwoolf joined #evergreen |
| 15:59 |
|
jvwoolf left #evergreen |
| 16:02 |
eeevil |
ISTM the angular pcrud client could have just started ignoring the authoritative flag. or remove the 15 uses of it via the pcrud service, after confirming that those uses /really/ don't want their own transaction for some reason (like isolation from in-page concurrent requests). because... you can't force that isolation explicitly now, anyway. |
| 16:12 |
berick |
ignoring the flag, which was my original goal, kind of defeats the purpose of having a config toggle to turn the feature on. |
| 16:14 |
berick |
and to be clear it only impacts pcrud read calls. writes all still go though transactions and transactions can be manually started. |
| 16:15 |
berick |
but let's move it out of localStorage. the goal was only to avoid having to make that API call on every page load. toss it in a expire-able cookie or some such |
| 16:15 |
berick |
or fully deprecate the feature and just ignore the flag |
| 16:18 |
eeevil |
or, only use it where it's useful for isolation purposes. there's an existing comment in Open-ILS/src/eg2/src/app/staff/catalog/record/holdings.component.ts that says it's rechecking, and it's not a high-usage UI, so I've got a patch where that's the only instance left under eg2/src/app/ |
| 16:18 |
berick |
but the definition of "useful" depends on whether you have pooled replicas |
| 16:19 |
eeevil |
my definition of useful is "I want this to be done in an explicit transaction that will not see the effects of other transactions" |
| 16:23 |
eeevil |
RO replicas are just the scenario that presents the largest time window for seeing "inconsistent" reads. they're not the only one that exists, though. but if we never care about /any/ version of that, let's just remove {authoriative:true} from all angular pcrud calls and move on. |
| 16:24 |
berick |
just never heard that use case before. |
| 16:24 |
berick |
well, if we remove them.. that means we are killing the feature |
| 16:24 |
eeevil |
but changing the default behavior is killing /me/ ;) |
| 16:26 |
berick |
i can see not wanting to use localStorage as a long-term solution to this, but what actual problem are you having? |
| 16:26 |
berick |
i'm confused how the introduction of a new method is a problem |
| 16:27 |
berick |
(and fwiw, i'm fully in favor of full feature removal. i'd be pretty surprised if anyone was affected). |
| 16:31 |
eeevil |
I'm not complaining about the introduction of a method, that's silly. |
| 16:40 |
eeevil |
my complaint is about the change existing at all, really. like, if we don't want or need the downsides (performance loss of extra round trips) of using a transaction for its side effects, why not simple remove the /uses/ of authoritative:true. I'm not saying remove the ability, I'm saying, don't ask for it when we don't want it. |
| 16:40 |
eeevil |
*existing on the client side |
| 16:43 |
eeevil |
I won't argue that the way it's used in dispatch() is particularly important to keep, but we don't have to use it, either. |
| 16:48 |
berick |
for sites using pooled RO replicas, though, authoritative use in the client has a tangible value. if we say we're supporting the feature, we can't just remove the uses. |
| 16:55 |
eeevil |
if that's important (and sure, I'd make different choices about the implementation 18 years on), then shouldn't we assume its use rather than, in my case of "new UI to debug, no new perl related to it, just update UI code", simply not work? |
| 17:02 |
eeevil |
as a baseline "don't change defaults, document the ability to change the behavior" kind of thing |
| 17:02 |
|
mmorgan1 left #evergreen |
| 17:05 |
berick |
when has "just update the ui code" ever been an expectation? |
| 17:12 |
eeevil |
as a separation of concerns WRT layers of logic? I mean, I expect that all the time. angular in particular makes it super easy to just swap branches that are temporally right next to each other, and have no relevant user-serving API changes, and have `build --watch` just make the other branch's changes appear in your browser. but, when has changing default behavior ever been an expectation, with the exception security issues? |
| 17:13 |
berick |
but there was a relevant server API change. |
| 17:15 |
eeevil |
I disagree strongly. I see it as a core behavioral change that /also/ relied on a backend API addition that was entirely unrelated to the _function_ of the UI. |
| 17:21 |
eeevil |
and, as it stands in the beta today, it's a core behavioral change that you cannot take back after you've logged in to the staff client, for any give staff computer |
| 17:21 |
|
kmlussier left #evergreen |
| 17:21 |
berick |
relevant, meaning you need it. |
| 17:24 |
berick |
agreed localstorage is not ideal |
| 17:32 |
eeevil |
well, yes, you need it because the /default behavior was changed/. if either 1) the setting was called "hey, turn off authoritative mode" and having ONLY a successful, truthy return value do that or 2) on error assume the same default as from the beginning of time, then I certainly couldn't complain about unexpected changes (I would vote multiple times for (1) fwiw) |
| 17:42 |
* eeevil |
drifts away |
| 17:43 |
berick |
running different front and back-end code is not something you can assume to work. i don't understand how that's an issue. if we want authoritative support to remain on by default, we can do that before the general release. |
| 17:43 |
berick |
happy to help with that |
| 18:07 |
eeevil |
I'm happy to help as well. just to be clear I'm not talking about wanting to run 3.12 staff client on 3.8 backend or anything like that. I'm talking about adjacent dev branches, of the same vintage and focusing on orthogonal UI work with no dev-branch-code relevant db or perl updates, having their default core behavior and installation assumption /diametrically opposite/ in a way that is both irreversible on the client side and functionally breaking, |
| 18:07 |
eeevil |
due to being separated by a single entirely unrelated commit. I argue we should /never/ change a default behavior or configuration requirement, except for security reasons. |
| 18:31 |
sandbergja |
I think we should work toward making authoritative go away. Nobody was relying on it, as far as I've heard, and it had no documentation that I could find until berick added some in his patch. Keeping code around that nobody uses and has no documentation just adds to the extensive maintenance burden and added complexity of onboarding new devs. |
| 18:31 |
sandbergja |
I'd be fine with changing the default to be "on" now, and say remove it in 4.0. |
| 18:32 |
sandbergja |
but I don't agree that we need to keep it around until The End Of Evergreen |
| 18:32 |
sandbergja |
or heck, even change the default to be *on* now, *off* in 4.0, and remove in 5.0 |
| 18:33 |
sandbergja |
If I understood your comment about never changing a default behavior |
| 21:40 |
eeevil |
@later tell sandbergja the corollary to that, though, is that everybody /is/ using it, and tossing out code that the system depends on for proper operation in non-trivial configurations will mean non-trivial configurations of Evergreen will not be possible in the future. my comment about not changing default behavior, especially deep, core behavior that is baked into the very center of the infrastructure layer, is about making sure that we don't |
| 21:40 |
eeevil |
break stuff. In this case, the biggest issue I've spotted so far is that you can't change your mind on the config decision if it turns out there's a bug surfaced by the new setting. that would be a big deal if you're a huge consortium with 2500+ workstations out in the field. that doesn't mean I don't want change, and I'd fully support a better/smarter/slimmer implementation of any infrastructure features. Indeed, I'm very much looking forward to |
| 21:40 |
eeevil |
opensrf on redis when it's truly ready, and I worked with berick on the side early on to help provide feedback and input on big/complicated instance need, but we just added back an absolutely necessary feature to the redis branch that was dropped because it was assumed nobody needed it, because it's not needed in trivial (though common) configurations. As for the authoritative stuff, there are a few things getting mixed in there together (the apps |
| 21:40 |
eeevil |
that talk to the DB, the perl "magic" in a few different places that helps the DB apps, and separately, the client side pcrud service -- they're all doing slightly different things for different audiences). However, I suspect (and hope!) that as postgres replicas become increasingly easier to build and maintain and Evergreen instances get bigger (broader/wider), we will see a need again for that capability. we don't today because SSDs and networks |
| 21:40 |
eeevil |
got fast and widespread enough, and CPU core count went up enough, that horizontal scaling isn't as necessary as in the past, but I'd bet it'll swing back the other way if Evergreen lasts long enough. ... I know my frustration with stumbling on this silent feature-breaking commit (and, yes, it literally breaks a capability by default, even in the case where you install fresh) is bleeding through, and I'm sorry to the channel for that, but my goal is |
| 21:40 |
pinesol |
eeevil: The operation succeeded. |
| 21:40 |
eeevil |
to improve the whole system, and I have been consistent from the beginning of the project that, while we can change implementation (and we do, pretty often) we should not and cannot change default behavior without a REALLY good (read: security) reason, or a REALLY long runway. **deep breath** thanks, all, and have a good evening! |
