| *** flatmush has quit IRC | 02:02 | |
| *** bochecha has joined #buildstream | 07:51 | |
| *** alexandrufazakas has joined #buildstream | 08:08 | |
| *** tiagogomes has joined #buildstream | 08:11 | |
| *** benschubert has joined #buildstream | 08:19 | |
| *** tristan has quit IRC | 08:33 | |
| *** tristan has joined #buildstream | 08:38 | |
| *** flatmush has joined #buildstream | 09:02 | |
| *** jonathanmaw has joined #buildstream | 09:02 | |
| *** phil has joined #buildstream | 09:04 | |
| raoul | juergbi, not been able to get buildbox-integration to work with buildstream master, I changed the freedesktop junction to track the branch that works with bst2 and tracked that, but it complains about "Destination is a symlink, not a directory: /usr/sbin" | 09:07 |
|---|---|---|
| juergbi | raoul: for which element do you get this message? | 09:08 |
| raoul | and if you try and source track everything else you get a _yaml.pyx error which may be bschubert domain | 09:08 |
| raoul | juergbi, on buildbox-fuse.bst | 09:08 |
| juergbi | odd that it wants to install into /usr/sbin | 09:09 |
| juergbi | it's supposed to install into cmake's BINDIR, which should be /usr/bin | 09:10 |
| juergbi | can you paste more log lines somewhere? | 09:11 |
| juergbi | ah, no, that's for a dependency, of course | 09:11 |
| juergbi | I'd guess fuse3 | 09:11 |
| raoul | https://pastebin.com/VW911gBY | 09:12 |
| raoul | It might be that the elements need to be updated, but with source track not working I can't do that | 09:12 |
| juergbi | need to set sbindir to "%{exec_prefix}/bin" | 09:13 |
| juergbi | in project.conf | 09:13 |
| juergbi | (or simply to "%{bindir}") | 09:13 |
| benschubert | raoul: could you show me the error please? | 09:14 |
| * tlater[m] wonders if doing sbindir -> bindir is a form of heresy | 09:14 | |
| raoul | it seems to be building now juergbi, thank! | 09:14 |
| benschubert | (a gitlab snippet would be better than a pastebin, I don't have access do it) | 09:14 |
| raoul | benschubert, one sec | 09:14 |
| juergbi | tlater[m]: I don't think the distinction really helps and it's sometimes harmful. but anyway this is just about being in sync with fdo-sdk | 09:17 |
| tlater[m] | juergbi: I wasn't implying there's anything bad about heresy, although i suppose it isn't heresy if fdo-sdk does it | 09:19 |
| raoul | there you go benschubert https://gitlab.com/snippets/1871447 | 09:19 |
| tlater[m] | benschubert: While you're here, i was wondering if you'd have recommendations for a "proper" way to compose two lists | 09:20 |
| tlater[m] | I'd create one of those synthetic nodes and compose them, but that feels dirty | 09:20 |
| benschubert | raoul: wut, we had tests for this one Oo Do you have a way I could reproduce on my end? Or a test that triggers? | 09:20 |
| benschubert | tlater[m]: you have two lists that you want to compose on a third one, is that it? | 09:21 |
| raoul | benschubert, I've only had this happen when trying to build buildbox | 09:21 |
| tlater[m] | Not quite, I just have two lists that may or may not be compose_lists, and I'd like to compose them into one. | 09:21 |
| tlater[m] | By that I mean, sometimes they'll be _yaml.Nodes, and sometimes [] | 09:22 |
| jonathanmaw | tristan: I've reworked !1409 again, have you had time to look at it? | 09:22 |
| gitlab-br-bot | MR !1409: Separate frontend state handling from core state https://gitlab.com/BuildStream/buildstream/merge_requests/1409 | 09:22 |
| benschubert | raoul: could you add the reproduction info on the snippet? i'll have a look at it | 09:23 |
| jonathanmaw | one thing that stands out right now is that I've written callbacks for changes to queue state, but since the frontend renders the queues periodically and doesn't cache anything about queue state, nothing subscribes to the callback | 09:23 |
| benschubert | tlater[m]: afaict we can only compose dicts nodes from the API | 09:24 |
| tlater[m] | benschubert: That's what I've gathered, just double checking I'm not missing something | 09:24 |
| benschubert | Yep | 09:24 |
| tlater[m] | I'll go with a synthetic node for now then - thanks! | 09:25 |
| benschubert | if you can make an issue with your usecase otherwise :) | 09:25 |
| benschubert | since I'm rewriting the API | 09:25 |
| tlater[m] | benschubert: It's related to this: https://gitlab.com/BuildStream/buildstream/issues/1061, but I suppose I'll make a new issue and write up exactly what I need | 09:26 |
| benschubert | thanks! | 09:27 |
| raoul | hmm benschubert I seem to get a slightly different error running it on my laptop compared to running on the desktop I was earlier, though the error is still at the same line... | 09:35 |
| raoul | updated the description on the snippet though | 09:35 |
| benschubert | thanks a lot! | 09:36 |
| *** ChanServ sets mode: +o tristan | 09:36 | |
| tristan | jonathanmaw, Not yet, I guess I should... I anticipate this one to be the winner ! | 09:36 |
| jonathanmaw | \o/ | 09:37 |
| raoul | Ah juergbi, seems that it just rebuilt grpc, but then on buildbox-fuse.bst, it still gives "FAILURE Destination is a symlink, not a directory: /usr/sbin" , with either sbindir set to exec_prefix/bin or bindir | 09:41 |
| *** lachlan has joined #buildstream | 09:42 | |
| juergbi | looking at the log it seems to be a util-linux issue | 09:43 |
| juergbi | maybe check with bst artifact checkout the structure of util-linux | 09:44 |
| juergbi | hm, fdo-sdk util-linux already seems to have a workaround for sbin | 09:45 |
| *** Kinnison has joined #buildstream | 09:45 | |
| juergbi | ah, we have our own util-linux | 09:46 |
| juergbi | maybe copy the install-command addition from fdo-sdk or use another way to avoid /usr/sbin | 09:47 |
| raoul | Shall have a go, though this is not stuff I'm too familiar with | 09:48 |
| tristan | benschubert, I know you have some opposite opinion, but as I am in mid-review I'd like to point out https://gitlab.com/BuildStream/buildstream/merge_requests/1409/diffs#8335eb935d94813ec3b720a67bd88e5c278daed5_70_70 | 10:07 |
| tristan | benschubert, What bothers me about that line... is that this new Job.name member *belongs* to Job, yet we have a subclass ElementJob *assigning* Job.name without going through any explicit mutator, meaning (A) It is not clear when reading the ElementJob code that `name` belongs to the parent, as there is no `set_name()` function being used... and (B) Job base class cannot be informed about changes of the name | 10:09 |
| tristan | (B) can be resolved by using properties, yeah, but it still doesnt really make (A) any more clear... that statement is naturally mixed with other statements which assign local variables which clearly *do* belong to ElementJob | 10:10 |
| *** lachlan has quit IRC | 10:10 | |
| * tristan will open up a comment there | 10:12 | |
| * tristan adds comment https://gitlab.com/BuildStream/buildstream/merge_requests/1409/diffs#note_187919656 | 10:14 | |
| gitlab-br-bot | tlater opened issue #1062 (Add API for composing lists in yaml nodes) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1062 | 10:14 |
| *** lachlan has joined #buildstream | 10:15 | |
| tlater[m] | benschubert: What do you think about that issue? I wouldn't mind changing `_yaml.get_node` according to my suggestion there as part of #1601 | 10:17 |
| tlater[m] | #1061, damnit | 10:17 |
| gitlab-br-bot | Issue #1061: Allow specifying element-plugin-wise default dependencies https://gitlab.com/BuildStream/buildstream/issues/1061 | 10:17 |
| *** lachlan has quit IRC | 10:24 | |
| *** traveltissues has joined #buildstream | 10:27 | |
| tristan | jonathanmaw, finished with comments... really nice stuff ! | 10:37 |
| *** lachlan has joined #buildstream | 10:38 | |
| gitlab-br-bot | juergbi opened MR !1446 (juerg/sourcecache->master: element.py: Do not implicitly import sources into source cache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1446 | 10:42 |
| *** lachlan has quit IRC | 10:42 | |
| juergbi | raoul: ^^ is a tiny source cache MR, in case you have a minute for a review at some point | 10:44 |
| gitlab-br-bot | raoul.hidalgocharman approved MR !1446 (juerg/sourcecache->master: element.py: Do not implicitly import sources into source cache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1446 | 10:50 |
| juergbi | ta | 10:50 |
| raoul | Provided it passes the pipeline LGTM | 10:50 |
| *** lachlan has joined #buildstream | 10:54 | |
| benschubert | tristan: I have no strong opinion on setting values, my strong opinion is against getters :) So I'm happy with this being changed to a set_name() | 10:55 |
| benschubert | tlater[m]: actually, if we were to compose 'autotools' from project on ROOT from the element, I think that would work as expected :) | 10:58 |
| *** lachlan has quit IRC | 10:59 | |
| tlater[m] | benschubert: I actually got that the wrong way around | 10:59 |
| tlater[m] | o\ | 10:59 |
| tlater[m] | The (>): should be in the elements dependencies | 11:00 |
| tlater[m] | That said, even if it wasn't, I feel like that's risky, since `autotools:` can contain other things as well | 11:00 |
| tlater[m] | We'd clobber things before the elment is properly loaded | 11:00 |
| benschubert | tlater[m]: I would not have this added in elements | 11:00 |
| benschubert | At least that's not how I understood the whole ML thread | 11:00 |
| benschubert | I had the understanding that you would not need to specify anything in your element | 11:01 |
| tlater[m] | benschubert: This is what I'm basing it on: https://mail.gnome.org/archives/buildstream-list/2019-June/msg00028.html | 11:01 |
| tristan | benschubert, Ah ok gotcha | 11:01 |
| tlater[m] | You still need to specify some dependencies in your element | 11:01 |
| tlater[m] | If they aren't just the default ones | 11:01 |
| tlater[m] | (i.e., foo-lib.bst) | 11:02 |
| benschubert | tristan: basically, having a getter in python is slow compared to accessing directly, so I would rather avoid it. For setters, it makes things more explicit so I'm more fine with it :) | 11:02 |
| benschubert | tlater[m]: yep, so we could have something like: Element.dependencies = my_dependencies + Element.kind.dependencies, which is read from another file or something like this? | 11:03 |
| benschubert | tlater[m]: I think we should avoid having to add information in the bst files about default, because that defeats the purpose of having them as default :) | 11:03 |
| tlater[m] | benschubert: But that's not what's happening here | 11:03 |
| tlater[m] | project.conf contains all the default dependencies (in this case autotools.bst) | 11:04 |
| tlater[m] | The element appends to that list, not knowing what it contains | 11:04 |
| tlater[m] | (With (>)) | 11:04 |
| benschubert | tlater[m]: I'm just not sure we need to handle that through yaml | 11:04 |
| *** lachlan has joined #buildstream | 11:05 | |
| benschubert | and I don't think the element should append anything to any list | 11:05 |
| *** tristan has quit IRC | 11:05 | |
| tlater[m] | Yeah, that's just an example to place it into the current structure of the API, eventually I'd make that implicit | 11:05 |
| benschubert | tlater[m]: I think sander's answer on https://mail.gnome.org/archives/buildstream-list/2019-June/msg00030.html is on point | 11:05 |
| tlater[m] | As for not handling this through yaml... | 11:05 |
| benschubert | tlater[m]: the concatenation I mean :) | 11:06 |
| tlater[m] | benschubert: From that emal: > Make error free append the default operation (even when it is not specified). | 11:06 |
| tlater[m] | Which requires going through the actual YAML layer, and defining a new "error free append" operation :) | 11:06 |
| tlater[m] | (Defined by tristan as (->)) | 11:07 |
| benschubert | tlater[m]: my concern is, I don't want to have to specify a (->) in every element I'm writing, because that defeats the purpose and is not better than what we have today. | 11:08 |
| tlater[m] | benschubert: Yes! For that reason, it would be implicit | 11:08 |
| benschubert | tlater[m]: then why go through yaml for this? | 11:09 |
| benschubert | If that's implicit and done in the code, we could just get the two lists and append them ourselves | 11:09 |
| tlater[m] | Because then we can use (=) to avoid the default dependencies when we need to | 11:09 |
| tlater[m] | It'd be a shame if we didn't use the strengths of composition here. | 11:09 |
| benschubert | I'd rather go with a 'no_default_dependencies' flag then, this seems too much like magic to me | 11:10 |
| tlater[m] | benschubert: https://mail.gnome.org/archives/buildstream-list/2019-June/msg00031.html defines that syntax | 11:10 |
| tlater[m] | And I personally think the (=) is less magic | 11:11 |
| tlater[m] | Given that it's how everything else works | 11:11 |
| tlater[m] | No need to learn a new syntax for a concept you already need | 11:11 |
| * tlater[m] doesn't mind just drafting this for now and seeing what opinions are on an actual MR/mailing thread | 11:12 | |
| tlater[m] | But for now I'm sticking to what that email thread decided | 11:12 |
| gitlab-br-bot | marge-bot123 merged MR !1446 (juerg/sourcecache->master: element.py: Do not implicitly import sources into source cache) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1446 | 11:14 |
| benschubert | tlater[m]: no that's fine, let's go for how the message defines it, thanks | 11:15 |
| tlater[m] | Given that, how do you think should I handle it? I still feel just blanket writing to element's root is a poor idea | 11:18 |
| benschubert | tlater[m]: if we node_sanitize first to ensure we have the only the keys we expect, it would be ok | 11:18 |
| tlater[m] | Ah, I suppose I was missing node_sanitize | 11:18 |
| * tlater[m] reads what that does | 11:18 | |
| benschubert | it actually verifies that you don't ahve extra keys | 11:19 |
| *** lachlan has quit IRC | 11:20 | |
| tlater[m] | I think you mean node_validate? | 11:20 |
| benschubert | oups sorry yes | 11:20 |
| tlater[m] | Right, so the problem with that is that that syntax is already in use | 11:20 |
| tlater[m] | So we expect other keys | 11:20 |
| tlater[m] | benschubert: i.e.: https://gitlab.com/freedesktop-sdk/freedesktop-sdk/blob/master/project.conf#L360 | 11:22 |
| benschubert | ah shoot | 11:22 |
| tlater[m] | Obviously, we want to compose those things too | 11:22 |
| tlater[m] | But that happens later | 11:22 |
| tlater[m] | Because loading elements is complex :) | 11:22 |
| benschubert | yay | 11:23 |
| benschubert | I'm not sure what's best there | 11:23 |
| tlater[m] | What do you think about the suggestion of adding a special _yaml.node type expectation for lists? | 11:23 |
| tlater[m] | I find that quite intuitive, and it's not destructive because it's not possible currently | 11:23 |
| benschubert | type expectation fo list? | 11:23 |
| tlater[m] | We can get provenance info for that node_get pretty easily | 11:24 |
| tlater[m] | So, use `_yaml.node_get(list_node, _yaml.Node, "key")` | 11:24 |
| tlater[m] | Which would internally return a pretty `_yaml.Node` with a [] inside it | 11:24 |
| tlater[m] | So we can make the `composite()` deal with it properly | 11:25 |
| benschubert | composite() expects two mappings, not lists, no? | 11:25 |
| tlater[m] | benschubert: Yes, so the functionality we want for !1061 would be added to `composite()` and do its thing | 11:26 |
| gitlab-br-bot | MR !1061: BREAK:remove unconditional 'are you sure?' prompts https://gitlab.com/BuildStream/buildstream/merge_requests/1061 | 11:26 |
| tlater[m] | I.e., by default safe-append or whatever | 11:26 |
| tlater[m] | Then we make safe-append the default operation for all composed lists (if we ever allow any others), and the magic goes away | 11:27 |
| tlater[m] | Alternatively keep that a special case for depends, of course, but well, this is more about programming architecture anyway | 11:28 |
| benschubert | I would not have the safe-append the default but the rest seems good :) | 11:28 |
| tlater[m] | Cool, I'll just treat that as your seal of approval for now, ta benschubert :) | 11:29 |
| benschubert | haha, just let me know if you need to change something in _yaml, as I'm reworking the entire API | 11:29 |
| tlater[m] | Sure, I'll keep it as localized as I can | 11:30 |
| tlater[m] | We'll have a hacking session soon anyway, I'm pretty much only preparing a draft so people can see how this looks in practice | 11:31 |
| *** lachlan has joined #buildstream | 11:37 | |
| *** rdale has quit IRC | 11:46 | |
| *** lachlan has quit IRC | 11:51 | |
| *** lachlan has joined #buildstream | 11:51 | |
| *** tristan has joined #buildstream | 11:58 | |
| *** lachlan has quit IRC | 12:02 | |
| *** lachlan has joined #buildstream | 12:08 | |
| *** lachlan has quit IRC | 12:14 | |
| *** rdale has joined #buildstream | 12:36 | |
| *** tristan has quit IRC | 12:37 | |
| benschubert | right, I might be able to push my yaml changes over the line this time :D | 12:38 |
| benschubert | raoul: I tried the steps you outlined with the laster buildstream master and... everything worked :/ | 12:38 |
| raoul | hmm strange, I was definitely on the latest master | 12:39 |
| benschubert | raoul: and latest master on the other repo too? | 12:41 |
| *** tristan has joined #buildstream | 12:41 | |
| *** rdale has quit IRC | 12:42 | |
| raoul | yep definitely | 12:49 |
| gitlab-br-bot | BenjaminSchubert opened MR !1447 (bschubert/rework-node-creations->bschubert/new-node-api: Rework synthetic node creation) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1447 | 12:50 |
| benschubert | raoul: mmh weird, could you show that to me tomorrow/on friday? | 12:52 |
| benschubert | actually friday would be good :) | 12:52 |
| raoul | Yeah will do :) | 12:53 |
| gitlab-br-bot | BenjaminSchubert merged MR !1444 (bschubert/node-api-noassertions->bschubert/new-node-api: _yaml: Move 'node_final_assertions' to 'Node._assert_fully_composited') on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1444 | 12:54 |
| *** lachlan has joined #buildstream | 13:27 | |
| *** lachlan has quit IRC | 13:43 | |
| *** lachlan has joined #buildstream | 13:50 | |
| *** lachlan has quit IRC | 13:56 | |
| jonathanmaw | tristan: good thing I added that assertion checking whether I'm creating the same TaskGroup twice, looks like I'm creating FetchQueues multiple times | 14:06 |
| jonathanmaw | which seems to be because we're making the Stream do a scheduler run to fetch subprojects | 14:07 |
| jonathanmaw | Given it specifically swaps out the queues, I gather having multiple FetchQueues exist at once is normal | 14:08 |
| *** ChanServ sets mode: +o tristan | 14:08 | |
| tristan | jonathanmaw, So forgetting to remove them after the first run perhaps ? | 14:08 |
| tristan | Oh ? | 14:08 |
| tristan | I don't see why you would have multiple fetch queues at the same time no | 14:08 |
| jonathanmaw | hmm, that's an interesting one, then | 14:09 |
| tristan | jonathanmaw, it's possible juergbi did something a bit recursive ? but I would think it is preferable to fetch the subprojects you know that need fetching... and then after completing that operation, finding out all the suprojects you need to fetch and then fetch those... and so on, until all subprojects are fetched | 14:10 |
| tristan | I wouldnt expect recursively creating new fetch queues | 14:10 |
| tristan | Although it perhaps works that way for right now | 14:10 |
| tristan | jonathanmaw, it's also fairly possible that you have some object reference pointing to Queue objects, causing them to never have their __del__ methods run | 14:11 |
| * tristan wonders if __del__ is related to the GC also | 14:12 | |
| jonathanmaw | yeah, I just had a look at which queues were swapped out, and it was an empty list of queues, so something else is the culprit this time | 14:12 |
| * tristan would expect (hope) __del__ to be run immediately as soon as an object falls out of scope, but it might not be the case ? | 14:13 | |
| jonathanmaw | yeah, looks like the queue wasn't being dereferenced | 14:15 |
| jonathanmaw | In Stream.fetch_subprojects(), if I explicitly call self.queues.clear() once I've finished the scheduler run, the tests pass | 14:15 |
| *** bochecha has quit IRC | 14:15 | |
| *** bochecha has joined #buildstream | 14:16 | |
| benschubert | tristan: __del__ is not called directly. It's called one the GC decides to remove the object | 14:16 |
| jonathanmaw | I think the problem is probably that both the Stream and the Scheduler hold the list of queues, so when Stream starts constructing the next list of queues, there's still some in the Scheduler | 14:17 |
| jonathanmaw | though if we remove the queues immediately after a scheduler run, we won't be able to include them in the summary we print at the end of a run | 14:18 |
| tpollard | It might not be relevant but I did notice that the buildqueue directly calls the schedulers private callback for job complete instead of going through the public job_completed() which does some extra bits | 14:23 |
| tristan | jonathanmaw, Maybe we want to rethink that in that case, as benschubert points out | 14:24 |
| *** lachlan has joined #buildstream | 14:24 | |
| tristan | jonathanmaw, We should not rely on __del__ for unregistering TaskGroups | 14:24 |
| jonathanmaw | hmm, similarly, I think the safest way to handle the existence of more than one Queue of the same type would probably be to not create TaskGroups when we're using a FetchQueue to fetch subprojects (i.e. that scheduler run is not reported to the frontend) | 14:26 |
| benschubert | could we have a message from the scheduler that a previous queue was deleted? Would that fix the problem? Because we know when queues are changed | 14:27 |
| tristan | benschubert, jonathanmaw https://gitlab.com/BuildStream/buildstream/merge_requests/1409/diffs#note_188065128 | 14:35 |
| tristan | benschubert, essentially we *do* have a notification, it's just not very explicitly invoked | 14:35 |
| tristan | I don't think it should be a Message(), though | 14:35 |
| tristan | The Message objects and the ongoing tasks and task groups are pretty separate | 14:36 |
| tristan | the latter is basically just "what is the current state", with notifications | 14:36 |
| benschubert | tristan: if we end up with the 'frontend' in a separate process, we'll need to have a Message for it no? Otherwise seems good to me! | 14:37 |
| benschubert | and I confirm, the queue's __del__ will only be called once it's not referenced by anything else, so what we currently have doesn't work :) | 14:37 |
| tristan | benschubert, I don't know that we'll need a *Message* as in _message.py per se... but indeed the code that jonathanmaw is adding in _state.py will need to be split in two, and some IPC will need to be crafted | 14:38 |
| tristan | Whether that leverages Message objects or not is really part of the scheduling subprocessing implementation details | 14:38 |
| benschubert | that seems good to me then. I haven't looked too much at the MR, but I can have a more in depth look if needed :) | 14:38 |
| jonathanmaw | tristan: so, we should explicitly unregister TaskGroups, but not necessarily do it at the end of every scheduler run, since we want them to hang around from the last Scheduler run so we can include them in the summary | 14:39 |
| tristan | jonathanmaw, Yeah... I've sort of left that detail to you :) | 14:39 |
| tristan | jonathanmaw, There are many ways to fry that cat... one way I can think of... is that it might be nice if the Stream() invocations *return* the task groups as they were at the time scheduling stopped, and to clear them then | 14:40 |
| tristan | for that purpose | 14:40 |
| tristan | That might be unweildy I'm not sure | 14:40 |
| tristan | jonathanmaw, remember that the frontend is only really interested in a specific scheduler run which is associated to the "session" which completed | 14:41 |
| tristan | jonathanmaw, Maybe as you say, we want to instead avoid clearing the queues until the moments that we setup new ones | 14:41 |
| tristan | In any case... we do have Stream in control of all of this, it creates the Queues and assigns them to the scheduler... it should be the one to clear the queues I guess | 14:42 |
| jonathanmaw | It sounds like the most straight-forward answer is that out-of-session scheduler runs shouldn't register TaskGroups | 14:42 |
| tristan | Hmm | 14:43 |
| tristan | jonathanmaw, I don't think that is right | 14:43 |
| tristan | Better to communicate all the time accurately what is going on | 14:44 |
| tristan | let the frontend decide what it wants to display | 14:44 |
| tristan | jonathanmaw, I would expect that if there are multiple scheduler runs, we should still be able to see consistent feedback about what is happening in the UI while things are ongoing | 14:44 |
| tristan | jonathanmaw, For now... I think most use cases do not involve more than one TaskGroup being present at a given time... mostly it is just the "fetch" operations, but I wouldnt want to paint us into a corner where that is a restriction | 14:45 |
| jonathanmaw | hmm, so for the meantime there's no reason why we *should* see multiple Queues of the same type at once, but hypothetically if there was a reason to, the most appropriate solution would be to pop all task groups out of State temporarily | 14:46 |
| tristan | Uhhhh, this is two separate topics | 14:46 |
| jonathanmaw | or perhaps display both sets of TaskGroups in parallel | 14:46 |
| jonathanmaw | hmm | 14:46 |
| tristan | jonathanmaw, I don't think there will ever be a time we want to see two task groups *of the same type* | 14:47 |
| tristan | jonathanmaw, rather: whenever the scheduler is running "There might be task groups" | 14:47 |
| tristan | From what I understand, there also might be jobs even in the absence of task groups | 14:48 |
| tristan | For instance... the load process will be turned into tasks that are not part of any task group | 14:48 |
| tristan | jonathanmaw, Anyway... what I understand is that you are running into a situation where the Fetch "task group" from a junction fetch job doesnt disappear when you later add a new Fetch "task group" | 14:49 |
| tristan | And that is probably because of using __del__ | 14:50 |
| tristan | Or neglecting to clear the queues at some point | 14:50 |
| jonathanmaw | yeah, the queues aren't getting cleared before a new FetchQueue is created, by the looks of it | 14:50 |
| tristan | Right, so first (A) Don't use __del__.... and second (B) Make sure there is an explicit codepath in Stream which is clearing the queues before populating the queues for a subsequent run... follow that codepath and decide where is the appropriate place to unregister the task group | 14:52 |
| tristan | jonathanmaw, I suspect that an explicit destructor function on Queue() would be appropriate for this | 14:52 |
| tristan | maybe Queue.destroy(), which needs to be called from the right place... maybe from a new function like Scheduler.clear_queues() or Scheduler.reset() | 14:53 |
| tristan | (if an appropriate place doesnt exist yet) | 14:53 |
| *** traveltissues has quit IRC | 14:53 | |
| *** lachlan has quit IRC | 14:57 | |
| *** tristan has quit IRC | 15:09 | |
| gitlab-br-bot | BenjaminSchubert opened MR !1448 (bschubert/node-no-isnode->bschubert/new-node-api: _yaml: Remove 'is_node', which is not used in the codebase anymore) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1448 | 15:17 |
| *** phil has quit IRC | 15:27 | |
| *** phil has joined #buildstream | 15:28 | |
| gitlab-br-bot | shashwatdalal opened (was WIP) MR !1441 (shashwatdalal/tar-file-extraction->master: utils.py: Change permissions for rootpath in `_force_rmtree`) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1441 | 15:34 |
| benschubert | juergbi, tristan, would one of you have time to just double check that !1441 would not have side effects that we might have overlooked? should be fairly quick to review | 15:35 |
| *** lachlan has joined #buildstream | 15:46 | |
| *** lachlan has quit IRC | 15:51 | |
| *** traveltissues has joined #buildstream | 15:57 | |
| gitlab-br-bot | BenjaminSchubert merged MR !1442 (bschubert/new-node-compose->bschubert/new-node-api: Rework node compose API) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1442 | 15:58 |
| *** lachlan has joined #buildstream | 16:06 | |
| *** lachlan has quit IRC | 16:14 | |
| *** bochecha has quit IRC | 16:15 | |
| *** tpollard has quit IRC | 16:15 | |
| *** phil has quit IRC | 16:24 | |
| *** lachlan has joined #buildstream | 16:27 | |
| *** alexandrufazakas has quit IRC | 16:29 | |
| *** lachlan has quit IRC | 16:34 | |
| *** david has joined #buildstream | 16:36 | |
| *** tristan has joined #buildstream | 16:47 | |
| *** bethw has quit IRC | 17:01 | |
| *** WSalmon has quit IRC | 17:01 | |
| *** johnward has quit IRC | 17:01 | |
| *** laurence has quit IRC | 17:01 | |
| *** jennis has quit IRC | 17:01 | |
| *** valentind has quit IRC | 17:01 | |
| *** jennis has joined #buildstream | 17:02 | |
| *** johnward has joined #buildstream | 17:02 | |
| *** laurence has joined #buildstream | 17:02 | |
| *** samkirkham has quit IRC | 17:04 | |
| *** adds68 has quit IRC | 17:04 | |
| *** ikerperez has quit IRC | 17:04 | |
| *** Becky has quit IRC | 17:04 | |
| *** lachlan has joined #buildstream | 17:04 | |
| *** WSalmon has joined #buildstream | 17:05 | |
| *** bethw has joined #buildstream | 17:06 | |
| *** tiagogomes has quit IRC | 17:06 | |
| benschubert | tristan: https://gitlab.com/BuildStream/buildstream/merge_requests/1447 is another tricky one and I'd like your input on it if you have time | 17:06 |
| *** valentind has joined #buildstream | 17:06 | |
| *** lachlan has quit IRC | 17:12 | |
| gitlab-br-bot | BenjaminSchubert opened MR !1450 (bschubert/node-api-novalidate->bschubert/new-node-api: _yaml: Remove 'node_validate' and replace by 'MappingNode.validate_keys') on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1450 | 17:14 |
| *** lachlan has joined #buildstream | 17:23 | |
| *** lachlan has quit IRC | 17:27 | |
| *** WSalmon has quit IRC | 17:33 | |
| *** laurence has quit IRC | 17:33 | |
| *** WSalmon has joined #buildstream | 17:33 | |
| *** jennis has quit IRC | 17:33 | |
| *** laurence has joined #buildstream | 17:34 | |
| *** jennis has joined #buildstream | 17:34 | |
| *** jonathanmaw has quit IRC | 17:40 | |
| *** lachlan has joined #buildstream | 17:43 | |
| *** lachlan has quit IRC | 17:47 | |
| *** paulsherwood has quit IRC | 17:52 | |
| *** benbrown has quit IRC | 17:52 | |
| gitlab-br-bot | LaurenceUrhegyi closed issue #477 (Allow rebuilding of a single element) on buildstream https://gitlab.com/BuildStream/buildstream/issues/477 | 17:53 |
| *** lachlan has joined #buildstream | 18:11 | |
| *** lachlan has quit IRC | 18:15 | |
| *** traveltissues has quit IRC | 18:18 | |
| *** lachlan has joined #buildstream | 18:25 | |
| *** lachlan has quit IRC | 18:28 | |
| *** bochecha has joined #buildstream | 18:38 | |
| *** lachlan has joined #buildstream | 18:43 | |
| *** lachlan has quit IRC | 18:47 | |
| *** lachlan has joined #buildstream | 19:49 | |
| *** lachlan has quit IRC | 19:51 | |
| *** raoul has quit IRC | 19:56 | |
| *** bochecha has quit IRC | 20:48 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!