IRC logs for #buildstream for Monday, 2019-11-18

*** tpollard has joined #buildstream09:04
*** phildawson has joined #buildstream09:52
*** phildawson_ has joined #buildstream10:01
*** phildawson has quit IRC10:02
*** bochecha has joined #buildstream10:04
*** jonathanmaw has joined #buildstream10:29
*** lachlan has joined #buildstream10:36
*** lachlan has quit IRC10:51
*** phildawson_ has quit IRC10:58
*** lachlan has joined #buildstream10:58
*** phildawson_ has joined #buildstream10:59
*** lachlan has quit IRC11:13
*** lachlan has joined #buildstream11:18
*** jonathanmaw_ has joined #buildstream11:21
*** jonathanmaw has quit IRC11:22
*** tpollard has quit IRC11:22
*** akvilebirgelyte_ has quit IRC11:22
*** ikerperez has quit IRC11:22
*** qinusty has quit IRC11:22
*** adds68 has quit IRC11:22
*** laurence has quit IRC11:22
*** lantw44 has quit IRC11:22
*** SotK has quit IRC11:22
*** hergertme has quit IRC11:22
*** Trevinho has quit IRC11:22
*** bochecha has quit IRC11:22
*** paulsherwood has quit IRC11:22
*** milloni has quit IRC11:22
*** gitlab-br-bot has quit IRC11:22
*** bochecha has joined #buildstream11:22
*** paulsherwood has joined #buildstream11:22
*** milloni has joined #buildstream11:22
*** gitlab-br-bot has joined #buildstream11:22
*** tpollard has joined #buildstream11:22
*** akvilebirgelyte_ has joined #buildstream11:23
*** laurence has joined #buildstream11:25
*** SotK has joined #buildstream11:25
*** qinusty has joined #buildstream11:25
*** ikerperez has joined #buildstream11:25
*** Trevinho has joined #buildstream11:25
*** adds68 has joined #buildstream11:25
*** lantw44 has joined #buildstream11:26
*** hergertme has joined #buildstream11:26
*** lachlan has quit IRC11:31
tlater[m]benschubert: I'd like to get !1660 done today - mind looking over the remaining discussions?11:36
gitlab-br-botMR !1660: Remove update_state https://gitlab.com/BuildStream/buildstream/merge_requests/166011:36
benschubertOups, sure, doing it right now11:37
*** lachlan has joined #buildstream11:39
benschuberttlater[m]: done11:42
tlater[m]ta!11:42
*** phoenix has joined #buildstream11:49
tlater[m]benschubert: I dislike the idea of forcing people to call `_can_schedule()` manually first11:52
tlater[m]It means you need to know that you need to call that ahead of time11:53
benschuberttlater[m]: isn't it called only in "Element" ?11:53
tlater[m]And you never want to call `_schedule_assemble()` when you can't...11:53
tlater[m]Yeah, it's private API, but I still don't think it's a good idea11:53
* tlater[m] would be happier with something like `_schedule_assemble_when_necessary()` or somesuch11:55
tlater[m]Maybe return `True/False` when we were able to schedule and document that?11:55
benschubertmy problem with that is that it's a method that is called a lot and I believe by reducing the number of calls we would gain some time. (might need to benchmark)11:55
tlater[m]Right, but by adding a function call in an `if` statement doesn't change the number of function calls much11:56
tlater[m]It's only 2 -> 1 if we don't want to schedule11:56
benschubertbut it's part of the systemic problem of why this is a bottleneck :)11:57
benschubertOk, let's keep it like that for now (I like the name you just gave better than the old one)11:57
tlater[m]Ok, I'll also leave a FIXME so we remember to benchmark later when we're looking at more optimizations ;)11:58
benschubertthanks!11:58
*** narispo has quit IRC12:26
*** narispo has joined #buildstream12:26
gitlab-br-botBenjaminSchubert approved MR !1714 (chandan/re-enable-import-check->master: plugins/sources/deb: Re-enable import check for arpy) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/171412:28
*** narispo has quit IRC12:32
gitlab-br-botmarge-bot123 merged MR !1714 (chandan/re-enable-import-check->master: plugins/sources/deb: Re-enable import check for arpy) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/171412:32
*** narispo has joined #buildstream12:32
tlater[m]Sorry, just noticed, benschubert: https://gitlab.com/BuildStream/buildstream/merge_requests/1660#note_24605860712:39
* tlater[m] looked at the results, timings seem about equal12:39
tlater[m]Although I'm not sure that benchmarking suite does kernel warmup, so it shows pretty inconsistent results for `bst show`12:39
*** toscalix has joined #buildstream12:57
*** lachlan has quit IRC13:05
*** narispo has quit IRC13:25
*** narispo has joined #buildstream13:25
*** phoenix has quit IRC13:40
tlater[m]benschubert, cs-shadow - before this ends up falling prey to review latency again, anything else outstanding on !1660?14:05
gitlab-br-botMR !1660: Remove update_state https://gitlab.com/BuildStream/buildstream/merge_requests/166014:05
*** lachlan has joined #buildstream14:26
*** lachlan has quit IRC14:33
*** lachlan has joined #buildstream14:53
*** lachlan has quit IRC15:05
*** bochecha has quit IRC15:09
*** bochecha has joined #buildstream15:09
*** bochecha has joined #buildstream15:10
benschuberttlater[m]: one last commeont on my side15:22
tlater[m]benschubert: I don't quite understand it: https://gitlab.com/BuildStream/buildstream/merge_requests/1660#note_24616798515:26
benschuberttlater[m]: ah didn't see it was a verbatime copy15:26
benschubertI just don't see why it doesn't explain when it changes/doesn't change15:26
tlater[m]Right, still want me to change it?15:26
benschubertinstead of saying 'doesn't necessarily'15:26
benschubertSince that touches a lot, it might be better :)15:27
tlater[m]Ah, fair enough15:27
benschubertSpecifying that it doesn't changes for runtime deps only build deps or something like that?15:27
tlater[m]Is it that way around?15:28
tlater[m]Yes15:28
tlater[m]Ok, sure, I'll amend the comment15:28
benschubertthanks :)15:29
tlater[m]benschubert: Think the new comment is clearer?15:37
cs-shadowtlater[m]: i haven't had the chance to go through it thoroughly myself, but seems good to me from what I did look at15:38
tlater[m]cs-shadow: Ta - just making sure; there's been plenty of thorough review from others :)15:38
benschuberttlater[m]: one last comment :) otherwise LGTM15:39
tlater[m]I'll take that as approval to marge then :D15:41
benschubertonce you fix the last comment ;)15:42
tlater[m]Yeah :)15:45
* tlater[m] is looking forward to not having to rebase against that branch anymore \o/15:46
tlater[m]Currently, if we get an exception from a plugin running `get_consistency` at load time we'll stacktrace.15:51
tlater[m]There's a FIXME in one of the tests saying that we should probably use a BUG message15:51
tlater[m]But I wonder if that should just be a normal PluginError resulting in a `FAILED` message15:51
tlater[m]Because it's not a bug in BuildStream - it's the plugin causing problems.15:52
cs-shadowI think making it BUG made sense if we think that buildstream is the only provider of plugins; in which case it's a buildstream bug. But since that's not the case, I'd probably be in favor of a FAILED message instead15:54
tpollardFAILED for sure, although getting it to be a handled failure might be tricky15:55
tlater[m]tpollard: Not as bad in the `fetch` case so far15:56
tlater[m]Although there are some situations in which we call `get_consistency` that will still cause stacktraces even if I fix this.15:57
tlater[m]The *real* fix would be to wrap `get_consistency()`, but that's public API...15:57
*** slaf has quit IRC16:00
*** slaf has joined #buildstream16:00
*** slaf has joined #buildstream16:00
*** slaf has joined #buildstream16:01
*** slaf has joined #buildstream16:01
*** slaf has joined #buildstream16:01
*** slaf has joined #buildstream16:01
*** slaf has joined #buildstream16:02
*** slaf has joined #buildstream16:02
*** slaf has joined #buildstream16:02
*** slaf has joined #buildstream16:02
*** slaf has joined #buildstream16:03
*** slaf has joined #buildstream16:03
*** slaf has joined #buildstream16:03
*** slaf has joined #buildstream16:03
*** lachlan has joined #buildstream16:04
benschuberttlater[m]: I'm planning in removing `get_consistency` anyways. so the problem will go away for that16:43
tlater[m]That'd be nice!16:43
benschubertwell, more required given how long it takes in some cases where we don't even need it :'D16:44
benschubertit's currently 2/3 of our 'bst show' time16:44
tlater[m]Oh wow, that's interesting16:45
*** bochecha has quit IRC16:50
*** toscalix has quit IRC16:57
benschuberttlater[m]: just bumped your debian build which failed17:00
tlater[m]benschubert: ta!17:02
benschubertshould it go back to marge?17:03
* tlater[m] takes a look at the failure...17:06
tlater[m]> 3328 worker 'gw1' crashed while running 'tests/integration/cachedfail.py::test_push_cached_fail[quit]'17:07
*** lachlan has quit IRC17:07
tlater[m]Yeah, I don't think that has anything to do with my branch17:07
*** lachlan has joined #buildstream17:16
gitlab-br-botmarge-bot123 closed issue #1054 (Remove the concept of update_state) on buildstream https://gitlab.com/BuildStream/buildstream/issues/105417:27
gitlab-br-botmarge-bot123 merged MR !1660 (tlater/annihilate_update_state->master: Remove update_state) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166017:27
tlater[m]\o/17:28
*** lachlan has quit IRC18:10
*** lachlan has joined #buildstream18:16
*** mohan43u has quit IRC18:33
*** lachlan has quit IRC18:43
*** phildawson_ has quit IRC18:44
*** jonathanmaw_ has quit IRC18:49
*** lachlan has joined #buildstream19:10
*** lachlan has quit IRC20:49
gitlab-br-botcs-shadow opened MR !1716 (chandan/glossary->master: doc: Add glossary of common terms) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/171623:18
*** phoenix has joined #buildstream23:23
*** phoenix has quit IRC23:33

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!