jam | seems quiet... too quiet | 07:58 |
---|---|---|
niemeyer | Must be because everyone is isolated in their homes.. | 07:59 |
niemeyer | Wait.. | 07:59 |
niemeyer | ;) | 07:59 |
niemeyer | Morning | 07:59 |
jam | morning | 07:59 |
mup_ | PR operator#226 opened: ops/framework.py: StoredStateChanged includes the attribute <Created by jameinel> <https://github.com/canonical/operator/pull/226> | 08:00 |
niemeyer | jam: I wonder if we should do that ^ or if we should hide the event altogether | 08:02 |
niemeyer | jam: This was part of the original implementation.. back then everything would fire changed | 08:03 |
niemeyer | jam: Then we moved to only snapshoting once for performance | 08:03 |
jam | niemeyer, I could see going either way. We've generally thought that _stored should be private, which means you shouldn't be listening for changes on it. | 08:04 |
jam | if we are going to have changes, then they should be useful | 08:04 |
jam | and right now it only emits if the top level attribute changes | 08:04 |
jam | so either remove it entirely, or make it correct. | 08:04 |
jam | the above makes it correct, but if we want to say 'don't do it' then we can do that easily enough. :) | 08:04 |
niemeyer | jam: Agreed | 08:04 |
niemeyer | jam: Is that attribute only the tip if there are multiple nested dicts? | 08:05 |
jam | niemeyer, so I considered a couple different things, but went with top level attribute to start | 08:09 |
jam | otherwise you end up with some sort of path, which might include lists, dicts, etc. | 08:09 |
jam | and the attribute of the dict might be a string or an int or | 08:09 |
niemeyer | jam: Agreed.. we'd have to build some sort of dotted notation | 08:10 |
niemeyer | foo[3].bar | 08:10 |
jam | yeah. | 08:10 |
jam | I didn't want to deal with paths yet, when the top level attribute is at least a good enough hint. | 08:10 |
niemeyer | I suggest just killing it for now, making it private if we still require it for the internal implementation to some degree | 08:10 |
niemeyer | We can ask to see what's the use case before we axe it | 08:11 |
niemeyer | Gut feeling is that this shouldn't be important, and might even lead to brittle code as this isn't precise enough | 08:11 |
jam | niemeyer, just ripping out the code no tests fail... | 08:19 |
niemeyer | +1 to that | 08:19 |
niemeyer | I'll copy & paste this conversation into the issue | 08:19 |
Chipaca | mo'in all! | 08:21 |
jam | morning Chipaca | 08:22 |
mup_ | PR operator#227 opened: ops/framework.py: remove StoredStateChanged <Created by jameinel> <https://github.com/canonical/operator/pull/227> | 08:23 |
jam | team policy decision on issue #225, do we fix it (PR #226) or get rid of it (PR #227) | 08:23 |
mup_ | Issue #225: BoundSharedState changed events is misnamed (or misleading) <Created by addyess> <https://github.com/canonical/operator/issues/225> | 08:23 |
mup_ | PR #226: ops/framework.py: StoredStateChanged includes the attribute <Created by jameinel> <https://github.com/canonical/operator/pull/226> | 08:23 |
mup_ | PR #227: ops/framework.py: remove StoredStateChanged <Created by jameinel> <https://github.com/canonical/operator/pull/227> | 08:23 |
niemeyer | jam: Given that theory, couldn't we get rid of all the sub-type magic we have there? | 08:25 |
niemeyer | jam: I mean, we introduced these StoredLists, etc, precisely to be able to handle the subevents | 08:25 |
jam | niemeyer, we still need dirty | 08:25 |
niemeyer | Ah, indeed | 08:25 |
niemeyer | Otherwise we'll always snapshot, okay | 08:25 |
niemeyer | jam: Here is my vote: https://github.com/canonical/operator/issues/225#issuecomment-614496681 | 08:28 |
jam | thx | 08:28 |
* Chipaca runs tests → wonders about coverage → looks into coverage → weeps → drags self back to work at hand → runs tests | 10:00 | |
Chipaca | to be fair, it's just main.py that has dreadful coverage, the others aren't too bad | 10:02 |
Chipaca | but i was working on main.py | 10:02 |
niemeyer | I don't think the coverage tests will handle that one correctly | 10:03 |
niemeyer | By definition the best testing for it needs to fire it externally | 10:04 |
niemeyer | That external process won't be accounted for coverage | 10:04 |
Chipaca | yes, and things like _get_charm_dir should be covered by unit tests | 10:12 |
Chipaca | it has two branches, one of them simple, one of them suspicious | 10:13 |
Chipaca | i replaced the suspicious one with "raise RuntimeError" and everything was happy | 10:13 |
Chipaca | i mean, main.py's main, sure, hard to unit-test, no problem. but that file is almost all helpers, almost all uncovered (without looking beyond the 20% coverage it has) | 10:15 |
Chipaca | ANYhow | 10:15 |
* Chipaca drags self back to work at hand again | 10:16 | |
mup_ | Issue operator#228 opened: Adopt a standard and well integrated templating module <Created by niemeyer> <https://github.com/canonical/operator/issues/228> | 10:21 |
mup_ | Issue operator#229 opened: The pod spec API must be improved <Created by niemeyer> <https://github.com/canonical/operator/issues/229> | 10:32 |
mup_ | Issue operator#113 closed: Charms as YAML <question> <Created by knkski> <Closed by niemeyer> <https://github.com/canonical/operator/issues/113> | 10:37 |
niemeyer | Chipaca: If you raise an exception and it does nothing, that's obviously uncovered.. but the "20%" number means nothing if our tests are exercising logic in this file via an independent process | 10:39 |
niemeyer | Which I think they are | 10:39 |
niemeyer | Sorry if that was clear before and I'm just repeating myself.. it just felt like a coverage number was still being taken as correct | 10:40 |
niemeyer | We have that charms/test_main charm that is trying to make the check as realistic as reasonable.. when we do that externally, none of the lines touched by the test will get into the report | 10:42 |
Chipaca | I'll look into this after | 10:42 |
* Chipaca steps out for a bit | 10:56 | |
niemeyer | Lunch | 11:01 |
facubatista | Muy buenos días a todos! | 11:09 |
facubatista | jam, if you can land #227, that means #226 could be just closed, right? | 11:28 |
mup_ | PR #227: ops/framework.py: remove StoredStateChanged <Created by jameinel> <https://github.com/canonical/operator/pull/227> | 11:28 |
mup_ | PR #226: ops/framework.py: StoredStateChanged includes the attribute <Created by jameinel> <https://github.com/canonical/operator/pull/226> | 11:28 |
jam | If we decide to do 227 then it replaces 226, correct | 11:29 |
mup_ | PR operator#227 closed: ops/framework.py: remove StoredStateChanged <Created by jameinel> <Merged by jameinel> <https://github.com/canonical/operator/pull/227> | 11:31 |
mup_ | PR operator#226 closed: ops/framework.py: StoredStateChanged includes the attribute <Created by jameinel> <Closed by jameinel> <https://github.com/canonical/operator/pull/226> | 11:32 |
mup_ | Issue operator#225 closed: BoundSharedState changed events is misnamed (or misleading) <Created by addyess> <Closed by addyess> <https://github.com/canonical/operator/issues/225> | 11:41 |
mup_ | PR operator#230 opened: Testing harness no events for own data <Created by jameinel> <https://github.com/canonical/operator/pull/230> | 11:51 |
Chipaca | facubatista: buen día señor! | 13:00 |
facubatista | hola Chipaca | 13:15 |
jam | Chipaca I'm assuming its ok if I miss the framework sync (in 45 min). or would you like me to be there? | 13:50 |
Chipaca | jam: indeed | 13:50 |
Chipaca | jam: if they wanted you in it it'd be at a reasonable time for you :-) | 13:51 |
=== mup__ is now known as mup_ | ||
* facubatista had(has) a car issue but is back | 14:32 | |
niemeyer | Ouch :( | 14:36 |
Chipaca | brb, reboot | 14:56 |
Chipaca | wooo! hello from my zippy laptop again! | 15:08 |
Chipaca | looking frankenstein-y right now so requires another reboot, and here's hoping it's still zippy after that | 15:08 |
* Chipaca brb rebooting again | 15:08 | |
facubatista | Chipaca, how did you fix it? | 15:26 |
Chipaca | I don't think it's fixed, but it works for now | 15:27 |
facubatista | Chipaca, just improved suddenly or you did something? | 15:27 |
Chipaca | facubatista: unplugged the battery | 15:27 |
Chipaca | and turned it on from just ac | 15:27 |
Chipaca | then power it off again, plug the battery back in | 15:28 |
Chipaca | getting at the battery is ~20 screws but hey, it worked | 15:28 |
Chipaca | facubatista: but i'm not going to do this every 20 days :-( so it's not "fixed", but i might do this every 20 days until the Q is over | 15:28 |
facubatista | Chipaca, if it serves of consolation, battery is also involved in my issue :p | 15:29 |
Chipaca | facubatista: hehe | 15:29 |
Chipaca | facubatista: batteries are the worst, except for not having batteries | 15:29 |
facubatista | wife took the car and went to the greengrocer's, got back, parked outside to clean some leaves from the garage, and then the car just didn't start anymore | 15:31 |
niemeyer | GOod place to get stuck | 15:31 |
facubatista | lucky us that it didn't die at the greengrocer's (which is ~10 blocks away, not much, but would complicate a lot the operational part of waiting some service or replacement) | 15:31 |
facubatista | niemeyer, exactly | 15:32 |
facubatista | of course, I can not just go and by a new battery, this kind of shop is closed | 15:32 |
Chipaca | facubatista: something something condensation | 15:33 |
facubatista | Chipaca, what? | 15:34 |
Chipaca | facubatista: sorry, that was a bit obscure of me. A few years ago somebody asked in a forum somewhere, something like (from memory), "dear $forum, I don't know what to do! I nipped out to the grocer's this morning and came back, and then when I went to go to work my car didn't start, so I came back into the house and my partner was in bed with another person!" and they answered "probably condensation in your $engine_part, try blowing it dry" | 15:36 |
facubatista | jejej | 15:37 |
Chipaca | and the discussion was about how doing the short prior trip lead to the condensation | 15:37 |
* Chipaca steps away for a break before the last meeting of the day | 17:06 | |
facubatista | niemeyer, https://www.pagina12.com.ar/259910-las-claves-del-modelo-portugues-frente-al-coronavirus | 17:26 |
niemeyer | Yeah, the story is pretty good so far.. now to see how the second half of the story goes | 17:31 |
niemeyer | It's not true that under 15 haven't returned to school, btw... at least not universally true.. our son has been doing doing remote schooling for a couple of weeks now | 17:32 |
niemeyer | This week they started doing video calls with the whole class | 17:34 |
* facubatista -> car support | 17:38 | |
facubatista | niemeyer, my kids are also with virtual classrooms and group video calls | 17:38 |
mup_ | Issue operator#231 opened: Harness: Has errors on charms which define action handlers <Created by Vultaire> <https://github.com/canonical/operator/issues/231> | 18:04 |
* facubatista is back | 18:17 | |
facubatista | new battery, ~USD 120 | 18:18 |
Chipaca | brb, rebooting | 18:41 |
mup_ | PR operator#232 opened: ops.testing.Harness actions metadata support. (#231) <Created by Vultaire> <https://github.com/canonical/operator/pull/232> | 18:54 |
mup_ | PR operator#233 opened: Execute the registered methods to events under PDB if proper envvar is set <Created by facundobatista> <https://github.com/canonical/operator/pull/233> | 21:10 |
* facubatista opened ^, but we need to talk around it | 21:10 | |
facubatista | which will happen tomorrow | 21:10 |
* facubatista eods | 21:10 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!