*** akvilebirgelyte has joined #buildstream | 07:37 | |
*** traveltissues has joined #buildstream | 07:41 | |
*** santi has joined #buildstream | 08:41 | |
*** tiagogomes has joined #buildstream | 08:42 | |
*** bochecha has joined #buildstream | 08:55 | |
*** benschubert has joined #buildstream | 09:10 | |
benschubert | overnight 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 |
---|---|---|
tpollard | yes | 09:11 |
benschubert | gah, seems it's broken | 09:13 |
benschubert | Didn'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 suite | 09:15 |
benschubert | ok :/ I'll open an issue | 09:16 |
*** santi has quit IRC | 09:16 | |
tpollard | https://gitlab.com/BuildStream/buildstream/issues/1025 | 09:16 |
tpollard | sorry https://gitlab.com/BuildStream/buildstream/merge_requests/1366 | 09:16 |
benschubert | yep that was about the update and we used to be able to not fail :) | 09:17 |
*** santi has joined #buildstream | 09:18 | |
gitlab-br-bot | BenjaminSchubert opened issue #1178 (BuildStream doesn't check that the artifact remotes has the required capabilities) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1178 | 09:19 |
*** santi has quit IRC | 09:21 | |
*** santi has joined #buildstream | 09:21 | |
*** jonathanmaw has joined #buildstream | 09:22 | |
gitlab-br-bot | aevri 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/1655 | 09:34 |
gitlab-br-bot | traveltissues 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/1655 | 09:35 |
*** lachlan has joined #buildstream | 09:36 | |
gitlab-br-bot | tlater opened MR !1660 (tlater/annihilate_update_state->master: Remove update_state) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1660 | 09:41 |
tpollard | haha nice branch name | 09:42 |
juergbi | aevri: oh, just saw !1655, I don't think this is enough | 09:43 |
juergbi | aevri: I've tried to explain it here https://gitlab.com/BuildStream/buildstream/merge_requests/1659#note_233770466 | 09:44 |
juergbi | traveltissues: or do you think !1655 is enough to fix the !1659 issue? | 09:45 |
gitlab-br-bot | MR !1659: WIP: create usage monitor early https://gitlab.com/BuildStream/buildstream/merge_requests/1659 | 09:45 |
juergbi | I'd think it's not enough but I might be missing something | 09:46 |
traveltissues | it does not seem to be | 09:46 |
juergbi | ok, so let's not merge it yet | 09:46 |
traveltissues | yes, it's worth doing but i'd agree that we could just disable it for spawn | 09:49 |
juergbi | as it still breaks in certain tests, let's completely disable it until we find a better solution | 09:49 |
juergbi | the getstate change is not needed if we completely disable it, afaict, so let's skip the noise | 09:50 |
tlater[m] | tpollard: :D | 09:50 |
benschubert | juergbi: could we run the tests with spawn instead of fork when running the spawn tests? | 09:51 |
benschubert | that wouls also fix it right? | 09:51 |
juergbi | benschubert: it might, yes | 09:51 |
tlater[m] | Gah, why does gitlab not add WIP to my title when all my commits start with `WIP` :( | 09:52 |
juergbi | we might actually be able to get away without forked tests now | 09:52 |
benschubert | tlater[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 | |
aevri | juergbi: I've un-marged !1655 since you raise the alarm :) | 10:00 |
aevri | I think 'not enough' depends on context though | 10:00 |
gitlab-br-bot | MR !1655: cascache: don't pickle _cache_usage_monitor https://gitlab.com/BuildStream/buildstream/merge_requests/1655 | 10:00 |
juergbi | not enough for !1659 which is blocked by the spawn issue | 10:00 |
gitlab-br-bot | MR !1659: WIP: create usage monitor early https://gitlab.com/BuildStream/buildstream/merge_requests/1659 | 10:00 |
juergbi | no point to merge it if we anyway urgently need a different approach | 10:01 |
aevri | In the context of my merge request, it does enable extra tests to pass | 10:01 |
juergbi | I understand but !1659 is a bugfix we want to merge as quickly as possible | 10:01 |
*** phildawson_ has joined #buildstream | 10:02 | |
aevri | does 1655 hurt that in some way? | 10:02 |
juergbi | not directly but it would likely conflict with the spawn fix we need there | 10:02 |
aevri | I see | 10:03 |
juergbi | and if we have to disable cache usage monitoring for spawn for now, the getstate changes in 1655 are not needed | 10:03 |
aevri | ok sure | 10:03 |
*** phildawson has quit IRC | 10:03 | |
aevri | I'll wip it again then | 10:03 |
juergbi | aevri: have you read my comment on !1659? what's the best way for this 'spawn' conditional? | 10:04 |
aevri | Let me see | 10:04 |
juergbi | or do you have an alternative suggestion off the top of your head? | 10:05 |
juergbi | ta | 10:05 |
juergbi | maybe simply multiprocessing.get_start_method() == 'spawn' | 10:06 |
aevri | that would do it | 10:06 |
juergbi | ok, ta, let's go with this for now to unblock this MR | 10:07 |
aevri | iiuc, this is simply a problem with the tests causing a fork; `bst` itself would be fine right? | 10:07 |
traveltissues | yes | 10:08 |
traveltissues | afaict | 10:08 |
aevri | I'm wondering if we can do `mark.run_in_subprocess` differently | 10:08 |
aevri | Playing locally, I didn't seem to need it in `spawn` mode, but I'm not sure what the orginal problem is | 10:08 |
juergbi | aevri: we might actually not need it at all anymore | 10:10 |
juergbi | originally it was to avoid issues between fork (job processes) and gRPC / background threads | 10:10 |
juergbi | which shouldn't be an issue with 'spawn' anyway | 10:10 |
aevri | I'm waiting on https://gitlab.com/BuildStream/buildstream/-/jobs/327970614 to see if we get tell-tale grpc errors | 10:10 |
aevri | ok cool | 10:11 |
juergbi | however, we should now properly clean up after gRPC and might not even need it anymore with regular 'fork' | 10:11 |
juergbi | so that could also be an option, although I'm not sure whether we should do it in the same MR | 10:11 |
juergbi | rather want to get this merged and handle this in a separate MR to avoid delaying this for much longer | 10:12 |
aevri | sure | 10:12 |
aevri | I can take on the `run_in_subprocess` thing if there's a rush on the other | 10:12 |
aevri | I'll do it separately | 10:12 |
traveltissues | https://gitlab.com/BuildStream/buildstream/pipelines/90518147 | 10:12 |
traveltissues | yikes | 10:13 |
juergbi | aevri: sounds good to me. however, I don't think it's terribly urgent | 10:14 |
juergbi | the 'run in subprocess' thing, I mean | 10:14 |
aevri | Sure, that's just more on my agenda of fixing up the spawn case | 10:14 |
juergbi | unless our workaround doesn't work ;) | 10:14 |
aevri | Hehe :) | 10:14 |
traveltissues | juergbi, see the above pipeline please | 10:15 |
traveltissues | aevri, ^ | 10:15 |
juergbi | exactly but I don't see the error message yet | 10:15 |
aevri | If 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 atm | 10:16 |
traveltissues | ah, nvm | 10:17 |
juergbi | should be easy to fix up | 10:17 |
traveltissues | yes | 10:17 |
aevri | ok | 10:17 |
gitlab-br-bot | santigl closed issue #898 (Remote Execution: Implement the REAPI Platform message) on buildstream https://gitlab.com/BuildStream/buildstream/issues/898 | 10:18 |
aevri | Small 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 IRC | 10:29 | |
*** narispo has joined #buildstream | 10:29 | |
*** akvilebirgelyte has quit IRC | 10:29 | |
*** akvilebirgelyte has joined #buildstream | 10:30 | |
gitlab-br-bot | traveltissues opened (was WIP) MR !1659 (traveltissues/1176->master: create usage monitor early) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1659 | 10:31 |
*** phildawson_ has quit IRC | 10:33 | |
gitlab-br-bot | BenjaminSchubert approved MR !1659 (traveltissues/1176->master: create usage monitor early) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1659 | 10:33 |
traveltissues | ty | 10:35 |
gitlab-br-bot | cs-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/1662 | 10:56 |
*** lachlan has quit IRC | 11:02 | |
*** phildawson_ has joined #buildstream | 11:03 | |
gitlab-br-bot | traveltissues approved MR !1662 (chandan/register-pytest-mark->master: setup.cfg: Register mark for pytest-datafiles) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1662 | 11:03 |
gitlab-br-bot | marge-bot123 closed issue #1176 (Interactive bst operations fail with open workspaces) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1176 | 11:17 |
gitlab-br-bot | marge-bot123 merged MR !1659 (traveltissues/1176->master: create usage monitor early) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1659 | 11:17 |
traveltissues | what's happening with !1549 btw | 11:18 |
gitlab-br-bot | MR !1549: plugins/elements/cmake.yaml: always specify variable types https://gitlab.com/BuildStream/buildstream/merge_requests/1549 | 11:18 |
*** phildawson_ has quit IRC | 12:01 | |
*** lachlan has joined #buildstream | 12:04 | |
gitlab-br-bot | marge-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/1662 | 12:12 |
*** santi has quit IRC | 12:16 | |
*** phildawson_ has joined #buildstream | 12:57 | |
*** phil has joined #buildstream | 13:08 | |
*** phildawson_ has quit IRC | 13:09 | |
*** santi has joined #buildstream | 13:17 | |
gitlab-br-bot | aevri 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/1661 | 13:31 |
aevri | ^ bye bye `mark.in_subprocess` :) | 13:31 |
tlater[m] | aevri: But we only added it like 2 weeks ago! | 13:39 |
aevri | whoosh, things are moving fast :D | 13:42 |
benschubert | aevri: is that correct? Don't we want it for some tests that do actually change the global state? | 13:45 |
aevri | iiuc we only needed it for working around grpc background threads not being cleaned up - if anyone knows better please say | 13:45 |
aevri | This is what I'm going by: https://gitlab.com/BuildStream/buildstream/commit/0ca7169a1fe4dfedb3524db35387e900bd4f4b10#626af34e8204774ea46e20955936146dedc828eb_70_75 | 13:46 |
aevri | https://www.irccloud.com/pastebin/JqPwNNnO/ | 13:47 |
aevri | ^ doh, my IRC web client made that pastebin for me, apologies. | 13:48 |
benschubert | ok thanks! | 13:48 |
juergbi | aevri: thanks for the quick MR, lgtm | 14:08 |
juergbi | commented with one nit | 14:08 |
aevri | Thanks! | 14:14 |
gitlab-br-bot | cs-shadow opened issue #1179 (tests/sources/keytest.py is buggy) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1179 | 14:32 |
gitlab-br-bot | cs-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/1664 | 14:45 |
*** phoenix has joined #buildstream | 14:46 | |
*** phoenix has quit IRC | 14:57 | |
gitlab-br-bot | tpollard 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/1664 | 14:58 |
gitlab-br-bot | marge-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/1661 | 15:03 |
*** traveltissues has quit IRC | 15:14 | |
*** santi has quit IRC | 15:43 | |
*** bochecha has quit IRC | 15:51 | |
*** akvilebirgelyte has quit IRC | 15:59 | |
*** santi has joined #buildstream | 16:03 | |
*** narispo has quit IRC | 16:29 | |
*** narispo has joined #buildstream | 16:29 | |
*** narispo has quit IRC | 16:53 | |
*** narispo has joined #buildstream | 16:53 | |
*** narispo has quit IRC | 16:57 | |
*** jonathanmaw has quit IRC | 17:00 | |
*** santi has quit IRC | 17:03 | |
*** tiagogomes has quit IRC | 17:08 | |
*** lachlan has quit IRC | 17:11 | |
*** cs-shadow has quit IRC | 20:56 | |
gitlab-br-bot | marge-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/1664 | 21:12 |
gitlab-br-bot | marge-bot123 closed issue #1179 (tests/sources/keytest.py is buggy) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1179 | 21:12 |
gitlab-br-bot | mostynb opened MR !1665 (link_fix->master: fix broken user guide link in README.rst) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1665 | 21:51 |
*** narispo has joined #buildstream | 22:28 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!