| *** xjuan has joined #buildstream | 02:11 | |
| *** xjuan has quit IRC | 02:43 | |
| *** tristan has quit IRC | 05:55 | |
| *** tristan has joined #buildstream | 06:26 | |
| *** xjuan has joined #buildstream | 06:30 | |
| *** xjuan has quit IRC | 06:43 | |
| jennis | benschubert, thanks for all the lint fixes | 08:07 |
|---|---|---|
| jennis | It'd be nice if we could disable "redefined-outer-name" at a directory level | 08:08 |
| jennis | And also, are *all* of the "unused-import" disables for cli really necessary? I think for some modules, we perhaps don't need them. i.e. when we explicitly invoke `cli.foo()` (would be my guess) we should avoid adding this disable comment if we don't need it | 08:09 |
| benschubert | jennis: no worries. We sadly can't disable at a per-directory level, I've checked and am subscribed on the pylint issue about that | 08:11 |
| benschubert | For the unused-import, I haven't found a place where we could not put it :/ | 08:11 |
| jennis | Ahh that's a shame, I just asked on #pylint too, but if there is an open issue I guess its not possible | 08:12 |
| jennis | Mhmm, so even for modules where we explicitly invoke it, pylint is complaining? | 08:12 |
| benschubert | Yep, because the name is shadowed by the function parameter | 08:13 |
| benschubert | so the import looks like unused | 08:13 |
| benschubert | https://github.com/PyCQA/pylint/issues/618 for the issue | 08:13 |
| jennis | Might be worth linking that issue in the commit message? | 08:14 |
| jennis | Also, the issue about no blanket disables | 08:15 |
| benschubert | jennis: we had it already linked in the first commit about tests and linting, I'm not sure it's good relinking on every linting change :) But if you'd rather have it, I can add it | 08:21 |
| *** adds68 has joined #buildstream | 08:21 | |
| jennis | mhmm I think for the sake of git blame, if someone goes, "why the hell are these disables in every file", it'd be nice for them to immediately see the issue | 08:25 |
| jennis | Which is exact thought process I just went through | 08:25 |
| benschubert | sure | 08:26 |
| benschubert | I'll add this | 08:26 |
| *** raoul has joined #buildstream | 08:28 | |
| *** ikerperez has joined #buildstream | 08:33 | |
| gitlab-br-bot | aevri approved MR !1378 (bschubert/pylint-fixes->master: Ensure pylint runs in some tests paths) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1378 | 08:37 |
| jennis | benschubert, from #pylint: jennis: maybe a pylintrc in the directory? The doc mentions "If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a pylintrc file" but I don't know whether that's the cwd, the root directory provided to pylint, or the directory it's currently working on | 08:38 |
| jennis | (aka the directory containing the files it's processing at the moment) | 08:38 |
| jennis | Regarding a blanket disable ^ | 08:39 |
| benschubert | jennis: the problem is you can't compose two configs | 08:39 |
| benschubert | so you defacto end up with two different pylint files | 08:39 |
| jennis | awh | 08:39 |
| benschubert | jennis: https://gitlab.com/BuildStream/buildstream/merge_requests/1222 for reference | 08:40 |
| benschubert | that's where we decided to disable per file instead | 08:40 |
| gitlab-br-bot | aevri approved MR !1377 (bschubert/pylint-artifactcache->master: Ensure pylint runs in tests/artifactcache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1377 | 08:40 |
| jennis | cool, thanks for pointing that out | 08:42 |
| Kinnison | benschubert: can we use a makefile to combine them? | 08:42 |
| benschubert | Kinnison: I'm not sure that's worth the hack | 08:43 |
| Kinnison | fair | 08:43 |
| *** pointswaves has joined #buildstream | 08:44 | |
| *** jonathanmaw has joined #buildstream | 08:46 | |
| jennis | tristan, are you around? Would you be able to respond to the final review comments on !1344? There is one comment (regarding Queue.PENDING) that I'm not clear on how you want documented | 08:46 |
| gitlab-br-bot | MR !1344: WIP: Push based pipeline https://gitlab.com/BuildStream/buildstream/merge_requests/1344 | 08:46 |
| gitlab-br-bot | jennis approved MR !1376 (bschubert/pylint-integration->master: Ensure pylint runs in tests/integration) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1376 | 08:50 |
| jennis | benschubert, I probably jumped the gun on that ^, I should say, subject to the upstream issues in the commit messages | 08:51 |
| benschubert | jennis: about !1376, could you double check that the new message is fine for you? | 08:51 |
| jennis | mhmm small nit from me: "https://github.com/PyCQA/pylint/issues/618 about this." doesn't read so well | 08:52 |
| jennis | Perhaps "See upstream issue: https://github.com/PyCQA/pylint/issues/618" | 08:53 |
| gitlab-br-bot | jennis approved MR !1376 (bschubert/pylint-integration->master: Ensure pylint runs in tests/integration) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1376 | 08:55 |
| jennis | Cool, thanks | 08:55 |
| benschubert | I'll do the same on the other two PRs | 08:55 |
| jennis | Ok, let me know when you've pushed and I'll re-review | 08:56 |
| *** pointswaves has quit IRC | 08:56 | |
| benschubert | jennis: ndone for !1377 | 09:02 |
| benschubert | marge seems to have died again? | 09:05 |
| jennis | :@ | 09:14 |
| *** phildawson_ has joined #buildstream | 09:14 | |
| gitlab-br-bot | jennis approved MR !1377 (bschubert/pylint-artifactcache->master: Ensure pylint runs in tests/artifactcache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1377 | 09:15 |
| jennis | benschubert, apparently she's not dead | 09:20 |
| benschubert | jennis: for https://gitlab.com/BuildStream/buildstream/merge_requests/1378 I added it on the MR description, so it will be on the merge commit, goodf for you? | 09:25 |
| jennis | benschubert, sec | 09:27 |
| jennis | yeah nice idea | 09:28 |
| gitlab-br-bot | jennis merged MR !1378 (bschubert/pylint-fixes->master: Ensure pylint runs in some tests paths) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1378 | 09:28 |
| jennis | Ahh | 09:28 |
| jennis | WHOOPS | 09:28 |
| jennis | merge != approve | 09:28 |
| jennis | my bad, I'll accept my public flogging | 09:29 |
| *** CTtpollard is now known as tpollard | 09:32 | |
| * tpollard flogs jennis | 09:35 | |
| *** lachlan has joined #buildstream | 09:44 | |
| *** lachlan has quit IRC | 09:52 | |
| *** lachlan has joined #buildstream | 09:59 | |
| jennis | https://gitlab.com/BuildStream/buildstream/merge_requests/1376#note_178493881 Marge is broken from the inside again | 10:02 |
| gitlab-br-bot | juergbi opened issue #1042 (Use buildbox-casd for CAS access) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1042 | 10:07 |
| *** lachlan has quit IRC | 10:14 | |
| gitlab-br-bot | danielsilverstone-ct approved MR !1379 (bschubert/optimize-loader-types->master: Optimize _loader/types.py) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1379 | 10:16 |
| gitlab-br-bot | tpollard approved MR !1374 (aevri/spawk->master: Rename (spawn, fork) -> 'start process') on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1374 | 10:16 |
| *** lachlan has joined #buildstream | 10:16 | |
| Kinnison | sigh | 10:16 |
| tpollard | ? | 10:17 |
| *** lachlan has quit IRC | 10:32 | |
| jennis | so tests/sourcecache/push.py::test_push_fail keeps failing and we're getting orange pipelines | 10:42 |
| jennis | I've run this test locally inside the same docker container we're using for "tests-fedora-update-deps" and it passes :/ | 10:43 |
| jennis | Not sure if anyone has an idea about this | 10:43 |
| tpollard | got a link to a failed console? | 10:44 |
| jennis | Every orange pipeline :P | 10:44 |
| jennis | but here's one: https://gitlab.com/BuildStream/buildstream/-/jobs/226353655 | 10:44 |
| jennis | Looks like it's just trying to equate a long string to an object | 10:45 |
| jennis | It doesn't look like the "in" syntax is doing what we'd expect, but I could be wrong | 10:45 |
| tpollard | :S | 10:50 |
| tpollard | didn't we have another problem recently with that docker image | 10:51 |
| jennis | I'm not sure | 10:52 |
| tpollard | it's ringing a bell | 10:52 |
| *** lachlan has joined #buildstream | 10:53 | |
| tpollard | I can't find anything, but I definitely remember recently looking at the definition of it for some reason | 10:55 |
| tpollard | something something an update to grpc which changed some error api | 10:59 |
| tpollard | doesn't that ring a bell raoul? | 10:59 |
| raoul | yep that sounds right | 11:01 |
| raoul | grpc gives a different error in the updated version so test fails | 11:01 |
| raoul | you might not have updated your docker image or something which could be causing it to pass locally. | 11:01 |
| laurence | We're seeing some issues with marge-bot at the moment - one is that there's a bug causing random intermittent failures | 11:02 |
| jennis | This is looking pretty consistent | 11:03 |
| laurence | https://github.com/smarkets/marge-bot/issues/188 | 11:03 |
| laurence | raised upstream ^ | 11:03 |
| laurence | and the other is that it's managed by Codethink and would be better if this project looked after it | 11:03 |
| laurence | at the same time i think it would be wise to help 'spread the load' of managing everything infra related away from just relying solely on valentind | 11:03 |
| laurence | and increase the bus factor | 11:04 |
| tpollard | jennis: https://irclogs.baserock.org/buildstream/%23buildstream.2019-05-30.log.html#t2019-05-30T10:56:45 the conversation was here | 11:04 |
| laurence | I shall email the list as well, but any volunteers for this? | 11:04 |
| jennis | tpollard, juergbi, raoul, so it turns out the assert is triggering because the std.err is actually different | 11:32 |
| jennis | I wanted to know whether it'd be ok to make the check less strict | 11:32 |
| jennis | Right now, we check for: "Failed to initialize remote http://localhost:34687: Connect Failed" | 11:33 |
| gitlab-br-bot | marge-bot123 merged MR !1376 (bschubert/pylint-integration->master: Ensure pylint runs in tests/integration) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1376 | 11:33 |
| jennis | Whereas this test is failing because: Failed to initialize remote http://localhost:34687: failed to connect to all addresses | 11:33 |
| juergbi | ah, gRPC error message change? | 11:33 |
| juergbi | definitely sounds fine to accept either | 11:34 |
| jennis | So failed to connect to all addresses != connect failed | 11:34 |
| jennis | Ok, I'll make it less strict | 11:34 |
| tpollard | yes accept either, else we'll get this when the other pipelines bump to the same version of grpc | 11:37 |
| *** bilelmoussaoui has joined #buildstream | 11:38 | |
| gitlab-br-bot | jennis opened MR !1381 (jennis/fix_failing_test->master: Don't assert gRPC messages which may change) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1381 | 11:45 |
| *** bilelmoussaoui has quit IRC | 11:47 | |
| jennis | juergbi, tpollard ^ | 11:51 |
| jennis | I didn't go with accepting either just incase it changes again... | 11:53 |
| gitlab-br-bot | tpollard approved MR !1381 (jennis/fix_failing_test->master: Don't assert gRPC messages which may change) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1381 | 11:55 |
| tpollard | We're only bothered that it fails to connect with the given remote ip, the rest of the message is irrelevant imo | 11:56 |
| gitlab-br-bot | raoul.hidalgocharman approved MR !1381 (jennis/fix_failing_test->master: Don't assert gRPC messages which may change) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1381 | 12:00 |
| gitlab-br-bot | marge-bot123 merged MR !1373 (aevri/defensive_send_message->master: _scheduler/jobs: refactor, defensive send_message) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1373 | 12:15 |
| gitlab-br-bot | willsalmon approved MR !1375 (bschubert/site-consolidation->master: Remove tests/testutils/site.py and move everything to buildstream/testing/_utils/site.py) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1375 | 12:53 |
| *** tpollard has quit IRC | 13:12 | |
| *** tpollard has joined #buildstream | 13:15 | |
| *** ChanServ sets mode: +o tristan | 13:25 | |
| tristan | jonathanmaw, Gah, I totally overlooked your email today, I saw it come in late last night and intended to give it attention :-/ | 13:25 |
| * tristan makes a note to not forget tomorrow | 13:26 | |
| jonathanmaw | :) | 13:26 |
| benschubert | Hey tristan in !1375, I consolidated all the sites.py files for testing. Would you see a reason not to make it public? A few plugin projects redefine them, I think we would gain by making this public | 13:27 |
| *** lachlan has quit IRC | 13:29 | |
| tristan | benschubert, seems odd to me to make public, if things are well segmented | 13:30 |
| benschubert | tristan: the HAS_SANDBOX for example is requried by many | 13:30 |
| benschubert | same for IS_WSL/Linux/Windows | 13:31 |
| tristan | benschubert, I mean... repo has their own different set of dependencies right ? it wouldnt make sense for BuildStream to have knowledge about third party plugin repo dependencies | 13:31 |
| benschubert | tristan: I didn't mean adding things from other repos | 13:31 |
| tristan | benschubert, for what is common, yeah that seems to make sense | 13:31 |
| benschubert | but I think that BuildStream should open the common stuff :) | 13:31 |
| tristan | I didn't look through the patch, just judging the book by its cover ;-) | 13:31 |
| benschubert | Ok, so not expose everything (Git, Bzr, etc) but open Has_sandbox and MACHINR_ARCH, etc ? | 13:32 |
| tristan | benschubert, it make sense only in testing/ ? or are there things that a plugin should know about at runtime and not at test time ? | 13:32 |
| benschubert | only test time | 13:32 |
| benschubert | that's in testings/_utils/site currently | 13:33 |
| tristan | Right, sounds sensible to me :) | 13:33 |
| benschubert | and I'd be for putting at least some of it in testing/site | 13:33 |
| benschubert | Ok! I'll add a new PR splitting those then | 13:33 |
| benschubert | thanks! | 13:33 |
| tristan | benschubert, sorry I didn't get around to that last night, saw you left a comment on IRC | 13:33 |
| * tristan has been booting VMs all day | 13:34 | |
| benschubert | ah no worries, it was mainly for information purposes :) | 13:34 |
| *** lachlan has joined #buildstream | 13:37 | |
| benschubert | jennis: was https://gitlab.com/BuildStream/buildstream/merge_requests/1377 good for you? | 13:40 |
| jennis | benschubert, one sec | 13:40 |
| gitlab-br-bot | jennis approved MR !1377 (bschubert/pylint-artifactcache->master: Ensure pylint runs in tests/artifactcache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1377 | 13:41 |
| jennis | benschubert, yes | 13:41 |
| gitlab-br-bot | marge-bot123 merged MR !1381 (jennis/fix_failing_test->master: Don't assert gRPC messages which may change) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1381 | 13:59 |
| tpollard | woop | 13:59 |
| *** tristan has quit IRC | 14:26 | |
| gitlab-br-bot | marge-bot123 merged MR !1374 (aevri/spawk->master: Rename (spawn, fork) -> 'start process') on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1374 | 14:33 |
| *** tristan has joined #buildstream | 14:54 | |
| laurence | There's an old ticket lying around in #nosoftware related to creation of a sub-group for infrastructure - https://gitlab.com/BuildStream/nosoftware/alignment/issues/42 | 15:02 |
| laurence | anyone mind if i create this? | 15:02 |
| laurence | valentin and I have started to write down all of the infra connected to the project | 15:03 |
| laurence | we need somewhere public to store it (apart from credentials, obviously) | 15:03 |
| gitlab-br-bot | marge-bot123 merged MR !1375 (bschubert/site-consolidation->master: Remove tests/testutils/site.py and move everything to buildstream/testing/_utils/site.py) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1375 | 15:04 |
| laurence | Maybe just using the wiki would be enough, actually | 15:05 |
| gitlab-br-bot | aevri opened (was WIP) MR !1380 (aevri/job_msg_enum->master: Use enums for Job-related constants) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1380 | 15:51 |
| gitlab-br-bot | marge-bot123 merged MR !1377 (bschubert/pylint-artifactcache->master: Ensure pylint runs in tests/artifactcache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1377 | 15:52 |
| gitlab-br-bot | raoul.hidalgocharman approved MR !1380 (aevri/job_msg_enum->master: Use enums for Job-related constants) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1380 | 16:22 |
| *** xjuan has joined #buildstream | 16:24 | |
| gitlab-br-bot | BenjaminSchubert approved MR !1380 (aevri/job_msg_enum->master: Use enums for Job-related constants) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1380 | 16:32 |
| *** raoul has quit IRC | 16:38 | |
| *** xjuan has quit IRC | 16:48 | |
| gitlab-br-bot | marge-bot123 merged MR !1379 (bschubert/optimize-loader-types->master: Optimize _loader/types.py) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1379 | 16:48 |
| *** pointswaves has joined #buildstream | 17:10 | |
| *** shibu has joined #buildstream | 17:11 | |
| *** shibu has quit IRC | 17:13 | |
| *** lachlan has quit IRC | 17:14 | |
| gitlab-br-bot | marge-bot123 merged MR !1380 (aevri/job_msg_enum->master: Use enums for Job-related constants) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1380 | 17:17 |
| *** jonathanmaw has quit IRC | 17:22 | |
| *** lachlan has joined #buildstream | 17:28 | |
| *** pointswaves has quit IRC | 17:33 | |
| *** pointswaves has joined #buildstream | 17:36 | |
| *** pointswaves has quit IRC | 17:39 | |
| *** pointswaves has joined #buildstream | 17:40 | |
| *** pointswaves has quit IRC | 17:41 | |
| *** xjuan has joined #buildstream | 17:41 | |
| *** pointswaves has joined #buildstream | 17:42 | |
| *** lachlan has quit IRC | 17:44 | |
| *** pointswaves has quit IRC | 17:47 | |
| *** jswagner has joined #buildstream | 18:57 | |
| gitlab-br-bot | BenjaminSchubert opened MR !1383 (bschubert/optimize-extract-depends-node->master: rewrite _extract_depends_from_node in Cython and optimize) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1383 | 20:26 |
| *** xjuan has quit IRC | 21:04 | |
| *** xjuan has joined #buildstream | 21:50 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!