| *** narispo has joined #buildstream | 00:14 | |
| *** narispo has quit IRC | 03:33 | |
| *** narispo has joined #buildstream | 03:48 | |
| *** narispo has joined #buildstream | 04:04 | |
| *** narispo has joined #buildstream | 04:21 | |
| *** narispo has quit IRC | 04:21 | |
| *** phildawson-ct has joined #buildstream | 04:26 | |
| *** narispo has joined #buildstream | 05:22 | |
| *** traveltissues has joined #buildstream | 06:38 | |
| *** narispo has joined #buildstream | 06:43 | |
| *** traveltissues has quit IRC | 06:43 | |
| *** narispo has quit IRC | 07:05 | |
| *** narispo has joined #buildstream | 07:08 | |
| *** phildawson-ct has quit IRC | 09:04 | |
| *** phoenix has joined #buildstream | 09:26 | |
| *** phoenix has quit IRC | 09:32 | |
| *** santi has joined #buildstream | 09:40 | |
| *** bochecha has joined #buildstream | 09:43 | |
| *** tiagogomes has joined #buildstream | 09:52 | |
| *** tme5 has joined #buildstream | 10:21 | |
| *** lachlan has joined #buildstream | 10:31 | |
| tme5 | is it possible someone could look at !1765 today? | 10:37 |
|---|---|---|
| gitlab-br-bot | MR !1765: Make Git source plugin clone submodules recursively https://gitlab.com/BuildStream/buildstream/merge_requests/1765 | 10:37 |
| benschubert | tme5: on it! | 10:44 |
| laurence | have we seen any issues with Marge Bot recently at all? | 10:47 |
| laurence | and hello everyone :) | 10:47 |
| *** lachlan has quit IRC | 10:49 | |
| benschubert | Hey laurence ! We did have quite a bunch yes, I'm not sure whether they are resolved | 10:50 |
| benschubert | tme5: Just to make sure I understand everything: submodules never get instantiated as a `GitSourceBase` right? | 10:51 |
| laurence | ok, reading into things a bit more i think it's an upstream problem, not sure i can actually help | 10:51 |
| laurence | sorry for the noise | 10:51 |
| benschubert | ah no worries :) | 10:51 |
| *** phoenix has joined #buildstream | 10:52 | |
| gitlab-br-bot | jjardon opened issue #1240 (Commands to disable remote cache are being ignored / they do not work) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1240 | 10:59 |
| gitlab-br-bot | BenjaminSchubert opened MR !1768 (bschubert/cleanup-element-fetch->master: Small cleanups in `element._fetch`) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1768 | 11:02 |
| tme5 | benschubert, that;s correct | 11:04 |
| benschubert | tme5: so they just get a fetcher and that's all right? | 11:04 |
| benschubert | I meant a GitMirror | 11:05 |
| tme5 | yes, that is consistent with previous behaviour | 11:05 |
| benschubert | tme5: great! Then I had a few more nits that I added, otherwise LGTM :) | 11:08 |
| benschubert | juergbi: with !1768 I'm not breaking cache expiry right? | 11:09 |
| tme5 | thank you | 11:10 |
| tme5 | i don't understand your point on my function naming but i will change it to your suggestion haha | 11:10 |
| benschubert | which one? :) | 11:11 |
| benschubert | https://gitlab.com/BuildStream/buildstream/merge_requests/1765/diffs?diff_id=66923530#d702e4184719212f721d6d7de27b95489ec90675_729_712 ? | 11:11 |
| tme5 | ignoring_submodule, but as i say i will take your suggestion | 11:11 |
| jjardon | benschubert: laurence pretty sure the problems you have with marge is https://github.com/smarkets/marge-bot/issues/240, so upstream issue https://gitlab.com/gitlab-org/gitlab/issues/38330 (we have problems with merging MRs in fdsdk as well) | 11:12 |
| *** lachlan has joined #buildstream | 11:13 | |
| benschubert | jjardon: oh thanks for the heads up I'll watch this one | 11:13 |
| tme5 | it is just difficult sometimes, to make MRs, because the standard for merging code is sometimes so much higher than that of the existing code | 11:13 |
| tme5 | obviously ik that is essential to improve things | 11:14 |
| benschubert | tme5: ah sorry about that :/ I know that I tend to do this. Don't hesitate to say that it's out of scope if you believe it is and then don't address the comment if you believe it's not important | 11:15 |
| benschubert | Also, (at least for me) when I `nit:` something, it's a nitpick, basically feel free to pick tthem or not | 11:15 |
| *** lachlan has quit IRC | 11:26 | |
| tme5 | it's alright, it's important to improve things, and well, i don't want to be difficult haha | 11:27 |
| *** phoenix has quit IRC | 11:27 | |
| benschubert | It's important but if it prevents you from fixing a more important bug, it can be put aside for later ;) | 11:29 |
| * tlater[m] is finally successfully running buildstream under nix :) | 11:31 | |
| tlater[m] | Needs a few hacks, unfortunately: https://gitlab.com/BuildStream/buildstream/snippets/1922223 | 11:31 |
| *** lachlan has joined #buildstream | 11:35 | |
| *** lachlan has quit IRC | 11:39 | |
| benschubert | tlater[m]: I believe https://gitlab.com/BuildStream/buildstream/merge_requests/1739/diffs?commit_id=8f3584e8840c43492fc61b3e1dbc577ec0deb7f8 should be enough :) | 11:41 |
| benschubert | for what we were discussing yesterday | 11:42 |
| *** lachlan has joined #buildstream | 11:42 | |
| *** lachlan has quit IRC | 11:47 | |
| tlater[m] | benschubert: Yup, that looks right | 11:48 |
| benschubert | tlater[m]: does https://gitlab.com/BenjaminSchubert/bst-benchmarks/merge_requests/15 seem sensible to you? | 11:48 |
| tlater[m] | benschubert: Pff, I'm useless at reviewing that. It looks good to me, assuming we can assert fetching doesn't just measure our network speed. | 11:53 |
| * tlater[m] doesn't actually know how the configs work | 11:53 | |
| benschubert | tlater[m]: oh it's a local source for everything | 11:53 |
| tlater[m] | Cool, yes, in that case it makes sense :) | 11:54 |
| benschubert | thanks | 11:54 |
| *** lachlan has joined #buildstream | 11:59 | |
| gitlab-br-bot | tlater opened MR !1769 (tlater/CASdiff->master: storage/_casbaseddirectory.py: Add `diff` methods) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1769 | 12:09 |
| tlater[m] | WSalmon: Since you were part of that chat, mind taking a quick look at it? | 12:12 |
| tlater[m] | traveltissues is really the only one who can approve, but they're not around today | 12:13 |
| *** lachlan has quit IRC | 12:23 | |
| tme5 | i've mostly gone off my proposal for local sources. What I was planning to do next was implement the project-path variable that ben and tristan suggested in response | 12:29 |
| tme5 | i assume that would be ok, being a far less intrusive change | 12:29 |
| *** lachlan has joined #buildstream | 12:35 | |
| *** santi has quit IRC | 12:45 | |
| *** lachlan has quit IRC | 12:59 | |
| tme5 | also i think i've addressed all the points on !1765 now, shall I assign it for merging? | 13:03 |
| gitlab-br-bot | MR !1765: Make Git source plugin clone submodules recursively https://gitlab.com/BuildStream/buildstream/merge_requests/1765 | 13:03 |
| tme5 | ah actually, i remembered now that someone told me that i'd have to increment some kind of cache key salt or something | 13:04 |
| tme5 | because i've made a change to how an element might behave. is there anything like that to do? | 13:04 |
| coldtom | yeah, that change should _probably_ have something added to GitSource.get_unique_key (if i remembered the api correctly) | 13:07 |
| *** lachlan has joined #buildstream | 13:15 | |
| *** lachlan has quit IRC | 13:20 | |
| *** lachlan has joined #buildstream | 13:21 | |
| tme5 | do you think coldtom? i don't think it needs to in general, it just needs to be marked as different from the previous version | 13:22 |
| tme5 | the current get_unique_key is still fine | 13:22 |
| coldtom | ime that's the easiest way to mark it as different | 13:22 |
| tme5 | as in i should add superfluous things to the key? | 13:23 |
| * tlater[m] is pretty sure there was a version API | 13:26 | |
| tlater[m] | So you could just bump that, whatever it was | 13:27 |
| tme5 | the format version? or for the source? | 13:27 |
| tlater[m] | tme5: I think there's a plugin version setting | 13:32 |
| tlater[m] | That will be included in the key, and therefore make everything just work | 13:32 |
| * tlater[m] takes a look | 13:33 | |
| tlater[m] | tme5: Aww, boo, there is no such thing for source plugins | 13:35 |
| *** santi has joined #buildstream | 13:50 | |
| benschubert | tlater[m]: should be BST_FORMAT_VERSION that is bumped no? | 13:54 |
| tlater[m] | benschubert: I don't see it factoring into source keys | 13:54 |
| benschubert | ah :/ Well, I'm not sure what's best. I'd rather not modify the cache key just to modify it though | 13:55 |
| coldtom | is the format version not for format changes anyway | 13:56 |
| benschubert | right, and the format here didn't change right? So maybe "just do nothing" ? | 13:57 |
| gitlab-br-bot | tlater approved MR !1768 (bschubert/cleanup-element-fetch->master: Small cleanups in `element._fetch`) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1768 | 14:08 |
| coldtom | i think "just do nothing" is probably the right move | 14:25 |
| juergbi | benschubert: fetch_original=True is only for workspaces, so I'm not expecting any expiry issues | 14:32 |
| tme5 | alright then, shall i assign to marge? | 14:35 |
| tlater[m] | benschubert: I'm thinking about how to reproduce #1239 - just to make sure I understand this right, it fails because the local build logs are missing or because they end up not being pushed to the cas server? | 14:38 |
| gitlab-br-bot | Issue #1239: BuildStream tries to push incomplete artifacts and fails https://gitlab.com/BuildStream/buildstream/issues/1239 | 14:38 |
| benschubert | juergbi: thanks :) we can therefore optimize this part there ! | 14:49 |
| juergbi | benschubert: the critical code path always uses fetch_original=False, though, so I don't see how this makes a difference | 14:50 |
| juergbi | or am I misreading the diff? | 14:50 |
| benschubert | tme5: would you mind adapting the method's documentations that you added for the format? Afterwards go for marge | 14:50 |
| benschubert | juergbi: it would be called every time we do a fetch even though it would be no-op-ish | 14:51 |
| juergbi | ah, I didn't look at sufficient context in the diff. let me take another look | 14:58 |
| juergbi | benschubert: yes, lgtm, if all sources are already in source cache or if it's freshly pulled from remote source cache, there should be no need to call __cache_sources() | 15:02 |
| juergbi | that said, _source_cached() caches the result in a variable. so I don't know whether this optimization is noticeable. as I think we call _source_cached() already in the main process as part of should_fetch() | 15:03 |
| *** lachlan has quit IRC | 15:03 | |
| *** lachlan has joined #buildstream | 15:06 | |
| lachlan | Hi, I'm going to create a test MR for bot testing please ignore it - this is for testing only. | 15:11 |
| benschubert | juergbi: agreed, just think it makes more sense to don't over do :) | 15:24 |
| juergbi | sure, looks fine to me | 15:24 |
| tme5 | benschubert, sorry if i'm being dense, but i'm not sure what you mean, which methods aren't in the right format? | 15:33 |
| benschubert | tme5: https://gitlab.com/BuildStream/buildstream/merge_requests/1765/diffs?diff_id=67467263#d702e4184719212f721d6d7de27b95489ec90675_247_257 for example :) | 15:39 |
| tme5 | oh, like change it to have a "Returns:" thing | 15:40 |
| tme5 | i understand | 15:40 |
| benschubert | and the # method_name() at the start too please :) | 15:40 |
| gitlab-br-bot | BenjaminSchubert merged MR !1768 (bschubert/cleanup-element-fetch->master: Small cleanups in `element._fetch`) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1768 | 15:48 |
| *** lachlan has quit IRC | 15:48 | |
| *** lachlan has joined #buildstream | 15:48 | |
| tme5 | i put () even if it takes arguments? that seems to be what other things do | 15:49 |
| tlater[m] | tme5: Yup | 15:51 |
| * tlater[m] wishes we could run pydocstyle | 15:51 | |
| tlater[m] | But alas, that only works on actual doc comments | 15:51 |
| tlater[m] | If only we used those for non-public interfaces as well. Would make using IDEs and things so much nicer too. | 15:52 |
| benschubert | ^ I definitely agree, but have other discussions on the ML before that :'D | 15:55 |
| tlater[m] | benschubert: I'm just day-dreaming, that would be a *really* rough refactor x) | 15:57 |
| tlater[m] | It'd mean coming up with a regex that somewhat reliably changes comment formats, double checking all of that and then fixing all our horribly outdated internal docs | 15:58 |
| benschubert | Or, a gentler approach with rewriting as we go but yeah, probably not worth the effort :) | 16:00 |
| *** lachlan has quit IRC | 16:12 | |
| *** phildawson-ct has joined #buildstream | 16:13 | |
| *** santi has quit IRC | 16:15 | |
| *** lachlan has joined #buildstream | 16:17 | |
| *** lachlan has quit IRC | 16:20 | |
| *** lachlan has joined #buildstream | 16:27 | |
| benschubert | Would someone know where I can find results of benchmarking we started doing in january? Like the earliest ones | 16:32 |
| tlater[m] | lachlan: Would know more, but afaik they're all in pipelines on the benchmarking repository | 16:39 |
| tlater[m] | Yes, we are using gitlab's caching as permanent file storage, we have been talking about moving that somewhere else. | 16:39 |
| tlater[m] | Or well, gitlab artifacts | 16:39 |
| tlater[m] | Those are different from caches, I suppose | 16:39 |
| tlater[m] | benschubert: ^ | 16:39 |
| Kinnison | benschubert: I *think* I sent a zip out to the ML | 16:46 |
| Kinnison | benschubert: if not, remind me on Monday (I'm about to head out to the airport) and I'll see if I can dig it up | 16:46 |
| tlater[m] | Oh, the other benchmarking | 16:46 |
| benschubert | great thanks ! I'll try more digging | 16:46 |
| tlater[m] | o\ | 16:46 |
| benschubert | the earliest I found was on https://gitlab.com/BuildStream/buildstream/merge_requests/1070 but I know we had some before | 16:47 |
| benschubert | and I 'd like to know around debian-stack.bst too :) | 16:47 |
| Kinnison | Hmm, oh, that stuff, hm | 16:47 |
| Kinnison | I was thinking you meant the profiling we brought to the Meetup in Jan | 16:47 |
| tlater[m] | benschubert: The earliest I know of is this one: https://gitlab.com/BuildStream/benchmarking/benchmarks/-/jobs/51899048 | 16:48 |
| tlater[m] | But it's not that benchmarking, obviously | 16:48 |
| tlater[m] | This one's likely more useful: https://gitlab.com/BuildStream/benchmarking/benchmarks/-/jobs/53611940 | 16:49 |
| *** santi has joined #buildstream | 16:50 | |
| *** bochecha has quit IRC | 16:50 | |
| *** lachlan has quit IRC | 16:53 | |
| benschubert | Kinnison: the profiling should have this data actually now that I think about it | 16:53 |
| benschubert | no it doesn't :( | 16:55 |
| *** lachlan has joined #buildstream | 17:06 | |
| *** phildawson-ct has quit IRC | 17:10 | |
| *** phildawson-ct has joined #buildstream | 17:16 | |
| *** lachlan has quit IRC | 17:21 | |
| *** lachlan has joined #buildstream | 17:24 | |
| *** lachlan has quit IRC | 17:25 | |
| *** phildawson-ct has quit IRC | 17:33 | |
| *** tiagogomes has quit IRC | 17:38 | |
| *** narispo has quit IRC | 17:40 | |
| *** narispo has joined #buildstream | 17:40 | |
| *** tme5 has quit IRC | 17:50 | |
| *** phildawson-ct has joined #buildstream | 17:54 | |
| *** lachlan has joined #buildstream | 17:54 | |
| *** santi has quit IRC | 17:57 | |
| *** phildawson-ct has quit IRC | 18:38 | |
| *** lachlan has quit IRC | 18:45 | |
| *** lachlan has joined #buildstream | 19:01 | |
| *** toscalix has joined #buildstream | 19:09 | |
| *** lachlan has quit IRC | 19:45 | |
| *** lachlan has joined #buildstream | 19:59 | |
| *** phoenix has joined #buildstream | 20:14 | |
| *** phoenix has quit IRC | 20:22 | |
| *** lachlan has quit IRC | 20:28 | |
| *** lachlan has joined #buildstream | 20:43 | |
| *** toscalix has quit IRC | 20:47 | |
| *** toscalix has joined #buildstream | 20:49 | |
| *** lachlan has quit IRC | 20:54 | |
| *** toscalix has quit IRC | 21:09 | |
| *** phildawson_ has joined #buildstream | 21:11 | |
| *** toscalix has joined #buildstream | 21:57 | |
| *** toscalix has quit IRC | 21:58 | |
| *** toscalix has joined #buildstream | 21:59 | |
| *** phoenix has joined #buildstream | 22:06 | |
| *** toscalix has quit IRC | 22:14 | |
| *** toscalix has joined #buildstream | 22:17 | |
| *** toscalix has quit IRC | 22:19 | |
| *** toscalix has joined #buildstream | 22:21 | |
| *** phildawson-ct has joined #buildstream | 22:23 | |
| *** phildawson_ has quit IRC | 22:24 | |
| *** phildawson-ct has quit IRC | 22:27 | |
| *** phoenix has quit IRC | 22:29 | |
| *** toscalix has quit IRC | 22:36 | |
| *** toscalix has joined #buildstream | 22:39 | |
| *** phildawson-ct has joined #buildstream | 22:41 | |
| *** phildawson-ct has quit IRC | 22:44 | |
| *** toscalix has quit IRC | 22:55 | |
| *** toscalix has joined #buildstream | 22:56 | |
| *** toscalix has quit IRC | 23:17 | |
| *** toscalix has joined #buildstream | 23:20 | |
| *** toscalix has quit IRC | 23:38 | |
| *** toscalix has joined #buildstream | 23:39 | |
| *** phoenix has joined #buildstream | 23:50 | |
| *** toscalix has quit IRC | 23:51 | |
| *** toscalix has joined #buildstream | 23:54 | |
| *** phoenix has quit IRC | 23:54 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!