IRC logs for #buildstream for Tuesday, 2019-10-22

*** akvilebirgelyte has joined #buildstream07:37
*** traveltissues has joined #buildstream07:41
*** santi has joined #buildstream08:41
*** tiagogomes has joined #buildstream08:42
*** bochecha has joined #buildstream08:55
*** benschubert has joined #buildstream09:10
benschubertovernight tests trying to pull from freedesktop caches: https://gitlab.com/BuildStream/buildstream/-/jobs/327518918 shouldn't this cache be skipped because it does not have the right capabilities??09:11
tpollardyes09:11
benschubertgah, seems it's broken09:13
benschubertDidn't we have any tests around this?09:13
tlater[m]I've not seen any, but I don't have an extensive view of the whole test suite09:15
benschubertok :/ I'll open an issue09:16
*** santi has quit IRC09:16
tpollardhttps://gitlab.com/BuildStream/buildstream/issues/102509:16
tpollardsorry https://gitlab.com/BuildStream/buildstream/merge_requests/136609:16
benschubertyep that was about the update and we used to be able to not fail :)09:17
*** santi has joined #buildstream09:18
gitlab-br-botBenjaminSchubert opened issue #1178 (BuildStream doesn't check that the artifact remotes has the required capabilities) on buildstream https://gitlab.com/BuildStream/buildstream/issues/117809:19
*** santi has quit IRC09:21
*** santi has joined #buildstream09:21
*** jonathanmaw has joined #buildstream09:22
gitlab-br-botaevri opened (was WIP) MR !1655 (aevri/enable_spawn_ci_4->master: cascache: don't pickle _cache_usage_monitor) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/165509:34
gitlab-br-bottraveltissues approved MR !1655 (aevri/enable_spawn_ci_4->master: cascache: don't pickle _cache_usage_monitor) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/165509:35
*** lachlan has joined #buildstream09:36
gitlab-br-bottlater opened MR !1660 (tlater/annihilate_update_state->master: Remove update_state) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166009:41
tpollardhaha nice branch name09:42
juergbiaevri: oh, just saw !1655, I don't think this is enough09:43
juergbiaevri: I've tried to explain it here https://gitlab.com/BuildStream/buildstream/merge_requests/1659#note_23377046609:44
juergbitraveltissues: or do you think !1655 is enough to fix the !1659 issue?09:45
gitlab-br-botMR !1659: WIP: create usage monitor early https://gitlab.com/BuildStream/buildstream/merge_requests/165909:45
juergbiI'd think it's not enough but I might be missing something09:46
traveltissuesit does not seem to be09:46
juergbiok, so let's not merge it yet09:46
traveltissuesyes, it's worth doing but i'd agree that we could just disable it for spawn09:49
juergbias it still breaks in certain tests, let's completely disable it until we find a better solution09:49
juergbithe getstate change is not needed if we completely disable it, afaict, so let's skip the noise09:50
tlater[m]tpollard: :D09:50
benschubertjuergbi: could we run the tests with spawn instead of fork when running the spawn tests?09:51
benschubertthat wouls also fix it right?09:51
juergbibenschubert: it might, yes09:51
tlater[m]Gah, why does gitlab not add WIP to my title when all my commits start with `WIP` :(09:52
juergbiwe might actually be able to get away without forked tests now09:52
benschuberttlater[m]: create fixups commits :)09:52
tlater[m]benschubert: I always just end up squashing those before I push since it's less keystrokes.09:55
* tlater[m] should probably stop running with encrypted ssh keys.09:55
aevrijuergbi: I've un-marged !1655 since you raise the alarm :)10:00
aevriI think 'not enough' depends on context though10:00
gitlab-br-botMR !1655: cascache: don't pickle _cache_usage_monitor https://gitlab.com/BuildStream/buildstream/merge_requests/165510:00
juergbinot enough for !1659 which is blocked by the spawn issue10:00
gitlab-br-botMR !1659: WIP: create usage monitor early https://gitlab.com/BuildStream/buildstream/merge_requests/165910:00
juergbino point to merge it if we anyway urgently need a different approach10:01
aevriIn the context of my merge request, it does enable extra tests to pass10:01
juergbiI understand but !1659 is a bugfix we want to merge as quickly as possible10:01
*** phildawson_ has joined #buildstream10:02
aevridoes 1655 hurt that in some way?10:02
juergbinot directly but it would likely conflict with the spawn fix we need there10:02
aevriI see10:03
juergbiand if we have to disable cache usage monitoring for spawn for now, the getstate changes in 1655 are not needed10:03
aevriok sure10:03
*** phildawson has quit IRC10:03
aevriI'll wip it again then10:03
juergbiaevri: have you read my comment on !1659? what's the best way for this 'spawn' conditional?10:04
aevriLet me see10:04
juergbior do you have an alternative suggestion off the top of your head?10:05
juergbita10:05
juergbimaybe simply multiprocessing.get_start_method() == 'spawn'10:06
aevrithat would do it10:06
juergbiok, ta, let's go with this for now to unblock this MR10:07
aevriiiuc, this is simply a problem with the tests causing a fork; `bst` itself would be fine right?10:07
traveltissuesyes10:08
traveltissuesafaict10:08
aevriI'm wondering if we can do `mark.run_in_subprocess` differently10:08
aevriPlaying locally, I didn't seem to need it in `spawn` mode, but I'm not sure what the orginal problem is10:08
juergbiaevri: we might actually not need it at all anymore10:10
juergbioriginally it was to avoid issues between fork (job processes) and gRPC / background threads10:10
juergbiwhich shouldn't be an issue with 'spawn' anyway10:10
aevriI'm waiting on https://gitlab.com/BuildStream/buildstream/-/jobs/327970614 to see if we get tell-tale grpc errors10:10
aevriok cool10:11
juergbihowever, we should now properly clean up after gRPC and might not even need it anymore with regular 'fork'10:11
juergbiso that could also be an option, although I'm not sure whether we should do it in the same MR10:11
juergbirather want to get this merged and handle this in a separate MR to avoid delaying this for much longer10:12
aevrisure10:12
aevriI can take on the `run_in_subprocess` thing if there's a rush on the other10:12
aevriI'll do it separately10:12
traveltissueshttps://gitlab.com/BuildStream/buildstream/pipelines/9051814710:12
traveltissuesyikes10:13
juergbiaevri: sounds good to me. however, I don't think it's terribly urgent10:14
juergbithe 'run in subprocess' thing, I mean10:14
aevriSure, that's just more on my agenda of fixing up the spawn case10:14
juergbiunless our workaround doesn't work ;)10:14
aevriHehe :)10:14
traveltissuesjuergbi, see the above pipeline please10:15
traveltissuesaevri, ^10:15
juergbiexactly but I don't see the error message yet10:15
aevriIf this is so urgent, I'm ok with a regression on the internals tests in the spawn case - we can fix that after, I'm the only customer for that atm10:16
traveltissuesah, nvm10:17
juergbishould be easy to fix up10:17
traveltissuesyes10:17
aevriok10:17
gitlab-br-botsantigl closed issue #898 (Remote Execution: Implement the REAPI Platform message) on buildstream https://gitlab.com/BuildStream/buildstream/issues/89810:18
aevriSmall yay - based on the test run, I'm optimistic we can drop `run_in_subprocess` for the `spawn` case at least.10:21
*** narispo has quit IRC10:29
*** narispo has joined #buildstream10:29
*** akvilebirgelyte has quit IRC10:29
*** akvilebirgelyte has joined #buildstream10:30
gitlab-br-bottraveltissues opened (was WIP) MR !1659 (traveltissues/1176->master: create usage monitor early) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/165910:31
*** phildawson_ has quit IRC10:33
gitlab-br-botBenjaminSchubert approved MR !1659 (traveltissues/1176->master: create usage monitor early) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/165910:33
traveltissuesty10:35
gitlab-br-botcs-shadow opened MR !1662 (chandan/register-pytest-mark->master: setup.cfg: Register mark for pytest-datafiles) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166210:56
*** lachlan has quit IRC11:02
*** phildawson_ has joined #buildstream11:03
gitlab-br-bottraveltissues approved MR !1662 (chandan/register-pytest-mark->master: setup.cfg: Register mark for pytest-datafiles) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166211:03
gitlab-br-botmarge-bot123 closed issue #1176 (Interactive bst operations fail with open workspaces) on buildstream https://gitlab.com/BuildStream/buildstream/issues/117611:17
gitlab-br-botmarge-bot123 merged MR !1659 (traveltissues/1176->master: create usage monitor early) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/165911:17
traveltissueswhat's happening with !1549 btw11:18
gitlab-br-botMR !1549: plugins/elements/cmake.yaml: always specify variable types https://gitlab.com/BuildStream/buildstream/merge_requests/154911:18
*** phildawson_ has quit IRC12:01
*** lachlan has joined #buildstream12:04
gitlab-br-botmarge-bot123 merged MR !1662 (chandan/register-pytest-mark->master: setup.cfg: Register mark for pytest-datafiles) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166212:12
*** santi has quit IRC12:16
*** phildawson_ has joined #buildstream12:57
*** phil has joined #buildstream13:08
*** phildawson_ has quit IRC13:09
*** santi has joined #buildstream13:17
gitlab-br-botaevri opened (was WIP) MR !1661 (aevri/no_mark_run_in_subprocess->master: tests: remove mark.in_subprocess, create_cas_usage_monitor) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166113:31
aevri^ bye bye `mark.in_subprocess` :)13:31
tlater[m]aevri: But we only added it like 2 weeks ago!13:39
aevriwhoosh, things are moving fast :D13:42
benschubertaevri: is that correct? Don't we want it for some tests that do actually change the global state?13:45
aevriiiuc we only needed it for working around grpc background threads not being cleaned up - if anyone knows better please say13:45
aevriThis is what I'm going by: https://gitlab.com/BuildStream/buildstream/commit/0ca7169a1fe4dfedb3524db35387e900bd4f4b10#626af34e8204774ea46e20955936146dedc828eb_70_7513:46
aevrihttps://www.irccloud.com/pastebin/JqPwNNnO/13:47
aevri^ doh, my IRC web client made that pastebin for me, apologies.13:48
benschubertok thanks!13:48
juergbiaevri: thanks for the quick MR, lgtm14:08
juergbicommented with one nit14:08
aevriThanks!14:14
gitlab-br-botcs-shadow opened issue #1179 (tests/sources/keytest.py is buggy) on buildstream https://gitlab.com/BuildStream/buildstream/issues/117914:32
gitlab-br-botcs-shadow opened MR !1664 (chandan/fix-source-key-test->master: tests/sources/keytest: Ensure element state is updated as expected) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166414:45
*** phoenix has joined #buildstream14:46
*** phoenix has quit IRC14:57
gitlab-br-bottpollard approved MR !1664 (chandan/fix-source-key-test->master: tests/sources/keytest: Ensure element state is updated as expected) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166414:58
gitlab-br-botmarge-bot123 merged MR !1661 (aevri/no_mark_run_in_subprocess->master: tests: remove mark.in_subprocess, create_cas_usage_monitor) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166115:03
*** traveltissues has quit IRC15:14
*** santi has quit IRC15:43
*** bochecha has quit IRC15:51
*** akvilebirgelyte has quit IRC15:59
*** santi has joined #buildstream16:03
*** narispo has quit IRC16:29
*** narispo has joined #buildstream16:29
*** narispo has quit IRC16:53
*** narispo has joined #buildstream16:53
*** narispo has quit IRC16:57
*** jonathanmaw has quit IRC17:00
*** santi has quit IRC17:03
*** tiagogomes has quit IRC17:08
*** lachlan has quit IRC17:11
*** cs-shadow has quit IRC20:56
gitlab-br-botmarge-bot123 merged MR !1664 (chandan/fix-source-key-test->master: tests/sources/keytest: Ensure element state is updated as expected) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166421:12
gitlab-br-botmarge-bot123 closed issue #1179 (tests/sources/keytest.py is buggy) on buildstream https://gitlab.com/BuildStream/buildstream/issues/117921:12
gitlab-br-botmostynb opened MR !1665 (link_fix->master: fix broken user guide link in README.rst) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/166521:51
*** narispo has joined #buildstream22:28

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