*** brlogger has joined #buildstream | 07:37 | |
*** SotK has quit IRC | 07:38 | |
*** qinusty has quit IRC | 07:38 | |
*** ikerperez has quit IRC | 07:38 | |
*** samkirkham has quit IRC | 07:38 | |
*** adds68 has quit IRC | 07:38 | |
*** bethw has quit IRC | 07:38 | |
*** slaf has quit IRC | 07:38 | |
*** lantw44 has quit IRC | 07:38 | |
*** Trevinho has quit IRC | 07:38 | |
*** slaf has joined #buildstream | 07:38 | |
*** SotK has joined #buildstream | 07:39 | |
*** qinusty has joined #buildstream | 07:39 | |
*** ikerperez has joined #buildstream | 07:39 | |
*** samkirkham has joined #buildstream | 07:39 | |
*** adds68 has joined #buildstream | 07:39 | |
*** bethw has joined #buildstream | 07:39 | |
*** lantw44 has joined #buildstream | 07:39 | |
*** Trevinho has joined #buildstream | 07:39 | |
*** toscalix has joined #buildstream | 08:07 | |
*** tme5 has joined #buildstream | 08:42 | |
*** jonathanmaw has joined #buildstream | 08:51 | |
gitlab-br-bot | tpollard approved MR !1559 (tmewett/build-deps-cli->master: Remove build --all flag in favour of --deps all/plan) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1559 | 08:55 |
---|---|---|
benschubert | if someone has time, a review of !1537 would be much appreciated :) | 09:04 |
tme5 | cs-shadow, how do you recommend I deal with the pytest-forked code? split it into a separate file with the MIT license at the top? | 09:30 |
tme5 | @ anyone else too.. regarding !1557 | 09:30 |
gitlab-br-bot | MR !1557: Add in_subprocess pytest mark and modify tests which run in another process to use it https://gitlab.com/BuildStream/buildstream/merge_requests/1557 | 09:30 |
*** benschubert has quit IRC | 09:35 | |
*** benschubert has joined #buildstream | 09:35 | |
benschubert | if someone has time, a review of !1537 would be much appreciated :) | 09:35 |
benschubert | (sorry if that was a duplicate post, the bot doesn't seem to be putting a link to my MR) | 09:36 |
tlater[m] | benschubert: I was experiencing some serious deja vu there | 09:36 |
benschubert | tlater[m]: ok, good to know the problem is on the bot's side :'D | 09:37 |
cs-shadow | tme5: hi, I think we can do something similar to https://gitlab.com/BuildStream/buildstream/blob/master/src/buildstream/_fuse/fuse.py | 09:52 |
cs-shadow | which is incidentally also MIT | 09:52 |
tme5 | ah yeah ok | 09:53 |
tme5 | i'll do that then, thanks | 09:57 |
cs-shadow | np | 09:59 |
gitlab-br-bot | tlater opened MR !1561 (tlater/source-pushll->master: Introduce `bst shource push/pull` commands) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1561 | 10:59 |
tpollard | shource :P | 10:59 |
tme5 | cs-shadow, turns out pytest-forked is already listed in dev-requirements ! | 11:01 |
benschubert | tme5: that seems... incorrect? | 11:01 |
tme5 | as in, it shouldn't be? | 11:02 |
tme5 | but as the mark depends on implementation details of the plugin I suppose we should have our own copy still, so we can control it | 11:02 |
tme5 | oh, added by pip freeze | 11:03 |
benschubert | interesting | 11:03 |
tme5 | resolved all threads on !1557 now, if anyone wants to have look (this is the MR tlater[m] ) | 11:07 |
gitlab-br-bot | MR !1557: Add in_subprocess pytest mark and modify tests which run in another process to use it https://gitlab.com/BuildStream/buildstream/merge_requests/1557 | 11:07 |
tlater[m] | tme5: If anything, this will make my tests neater :) | 11:31 |
tlater[m] | I almost went down the route of doing something like that myself yesterday, good thing I didn't. | 11:31 |
tlater[m] | benschubert: Regarding types.py, is your intention to have all `FastEnum`s in that module? | 11:32 |
tlater[m] | i've shoved my enum closer to the place where things are parsed, but I'm not sure if that's the best place to put it. | 11:33 |
tme5 | don't merge me yet! | 11:36 |
tme5 | oh wait, yes, you can see that it fails CI too... lol | 11:36 |
tlater[m] | It'll refuse to merge if CI fails anyway ;) | 11:39 |
tme5 | tlater[m], your comment about "where did this go," do you mean the code in _test_push_tree or a specific line? | 11:47 |
tlater[m] | That specific line. I couldn't figure out why it disappeared. | 11:47 |
tme5 | cas = context.get_cascache() ? | 11:48 |
tlater[m] | Yep | 11:48 |
tlater[m] | I thought it should be in the new code too, but couldn't spot it. | 11:48 |
tlater[m] | Although I didn't see cas used either, so... | 11:48 |
tme5 | the diff is unfortuntely a huge mess | 11:48 |
tme5 | using git diff -b locally makes it a bit nicer | 11:49 |
tme5 | i'll have a look | 11:49 |
benschubert | tlater[m]: not neccessarily, making it closer to where it's used is better unless it's used accross the codebase :) | 12:36 |
tlater[m] | Great, just wanted to double check. Thanks! | 12:41 |
benschubert | Did anyone ever saw https://gitlab.com/BuildStream/bst-plugins-experimental/-/jobs/278649488 happening on master or something? juergbi ? I find it very weird that this is broken on bst-plugins-experimental (Note: the CI had actually never tested all of those tests on the plugins...) | 13:29 |
benschubert | That happens when running 2 tests one after the other | 13:29 |
juergbi | "ValueError: I/O operation on closed file" | 13:30 |
juergbi | no, I don't think I've ever seen that before | 13:30 |
benschubert | ok :( it's part of the Mounter... which hasn't been touched in months so not sure what's happening there | 13:30 |
*** jonathanmaw has quit IRC | 13:42 | |
tme5 | Somehow my MR has dropped the coverage to 48% ... which is a little alarming? | 13:43 |
tlater[m] | Hm, is coverage.py smart enough to figure out when you're calling things on a different thread? | 13:44 |
tme5 | maybe not, where is that file? | 13:47 |
benschubert | tlater[m]: coverage on multi-thread/process needs to be configured, look at the docs for it :) | 13:48 |
tlater[m] | tme5: It's a library ;) | 13:48 |
tlater[m] | Well, a tool, I guess | 13:49 |
tlater[m] | tme5: https://coverage.readthedocs.io/en/v4.5.x/ | 13:49 |
tme5 | thanks, i'll investiage | 13:53 |
tme5 | igate | 13:53 |
tme5 | interesting, looks like i need to run a coverage hook in the child process | 14:01 |
*** narispo has quit IRC | 14:26 | |
*** narispo has joined #buildstream | 14:26 | |
*** narispo has quit IRC | 14:33 | |
*** narispo has joined #buildstream | 14:33 | |
*** toscalix has quit IRC | 14:38 | |
* tlater[m] wonders why he needs to have cmake installed to build the docs | 14:53 | |
tme5 | you do? i didn;t when i ran `tox -e docs` | 15:03 |
tlater[m] | Looks like I had some old debris in my repo | 15:10 |
* tlater[m] thinks `git clean -fxd` will have solved it. | 15:10 | |
tlater[m] | Yup, that was it | 15:10 |
tlater[m] | Aww, boo, I also made changes in one of those debris files | 15:13 |
tme5 | i was going to say... be careful haha | 15:13 |
tme5 | did you ever see that VSCode thing, where their git UI had a reset option, except it silently and without warning ran `git clean` also | 15:14 |
* tlater[m] has never used VSCode | 15:15 | |
tlater[m] | Sounds like a fun little implicit thing to do though | 15:15 |
tme5 | me neither, the issue report got passed around though because it was rather.. pointed.. haha | 15:16 |
tlater[m] | Fair enough. I'm a little oblivious regarding these things :) | 15:16 |
* tlater[m] continues to hack in his isolated little micro-environment | 15:17 | |
tlater[m] | I wonder where this debris came from though. It seems like it was an entire second copy of the docs directory that masked the real one. | 15:17 |
tlater[m] | But entirely hidden from git | 15:18 |
tme5 | this coverage thing looks weird. it seems like I have to place a coverage call in the normal codebase | 15:49 |
*** tpollard has quit IRC | 15:50 | |
tme5 | actually that might not be true.. if i can somehow install a sitecustomize module in the python path with tox | 15:54 |
gitlab-br-bot | tlater opened (was WIP) MR !1540 (tlater/cache-endpoints->master: Support separate end points for artifact caches) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1540 | 15:57 |
tlater[m] | Ugh, too soon, pipelines are failing :| | 15:57 |
* tlater[m] wonders why # noqa isn't doing anything | 15:59 | |
tlater[m] | Right, they only let you use noqa for certain errors: https://github.com/PyCQA/pycodestyle/issues/180 | 16:03 |
* tlater[m] boggles | 16:03 | |
tlater[m] | The reasoning is "we know better and you should not ignore errors" | 16:03 |
tlater[m] | Like, sure, 99% of the time your tool is great, but I can't merge with it in CI, so let me make it shut up when it's wrong :| | 16:04 |
tlater[m] | Can I remove pycodestyle as part of my MR? | 16:04 |
tlater[m] | :D | 16:05 |
benschubert | No ? :'D | 16:15 |
tme5 | lol | 16:15 |
benschubert | tme5: you could also possibly register the coverage call on startup of the tests in conftest.py :) | 16:15 |
tme5 | ahh yeah, that's a good idea. it would have to be in a try ... except to catch ImportError i suppose, as coverage isn't a runtime dep? | 16:17 |
benschubert | correct | 16:17 |
benschubert | otherwise normally in setup.cfg you should be able to configure coverage | 16:18 |
benschubert | at least I remember doing it that way long ago | 16:18 |
tme5 | alright, yeah, i think i actually need to adjust coverage in tox.ini too because each process will need to write to a different report file | 16:18 |
tme5 | needs to be collected | 16:19 |
benschubert | correct | 16:19 |
tlater[m] | benschubert: tbf, I think the only errors pycodestyle catches that pylint doesn't is various whitespace details, and this thing about always using named functions instead of lambdas. | 16:22 |
* tlater[m] wonders if there's a better tool for this. | 16:23 | |
gitlab-br-bot | BenjaminSchubert opened MR !1562 (bschubert/fix-mutable-args->master: sandbox/_mounter: Remove default mutable arguments stderr/out) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1562 | 16:23 |
tlater[m] | Or really, if pylint would like a couple more checks against whitespace. | 16:23 |
benschubert | we probably could yes | 16:24 |
benschubert | I however really like pycodestyle since it's much faster than pylint, so I usually end up running it first and fixing everything it finds before running pylint | 16:24 |
tlater[m] | Ah, hehe, that's a point | 16:25 |
* tlater[m] just has both continuously running through python-language-server | 16:25 | |
benschubert | that's what I do on my personal laptop, the work laptop dies a bit too much with it :) | 16:25 |
tlater[m] | Hm, well, maybe I can work around this error somehow | 16:25 |
* tlater[m] likes the irony of more complex code to avoid making comments more complex | 16:26 | |
tlater[m] | Which, of course, leads to more complex comments! | 16:26 |
benschubert | If someone has time, Id' appreciate code reviews on !1562 and !1537, they are needed to fix bst-plugins-experimental problems :) | 16:29 |
gitlab-br-bot | MR !1537: testing/sources: Automatically register plugin sources https://gitlab.com/BuildStream/buildstream/merge_requests/1537 | 16:29 |
gitlab-br-bot | cs-shadow approved MR !1562 (bschubert/fix-mutable-args->master: sandbox/_mounter: Remove default mutable arguments stderr/out) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1562 | 16:33 |
*** tme5 has quit IRC | 16:43 | |
cs-shadow | i'm not sure how the docs.buildstream.build website works but the link to docs for master is 404'ing at the moment. | 16:43 |
cs-shadow | https://docs.buildstream.build/master/index.html | 16:43 |
tlater[m] | cs-shadow: I think juergbi changed that yesterday, probably accidental regression. | 16:44 |
* tlater[m] checks if he can find it | 16:44 | |
cs-shadow | thanks for looking! | 16:44 |
tlater[m] | Oh, no, 3 days ago... Hrm | 16:44 |
tlater[m] | Oof, that looks to be generated in some hairy bash regexes in a different repository | 16:51 |
* tlater[m] wishes he'd reviewed this | 16:51 | |
tlater[m] | cs-shadow: I'm pretty sure this script relies on an assumption that is simply not true for master | 16:59 |
tlater[m] | I don't think it was tested... | 16:59 |
tlater[m] | I'd need to ask alexfazakas, but he's currently at guadec | 16:59 |
cs-shadow | tlater[m]: no worries, it's not urgent. Just looks kinda bad if our docs website returns 404 :) | 17:00 |
* tlater[m] doesn't like it either :| | 17:00 | |
tlater[m] | Hrm, it looks like it might have tried to download generated docs from yesterday's failed pipeline on master | 17:03 |
* tlater[m] restarts that pipeline and hope that fixes the problem | 17:03 | |
benschubert | tlater[m]: a new MR should soon land on master anyways | 17:04 |
tlater[m] | benschubert: maybe, but I'd like to get this back up before the bank weekend, and I'll need to babysit the other pipeline that actually publishes the docs | 17:05 |
benschubert | right | 17:05 |
benschubert | but when the MR lands, you might loose this build | 17:06 |
tlater[m] | Go faster build! Hehe | 17:06 |
* tlater[m] will need to talk to alexfazakas about this script, it needs to be a bit more robust. | 17:07 | |
benschubert | and if we could avoid bash... especially since the codebase is in python already | 17:08 |
tlater[m] | Yeah, it's an odd choice. | 17:08 |
gitlab-br-bot | marge-bot123 merged MR !1562 (bschubert/fix-mutable-args->master: sandbox/_mounter: Remove default mutable arguments stderr/out) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1562 | 17:15 |
*** rdale has quit IRC | 18:26 | |
*** traveltissues has joined #buildstream | 23:52 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!