| *** dftxbs3e has joined #buildstream | 00:52 | |
| *** dftxbs3e has joined #buildstream | 00:53 | |
| *** dftxbs3e has quit IRC | 01:52 | |
| *** dftxbs3e has joined #buildstream | 02:27 | |
| *** dftxbs3e has quit IRC | 02:29 | |
| *** dftxbs3e has joined #buildstream | 02:29 | |
| *** dftxbs3e has quit IRC | 04:51 | |
| *** dftxbs3e has joined #buildstream | 04:53 | |
| *** tpollard has quit IRC | 05:02 | |
| *** rdale has quit IRC | 05:02 | |
| *** tiagogomes has quit IRC | 05:02 | |
| *** rdale has joined #buildstream | 05:05 | |
| *** tpollard has joined #buildstream | 05:05 | |
| *** jswagner has quit IRC | 05:50 | |
| *** jswagner has joined #buildstream | 05:51 | |
| *** jswagner has joined #buildstream | 05:51 | |
| *** jswagner has quit IRC | 06:09 | |
| *** jswagner has joined #buildstream | 06:12 | |
| *** jswagner has joined #buildstream | 06:19 | |
| *** jswagner has joined #buildstream | 06:27 | |
| *** jswagner has joined #buildstream | 06:29 | |
| *** jswagner has joined #buildstream | 06:29 | |
| *** jswagner has joined #buildstream | 06:30 | |
| *** jswagner has joined #buildstream | 06:31 | |
| *** jswagner has joined #buildstream | 06:32 | |
| *** jswagner has joined #buildstream | 06:32 | |
| *** tristan has quit IRC | 07:15 | |
| *** tristan has joined #buildstream | 07:23 | |
| *** tristan has quit IRC | 07:28 | |
| *** tristan has joined #buildstream | 07:42 | |
| *** ChanServ sets mode: +o tristan | 07:42 | |
| gitlab-br-bot | BenjaminSchubert merged MR !1460 (bschubert/node-provenance-2->bschubert/new-node-api: Replace 'node_get_provenance' by method on 'Node') on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1460 | 08:11 |
|---|---|---|
| Kinnison | tristan: Is !1411 abandoned? (I ask because it was approved 2 weeks ago but never merged, and when I handed it to Marge a week ago, it failed to merge due to test failures, and since then it has had no movement) | 08:13 |
| gitlab-br-bot | MR !1411: _frontend/cli.py: Exit with error if output streams are set to nonblocking https://gitlab.com/BuildStream/buildstream/merge_requests/1411 | 08:13 |
| *** alexandrufazakas has joined #buildstream | 08:13 | |
| tristan | Kinnison, Interestingly that was on my mind right now because of jonathanmaw's comments about it happening to him | 08:14 |
| tristan | Kinnison, I honestly thought it was merged | 08:14 |
| Kinnison | :D | 08:14 |
| * Kinnison is glad to have re-raised it to your attention at least | 08:14 | |
| tristan | I rebased it, looks like CI failed for a spurious error in one of the jobs | 08:15 |
| tristan | lets just hand it back to marge | 08:15 |
| tristan | Hmmm, now it has "two assignees", is that a new gitlab thing ? | 08:16 |
| Kinnison | Dunno, but I've noticed marge might not work if she's not alone | 08:16 |
| Kinnison | so you might want to unassign yourself | 08:16 |
| tristan | done | 08:17 |
| tristan | !1389 didnt land either | 08:18 |
| gitlab-br-bot | MR !1389: _frontend/cli.py: Exit with error if output streams are set to nonblocking https://gitlab.com/BuildStream/buildstream/merge_requests/1389 | 08:18 |
| * Kinnison has only been moving slowly through the list, and admittedly I've avoided things targetted purely at 1.2 because I wasn't sure of the rules there | 08:19 | |
| jennis | The docker we're using for marge (:latest) doesn't include a bug fix where marge can deal with being assigned, as well as another | 08:20 |
| jennis | Which, incidentially, causes marge to be inactive | 08:20 |
| Kinnison | heh | 08:20 |
| jennis | I've filed an issue upstream ad hopefully they'll do a release | 08:20 |
| gitlab-br-bot | BenjaminSchubert opened MR !1464 (bschubert/node-private-public->bschubert/new-node-api: Mark APIs on Node as private or public) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1464 | 08:21 |
| benschubert | hey tristan, ^ marks the methods on the nodes as public or private, would oyu have time ot take a look? I'll have a MR afterwards to reorganize the code, and another one to move the Node parts to a 'node.py'public module | 08:22 |
| benschubert | Kinnison, jennis ^i'd also like your input | 08:22 |
| *** tiagogomes has joined #buildstream | 08:23 | |
| tristan | benschubert, Ah, moving __contains__ to only be on MappingNode, good call :) | 08:25 |
| benschubert | tristan: we could also have it for sequence, but that would be weird :) | 08:25 |
| * tristan got confused with the simple diff, and appreciate's gitlab's new "show full file" feature | 08:25 | |
| benschubert | agreed :D | 08:25 |
| tristan | benschubert, I'm having trouble understanding the intentation... are get_scalar(), get_mapping(), get_node() nested functions of the renamed _get() function ? | 08:27 |
| tristan | oh obviously not | 08:28 |
| tristan | as they call _get() | 08:28 |
| benschubert | tristan: they are not, same level :) weird that gitlab shows a weird indent | 08:29 |
| tristan | Nah it's just I guess my fonts, hard to see | 08:32 |
| tristan | benschubert, I have some comments which are not suitable for the review of this particular patch | 08:33 |
| tristan | I made one comment, tiny thing | 08:33 |
| benschubert | tristan: I'm all ears! | 08:33 |
| *** traveltissues has joined #buildstream | 08:34 | |
| tristan | benschubert, First one is... when you say the next MR will reorganize the code... I suppose you mean to place private functions at the bottom (except I think for magic __pythonfunctions__ which usually go right at the top with __init__) ? | 08:34 |
| benschubert | correct | 08:34 |
| benschubert | https://gitlab.com/BuildStream/buildstream/commit/924bc4e7e7b7c74210aefc900db2a8fb51fa3ef3 the commit is already ready | 08:35 |
| tristan | Just checking..., almost commented that but then realized you intended to do it separately :) | 08:35 |
| benschubert | would you rather have it in the same MR? | 08:35 |
| benschubert | I can do that easily if so | 08:35 |
| tristan | benschubert, The other thing is about methods which get overridden/implemented in subclasses, and about documentation | 08:35 |
| benschubert | was just fearing the diff might be harder | 08:35 |
| tristan | benschubert, Nah I really don't mind | 08:35 |
| benschubert | ok | 08:35 |
| benschubert | for documentation, I intend in adding it when moving the classes to 'node.py' | 08:36 |
| benschubert | and I'm not sure what you mean about the methods for override/implement in subclasses | 08:36 |
| tristan | So about "abstract" or "virtual" methods... I find the current _composite() approach confusing and not inline with how we do it elsewhere | 08:36 |
| tristan | So yeah this is partly about documentation but also about code structure for that documentation | 08:37 |
| tristan | So it's nice that the different node types have their own implementations of _composite(), that's the right approach | 08:37 |
| tristan | But as they all derive from Node, I would expect Node to have an empty _composite() implementation which documents what that function should do for subclasses | 08:38 |
| tristan | Rather than having subclasses redundantly document it | 08:38 |
| tristan | Since there is now a _composite() __and__ a __composite() apparently... I would expect them to both do this | 08:39 |
| tristan | Or rather, whichever one of the two is intended for Node classes to implement, should have that documented in the common ancestor | 08:39 |
| tristan | benschubert, Am I making any sense ? :) | 08:39 |
| tristan | Usually we have an empty declaration in a common ancestor which either does 'pass' or 'raise ImplError()' | 08:40 |
| benschubert | right | 08:44 |
| benschubert | sorry, was just grabbing a coffee | 08:44 |
| benschubert | So, _composite and __composite are really MappingNode specific | 08:45 |
| benschubert | the one that is shared is _compose_on | 08:45 |
| benschubert | which is defined in 'Node' :) | 08:45 |
| *** bochecha has joined #buildstream | 08:45 | |
| benschubert | _composite is the previous 'composite_dict' function we had, and __composite is the previous '_composite' helper we had, that was MappingNode specific | 08:46 |
| tristan | benschubert, Ok so I was reading it wrong :) | 08:51 |
| tristan | With incoming doc comments I expect this will read more clearly :) | 08:52 |
| benschubert | Yep | 08:52 |
| benschubert | otherwise I will have failed the documentation | 08:52 |
| tristan | benschubert, While we're on the general topic, I keep thinking this is an opportunity to be stricter about plugin's validating keys in mapping nodes | 08:53 |
| tristan | but it's a can of worms I don't know if we have time to allocate to | 08:53 |
| benschubert | what's your high level idea? | 08:54 |
| tristan | benschubert, what's been playing in my mind is basically, we have these ALL_CAPS_CLASS_VARIABLES which affect class behavior | 08:54 |
| tristan | like "am I allowed to have build dependencies" and the like | 08:54 |
| tristan | What if we had something like CONFIG_SCHEMA | 08:54 |
| gitlab-br-bot | BenjaminSchubert merged MR !1462 (bschubert/node-cleanup->bschubert/new-node-api: _yaml: Remove code duplication on '_new_node_from_*') on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1462 | 08:54 |
| gitlab-br-bot | marge-bot123 closed issue #929 (Spurious blockingio error) on buildstream https://gitlab.com/BuildStream/buildstream/issues/929 | 08:55 |
| gitlab-br-bot | marge-bot123 merged MR !1411 (tristan/exit-on-nonblock-terminal->master: _frontend/cli.py: Exit with error if output streams are set to nonblocking) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1411 | 08:55 |
| tristan | And no plugin need ever call validate_node() explicitly | 08:55 |
| Kinnison | That'd need to be arbitrarily complex potentially | 08:55 |
| tristan | The core just does it in advance of calling Plugin.configure(), so anything not specified in the schema will cause an error; forcing plugin authors to update the schema | 08:55 |
| tristan | Kinnison, I agree, that's why I don't know if we have effort to allocate to it | 08:56 |
| *** jonathanmaw has joined #buildstream | 08:56 | |
| tristan | I mean... the current approach is "okay", but it just seems like an opportunity | 08:56 |
| tristan | benschubert, to be clear, I really don't mean to push for this to happen - just floating the idea to see how it's received | 08:57 |
| benschubert | tristan: Also, a 'valid' access to nodes through the yaml API would catch most of those later on right? | 08:57 |
| tristan | Right | 08:57 |
| benschubert | I would be worried that we add this and it slows us down more than we need but I like the idea | 08:57 |
| benschubert | I might have a shot at it but not in this PR | 08:58 |
| tristan | You mean performance wise or code wise ? | 08:58 |
| tristan | Both I suppose are risks | 08:58 |
| benschubert | right | 08:59 |
| tristan | But performance wise hopefully it would not be much... Assuming we expect that plugins call validate_node() on every dict/sub-dict anyway, it implies a dict traversal happens | 08:59 |
| tristan | Still it is complex I know :) | 08:59 |
| benschubert | correct | 08:59 |
| benschubert | I'll think about that | 08:59 |
| benschubert | once I'm done with this yaml rework | 09:00 |
| benschubert | I updated my MR which should have tests passing and addressed your comment tristan | 09:00 |
| benschubert | Does the public API on nodes seems good to you? | 09:00 |
| tristan | Hmmm, that's a much larger question :) | 09:02 |
| tristan | benschubert, do we need Node.get_provenance() if ProvenanceInformation() is public anyway ? | 09:03 |
| tristan | benschubert, maybe it would be easier if we got this into a clean organized and documented state before doing a full public API scan ? | 09:03 |
| benschubert | ok sure! | 09:03 |
| Kinnison | I imagine ProvenanceInformation will be becoming a private name | 09:04 |
| benschubert | Kinnison: I'm not sure, but we could | 09:04 |
| benschubert | but tristan is right, I'll reorganize everything first | 09:04 |
| tristan | I don't see why a plugin would want to call strip_node_info() also... | 09:05 |
| tristan | but yeah, I'll stop looking for now :) | 09:05 |
| benschubert | tristan: 'DynamicElement' in one of the tests is doing it | 09:05 |
| benschubert | but I dislike that | 09:06 |
| benschubert | I'll put this private | 09:06 |
| benschubert | and fix the test | 09:06 |
| benschubert | then merge this MR, reorganize, and move to 'Node' | 09:06 |
| benschubert | then let's discuss the rest | 09:06 |
| Kinnison | :D | 09:11 |
| gitlab-br-bot | juergbi opened MR !1465 (juerg/context->master: Make Context class a Python context manager) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1465 | 09:18 |
| *** lachlan has joined #buildstream | 09:19 | |
| tpollard | that MR name sounds intriguing | 09:27 |
| *** lachlan has quit IRC | 09:40 | |
| *** lachlan has joined #buildstream | 09:45 | |
| *** jswagner has quit IRC | 09:47 | |
| *** jswagner has joined #buildstream | 09:47 | |
| *** jswagner has joined #buildstream | 09:50 | |
| *** lachlan has quit IRC | 09:52 | |
| *** lachlan has joined #buildstream | 09:55 | |
| gitlab-br-bot | traveltissues opened issue #1073 (cache key dictionary changes) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1073 | 10:17 |
| gitlab-br-bot | BenjaminSchubert merged MR !1464 (bschubert/node-private-public->bschubert/new-node-api: Mark APIs on Node as private or public) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1464 | 10:18 |
| *** lachlan has quit IRC | 10:45 | |
| *** lachlan has joined #buildstream | 11:17 | |
| *** lachlan has quit IRC | 11:31 | |
| *** tiagogomes has quit IRC | 11:33 | |
| *** dftxbs3e has quit IRC | 12:09 | |
| *** dftxbs3e has joined #buildstream | 12:09 | |
| *** traveltissues_ has joined #buildstream | 12:24 | |
| *** traveltissues has quit IRC | 12:24 | |
| *** tiagogomes has joined #buildstream | 12:27 | |
| *** bochecha has quit IRC | 13:02 | |
| *** bozcan has joined #buildstream | 13:06 | |
| *** tristan has quit IRC | 13:10 | |
| gitlab-br-bot | BenjaminSchubert opened MR !1467 (bschubert/node-api-split->bschubert/new-node-api: _yaml: Split Node-related parts into 'node.pyx' and document) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1467 | 13:20 |
| *** lachlan has joined #buildstream | 13:23 | |
| *** tristan has joined #buildstream | 13:39 | |
| WSalmon | I benschubert, i think i have resolved all the issues on https://gitlab.com/BuildStream/buildstream/merge_requests/1429 if you dont have time to double check the ones i havent marked as resolved it think they should be ok but it would be great if you could have a quick look and confirm/update | 13:41 |
| WSalmon | * Hi benschubert, i think i have resolved all the issues on https://gitlab.com/BuildStream/buildstream/merge_requests/1429 if you dont have time to double check the ones i havent marked as resolved it think they should be ok but it would be great if you could have a quick look and confirm/update | 13:41 |
| *** lachlan has quit IRC | 13:45 | |
| *** lachlan has joined #buildstream | 13:53 | |
| *** lachlan has quit IRC | 13:56 | |
| *** lachlan has joined #buildstream | 14:01 | |
| *** lachlan has quit IRC | 14:05 | |
| WSalmon | hi juergbi, i have been thinking about https://gitlab.com/BuildStream/buildstream/merge_requests/951#note_177691574 re your point about testing, were you invisiging this api as stable enought for some unit test or going for our usual route of testing through full runs of bst? | 14:16 |
| WSalmon | I can think of a lot of edge cases which seem a bit waste full to be tested by a full end to end test, but i am happy to do full test if we need/want the overhead | 14:17 |
| juergbi | WSalmon: not sure. we already do have casbaseddirectory unit tests, so it could make sense to extend those | 14:24 |
| *** mohan43u has joined #buildstream | 14:24 | |
| WSalmon | oooh i didnt notice that, i will go look for them :D | 14:26 |
| WSalmon | Thanks juergbi | 14:26 |
| WSalmon | I think i will want one full test that dose what FDSDK dose but if i can do all the other edge cases in unit tests that seems like a good middle ground | 14:27 |
| *** lachlan has joined #buildstream | 14:56 | |
| *** slaf has quit IRC | 15:10 | |
| *** lachlan has quit IRC | 15:33 | |
| *** lachlan has joined #buildstream | 15:38 | |
| *** lachlan has quit IRC | 15:48 | |
| *** lachlan has joined #buildstream | 16:03 | |
| *** lachlan has quit IRC | 16:32 | |
| *** alexandrufazakas has quit IRC | 16:32 | |
| *** slaf has joined #buildstream | 16:34 | |
| *** slaf has joined #buildstream | 16:34 | |
| *** slaf has joined #buildstream | 16:34 | |
| *** slaf has joined #buildstream | 16:35 | |
| *** slaf has joined #buildstream | 16:35 | |
| *** slaf has joined #buildstream | 16:35 | |
| *** slaf has joined #buildstream | 16:36 | |
| *** bozcan has quit IRC | 16:37 | |
| *** bozcan_ has joined #buildstream | 16:37 | |
| *** bozcan_ is now known as bozcan | 16:37 | |
| *** lachlan has joined #buildstream | 16:41 | |
| gitlab-br-bot | traveltissues opened issue #1074 (update.py throws an error) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1074 | 16:43 |
| *** lachlan has quit IRC | 16:45 | |
| *** bozcan has quit IRC | 16:47 | |
| jennis | tristan, aevri, marge has been updated to the latest stable version; so if she is assigned with someone else she should still do her thing | 16:51 |
| jennis | Well, were were always using the latest stable version, but they just did a release :) | 16:51 |
| jennis | we were* | 16:51 |
| *** lachlan has joined #buildstream | 16:52 | |
| jennis | It should also include the timeout bug fix, so we're not expecting that we'll need to restart her as often (or hopefully not at all!) | 16:52 |
| aevri | Ace :) | 16:56 |
| *** lachlan has quit IRC | 16:57 | |
| *** jonathanmaw has quit IRC | 17:14 | |
| *** lachlan has joined #buildstream | 17:38 | |
| gitlab-br-bot | traveltissues opened (was WIP) MR !1468 (traveltissues/fix-update-script->master: Mock BST_TEST_SUITE env when running update.py) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1468 | 17:52 |
| *** lachlan has quit IRC | 17:52 | |
| *** traveltissues_ has quit IRC | 17:55 | |
| *** lachlan has joined #buildstream | 18:19 | |
| *** lachlan has quit IRC | 18:28 | |
| *** lachlan has joined #buildstream | 18:54 | |
| *** lachlan has quit IRC | 19:37 | |
| *** xjuan has joined #buildstream | 22:21 | |
| *** xjuan has quit IRC | 22:56 | |
| *** xjuan has joined #buildstream | 23:15 | |
| *** xjuan has quit IRC | 23:49 | |
| *** xjuan has joined #buildstream | 23:51 | |
| *** xjuan has quit IRC | 23:58 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!