| *** narispo has quit IRC | 03:04 | |
| *** narispo has joined #buildstream | 03:05 | |
| gitlab-br-bot | juergbi opened issue #1223 (Optimize output_directories in SandboxREAPI (remote execution / buildbox-run)) on buildstream https://gitlab.com/BuildStream/buildstream/issues/1223 | 06:17 |
|---|---|---|
| *** misterwhatever has quit IRC | 07:15 | |
| *** rdale has joined #buildstream | 08:55 | |
| *** traveltissues has joined #buildstream | 09:34 | |
| *** phildawson_ has joined #buildstream | 10:03 | |
| *** tme5 has joined #buildstream | 10:08 | |
| *** bochecha has joined #buildstream | 10:20 | |
| *** bochecha has quit IRC | 10:20 | |
| *** jonathanmaw has joined #buildstream | 10:23 | |
| *** lachlan has joined #buildstream | 10:30 | |
| *** lachlan has joined #buildstream | 10:31 | |
| benschubert | tlater[m]: need another round of review? Or can I help? | 10:34 |
| *** tiagogomes has joined #buildstream | 10:34 | |
| *** tiagogomes has quit IRC | 10:36 | |
| *** tiagogomes has joined #buildstream | 10:43 | |
| tlater[m] | benschubert: I'll ask for review soon, just need a minute to finish up | 10:45 |
| *** lachlan has quit IRC | 10:51 | |
| *** lachlan has joined #buildstream | 10:57 | |
| *** phildawson has joined #buildstream | 11:01 | |
| *** phildawson_ has quit IRC | 11:01 | |
| *** lachlan has quit IRC | 11:02 | |
| *** narispo has quit IRC | 11:03 | |
| *** narispo has joined #buildstream | 11:03 | |
| tlater[m] | Is... GitLab down? | 11:03 |
| tlater[m] | Looks like it | 11:03 |
| tlater[m] | Ah, Google at it again | 11:04 |
| *** lachlan has joined #buildstream | 11:04 | |
| tlater[m] | https://status.gitlab.com/ taunts me with their green icon :( | 11:08 |
| *** lachlan has quit IRC | 11:13 | |
| *** lachlan has joined #buildstream | 11:13 | |
| *** lachlan has quit IRC | 11:16 | |
| benschubert | not green anymore :'D | 11:18 |
| benschubert | gitlab outage o/ | 11:18 |
| tlater[m] | \o/ | 11:22 |
| * tlater[m] plays with click.Choice in the mean time | 11:22 | |
| tlater[m] | benschubert: on that note, what is your suggestion on how to handle the click conversion? | 11:23 |
| benschubert | choices=MyEnum.__members__ :D | 11:23 |
| tlater[m] | I deliberately don't want to use FastEnum | 11:23 |
| benschubert | that's not fastenum :) | 11:23 |
| tlater[m] | Oh, right, yeah, that's probably a better idea :D | 11:23 |
| benschubert | it _should_ work | 11:24 |
| tlater[m] | benschubert: we've seen some odd interaction between static and instantiated things when used in click before | 11:25 |
| benschubert | ah? | 11:25 |
| tlater[m] | I don't exactly remember what, but static components of objects didn't particularly like being given to click args. | 11:26 |
| tlater[m] | Since click is evaluated before those components are evaluated or somesuch? Been a little while... | 11:26 |
| * tlater[m] will try anyway | 11:26 | |
| tlater[m] | Hmm, no, looks like I won't | 11:27 |
| tlater[m] | https://click.palletsprojects.com/en/7.x/options/#choice-options | 11:27 |
| tlater[m] | Choice options are a type in the click world | 11:27 |
| benschubert | type=click.Chocie([Enum.__members__]) | 11:28 |
| tlater[m] | benschubert: That removes the conversion | 11:28 |
| tlater[m] | Oh, but hang on, my type inherits from click.Choice | 11:29 |
| tlater[m] | Surely that means it just works? | 11:29 |
| benschubert | possibly | 11:29 |
| tlater[m] | Yeah, should do, I call super().init(LogLevel.member_names) | 11:29 |
| tlater[m] | So this is equivalent | 11:30 |
| benschubert | yup | 11:30 |
| tlater[m] | Any other suggestions for that bit of code? | 11:30 |
| tlater[m] | Without being able to look at it, of course :D | 11:31 |
| benschubert | hard to do, must admit I don't remember it entirely :'D | 11:32 |
| tlater[m] | Yeah, nw | 11:33 |
| * tlater[m] will wait for GitLab to come back alive | 11:33 | |
| *** lachlan has joined #buildstream | 11:34 | |
| tlater[m] | benschubert: Or maybe some ad hoc review over hastebin? https://hastebin.com/hilugacaze.rb :D | 11:34 |
| benschubert | can't access that site, proxies.... github's gist? | 11:35 |
| tlater[m] | https://gist.github.com/TLATER/d057723efc63405214048de6d5f6443a | 11:36 |
| benschubert | cheers | 11:37 |
| benschubert | the thing I don't like here is the very tight coupling between the two classes. Could we move LogLevel as an internal class of ClickLogLevel and rename it BuildBoxCasdLogLevelChoice or something akin? | 11:39 |
| tlater[m] | benschubert: That's a good call | 11:42 |
| * tlater[m] was trying to see if there's a way to do this with multiple inheritance, but that seems simpler | 11:44 | |
| *** lachlan has quit IRC | 11:44 | |
| *** akvilebirgelyte__ has quit IRC | 11:52 | |
| *** lachlan has joined #buildstream | 11:54 | |
| *** akvilebirgelyte__ has joined #buildstream | 12:01 | |
| *** misterwhatever has joined #buildstream | 12:08 | |
| *** narispo has quit IRC | 12:15 | |
| *** narispo has joined #buildstream | 12:16 | |
| *** lachlan has quit IRC | 12:17 | |
| *** misterwhatever has quit IRC | 12:19 | |
| *** lachlan has joined #buildstream | 12:28 | |
| *** narispo has quit IRC | 12:29 | |
| *** narispo has joined #buildstream | 12:29 | |
| *** lachlan has quit IRC | 12:34 | |
| *** lachlan has joined #buildstream | 12:37 | |
| *** lachlan has quit IRC | 12:45 | |
| tlater[m] | juergbi: Regarding this comment: https://gitlab.com/BuildStream/buildstream/merge_requests/1645#note_251373522 | 12:52 |
| tlater[m] | Does the EACCESS case matter? | 12:53 |
| tlater[m] | In that case we simply don't have permissions to prune that tree; that's fine, and should raise an error, right? | 12:53 |
| *** lachlan has joined #buildstream | 12:53 | |
| juergbi | tlater[m]: yes, if we always specify the basedir | 12:53 |
| juergbi | with the implicit basedir '/' it would have been odd, imo | 12:53 |
| tlater[m] | Oh, I see what you mean | 12:53 |
| tlater[m] | Technically catching that errno as well and checking if the directory is perhaps empty would have worked, but we don't per-se need that. | 12:54 |
| juergbi | yes, I think requiring basedir is really the best option | 12:54 |
| juergbi | and we shouldn't need to deal with EACCES in that case | 12:55 |
| juergbi | (checking whether the directory is empty would also be racy) | 12:55 |
| tlater[m] | Awesome, in that case, juergbi / benschubert I think that branch is ready now :) | 12:55 |
| juergbi | \o/ | 12:55 |
| tlater[m] | !1645 for convenience | 12:56 |
| gitlab-br-bot | MR !1645: Refactor casserver.py: Stop relying on the buildstream-internal `CASCache` implementation https://gitlab.com/BuildStream/buildstream/merge_requests/1645 | 12:56 |
| tlater[m] | Go bot! | 12:56 |
| * tlater[m] still likes that :D | 12:56 | |
| juergbi | remove_path_with_parents looks good to me now | 12:57 |
| gitlab-br-bot | juergbi approved MR !1645 (tlater/artifactserver-casd->master: Refactor casserver.py: Stop relying on the buildstream-internal `CASCache` implementation) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1645 | 12:57 |
| tlater[m] | \o/ | 12:58 |
| *** lachlan has quit IRC | 12:58 | |
| juergbi | tlater[m]: pylint/mypy aren't happy yet | 13:02 |
| tlater[m] | Boo | 13:02 |
| tlater[m] | They didn't tell me they were unhappy | 13:03 |
| tlater[m] | Or maybe they did and I've just started tuning them out because my config was broken for so long | 13:03 |
| tlater[m] | ... no. Seems like we have a different pylint version upstream now? | 13:04 |
| juergbi | shouldn't tox take care of this or are you using a manual install of pylint? | 13:12 |
| juergbi | or it might need tox --recreate | 13:12 |
| tme5 | so, i'm starting to look again at #1047, as it's been bugging us on a project using buildstream | 13:17 |
| gitlab-br-bot | Issue #1047: Downloadable file plugins should be able to handle local sources within the project https://gitlab.com/BuildStream/buildstream/issues/1047 | 13:17 |
| tme5 | a while ago i discussed this with van berkom in the mailing list but we didn't get to a clear outline of what the solution should be | 13:19 |
| tme5 | i'm going to have a bit more of a think about how to get the best UX with this | 13:20 |
| tlater[m] | tme5: We have an MR that's been closed because nobody had time to bring it to an end | 13:20 |
| tlater[m] | It's a fairly complex issue. | 13:20 |
| tme5 | you have a number for that? | 13:21 |
| tme5 | do you* | 13:21 |
| tlater[m] | tme5: !1423 | 13:21 |
| gitlab-br-bot | MR !1423: WIP: Allowing a downloadable file to be sourced locally https://gitlab.com/BuildStream/buildstream/merge_requests/1423 | 13:21 |
| tme5 | yes, i read this at the time, it contributed to my initial proposal for beaviour | 13:22 |
| tlater[m] | Ah, you've commented on that | 13:22 |
| tlater[m] | Well, bring it up on the ML again | 13:22 |
| tme5 | will do, after i have something new to say | 13:25 |
| *** phildawson has quit IRC | 13:36 | |
| *** phildawson has joined #buildstream | 13:37 | |
| tme5 | just because i don't know much about the caching: are sources caches only local? like they can't be shared on the cache servers? | 13:44 |
| benschubert | tme5: on master, you can now have source caches on remote too :) | 13:46 |
| benschubert | you have basically 3 caches for sources: | 13:47 |
| benschubert | 1) the buildstream local source cache, based on CAS | 13:47 |
| benschubert | 2) the plugin-dependent source cache | 13:47 |
| benschubert | 3) The remote source cache, which is a bst-artifact-server (or potentially external implementations when this becomes a thing) | 13:47 |
| coldtom | just ooi, could someone use an external CAS as a remote source cache, provided there was a bst-artifact-server somewhere else, or does that only work with artifacts? | 13:50 |
| *** rdale has quit IRC | 13:51 | |
| benschubert | tlater[m]: ^ ? | 13:52 |
| tlater[m] | Should work with either, iirc | 13:52 |
| coldtom | neat! | 13:53 |
| benschubert | tlater[m]: finished review of your MR, I have some last few nits around logging and code style, for the rest LGTM :) | 13:55 |
| *** lachlan has joined #buildstream | 14:21 | |
| *** lachlan has quit IRC | 14:31 | |
| tlater[m] | benschubert: Alright, I've cleared those out :) | 14:39 |
| tlater[m] | Happy for me to merge the branch at this point? | 14:39 |
| gitlab-br-bot | BenjaminSchubert approved MR !1645 (tlater/artifactserver-casd->master: Refactor casserver.py: Stop relying on the buildstream-internal `CASCache` implementation) on buildstream https://gitlab.com/BuildStream/buildstream/merge_requests/1645 | 14:44 |
| tlater[m] | \o/ | 14:45 |
| tme5 | potentially major suggestion, how do we feel about retiring "ref" as a word and replacing it with something more descriptive on each plugin | 14:57 |
| tme5 | like "sha256" for remote and "commit" for git, for example | 14:58 |
| coldtom | not sure it's worth the huge breaking change really | 14:59 |
| tme5 | (btw, i'm suggesting these off-the-cuff because as far as i know, 2.0 is the next milestone for bst and i'd have thought we can break indiscriminately for that) | 14:59 |
| tlater[m] | tme5: It makes parsing significantly more difficult since it leaves the key up to the plugin, so we now need to know what ref key the plugin uses ahead of loading element state. | 15:00 |
| coldtom | tlater[m]: it's abstracted isn't it? | 15:00 |
| coldtom | using Source.get_ref() etc. | 15:00 |
| tlater[m] | coldtom: Yes, but you don't need to load the plugin for that | 15:01 |
| * tlater[m] thinks it also makes grepping and otherwise interacting with projects through non-buildstream means needlessly difficult | 15:01 | |
| tlater[m] | Ultimately all those more specific types of refs are just refs | 15:01 |
| tlater[m] | From a project point of view you don't care about the difference. | 15:02 |
| tlater[m] | The benefit is that someone entirely new to BuildStream can use the terminology they're used to that's inherent to their specific tool, but I think that's a very small benefit, especially since users aren't expected to actually interact with `ref` much. | 15:03 |
| tme5 | hrmm, i suppose it's also more info to keep in your head when using buildstream | 15:03 |
| tlater[m] | Yeah, needing to know exactly which plugin uses which `ref` name is also a pain x) | 15:04 |
| tme5 | yeah, think it's probably a usability net-loss | 15:04 |
| coldtom | also, even if now is the best time for breaking changes, it'd still be nice to have a smallish diff to port a project to bst2 | 15:04 |
| tlater[m] | Currently it'll stay reasonably small, I think :) Main differences are in plugins and CLI. | 15:05 |
| coldtom | +1, it's currently pretty painless I think | 15:05 |
| coldtom | so, i have a master bst-artifact-server configured to use 250G, apparently things succeed at pushing to the server, however when I try to pull from said server the pull just immediately says "SUCCESS" without pulling any objects. Some of the artifact refs are there, some are not at all there, anyone got any ideas? | 15:05 |
| tlater[m] | coldtom: The artifact refs are on the server? | 15:06 |
| coldtom | tlater[m]: some are, some are not | 15:06 |
| tlater[m] | Interesting | 15:06 |
| tlater[m] | Do the objects appear on the server end? | 15:07 |
| coldtom | there's 14G in cas/objects so I think so | 15:07 |
| tlater[m] | And none of the objects are there locally anymore? | 15:07 |
| tlater[m] | It sounds like BuildStream isn't pulling what you already have locally | 15:08 |
| coldtom | it's definitely not there locally, because running a build starts fetching everything | 15:08 |
| coldtom | also, the build to populate the cache was on a separate machine | 15:08 |
| tlater[m] | Specifically source cahce, then? | 15:08 |
| *** rdale has joined #buildstream | 15:08 | |
| * tlater[m] hasn't seen this before, wouldn't know why it happens | 15:09 | |
| * coldtom is going to try nuking the cache and pushing afresh to see if it works now | 15:09 | |
| tlater[m] | I doubt it would slip through our test suite. Any chance you could look at more verbose logging to see what messages are exchanged between BuildStream and the casserver? | 15:09 |
| coldtom | sure, i assume you want bst --debug? | 15:10 |
| tlater[m] | I hope that gives enough detail, yeah | 15:10 |
| tlater[m] | If not, In a minute or 10 from now a branch will land that allows enabling logging on the server end. | 15:11 |
| tlater[m] | That should make debugging these sorts of issues significantly easier :D | 15:11 |
| coldtom | \o/ that is a good patch, ty | 15:12 |
| tlater[m] | coldtom: If you have feedback on the logging, send it my way - this has been a thorn in my side for a while now. | 15:14 |
| *** lachlan has joined #buildstream | 15:14 | |
| *** lachlan has quit IRC | 15:17 | |
| coldtom | hmm pulling with debug didn't give any more information | 15:20 |
| coldtom | oh, pulling is working on the CI runner that did the push, just not other machines (even when running the same docker image) | 15:22 |
| *** lachlan has joined #buildstream | 15:27 | |
| *** lachlan has quit IRC | 15:30 | |
| tlater[m] | coldtom: After clearing out the objects in the CI runner? | 15:44 |
| * tlater[m] wonders if their BuildStream versions don't match up and cause cache key oddities | 15:44 | |
| coldtom | the log shows `fetch needed` rather than `cached` | 15:47 |
| tlater[m] | :| | 15:48 |
| tlater[m] | coldtom: Any particularities to your setup that we should know of? | 15:48 |
| coldtom | i think everything should be on the same docker image, if not there's 8 commits between and cache keys seem to be the same | 15:49 |
| coldtom | only thing i can think of is that the bst-artifact-server is actually two servers, one running as push and the other as pull on the same drive | 15:49 |
| coldtom | both are configured to have the same quota | 15:50 |
| *** akvilebirgelyte__ has quit IRC | 15:51 | |
| tlater[m] | Right | 15:51 |
| tlater[m] | They're operating on the same directory, I assume? | 15:52 |
| coldtom | yes | 15:52 |
| coldtom | i believe this setup works for 1.4.x fwiw | 15:52 |
| tlater[m] | I can see that being somewhat problematic in the current world order, because both of them will be running buildbox-casd instances, which don't seem to support running at the same time terribly well | 15:53 |
| coldtom | ah, that's a good point | 15:53 |
| tlater[m] | juergbi: would be better to judge what could go wrong here | 15:53 |
| tlater[m] | We do try to keep them from interfering as far as I know (certainly with my branch-to-land), but I'm not sure how successful we'd be | 15:54 |
| *** akvilebirgelyte__ has joined #buildstream | 15:55 | |
| juergbi | we should probably look into that. however, it's supposed to be safe as the pull-only server should not perform any writes and thus, shouldn't trigger or interfere with expiry | 15:55 |
| juergbi | we should also really fix it such that we can handle both in the same process. it's a gRPC (python) issue, though, iirc | 15:56 |
| tlater[m] | juergbi: Any chance they end up using the same socket in a slightly older buildstream version? | 15:57 |
| juergbi | I think we've always used sockets in random temp dirs in master, so, no | 15:58 |
| juergbi | your branch changes this slightly but risk of conflict should be extremely low | 15:58 |
| juergbi | eh, your commit in my branch | 15:58 |
| tlater[m] | Yeah, just wasn't sure if we'd ever used a single deterministic path | 15:59 |
| juergbi | tlater[m]: hm, remote execution CI job fails in your branch | 16:12 |
| tlater[m] | juergbi: Yeah, just spotted that | 16:13 |
| tlater[m] | No logging in CI unfortunately, I'll try to reproduce locally | 16:13 |
| juergbi | only in the last 3 pipelines | 16:13 |
| tlater[m] | Eh, it'll be easiest to push a commit with logging enabled | 16:14 |
| tlater[m] | Why is that test set to be allowed to fail anyway? | 16:14 |
| juergbi | it might be using buildgrid master | 16:16 |
| juergbi | ideally we'd have two jobs. a gating one with a fixed buildgrid revision, and master non-gating | 16:16 |
| *** kapip has joined #buildstream | 16:16 | |
| WSalmon | tpollard, docker exec -u 0 -it mycontainer bash | 16:32 |
| WSalmon | https://stackoverflow.com/a/35485346 | 16:32 |
| coldtom | if i have an existing bst cache, is it safe to start up a bst-artifact-server using it as the cache dir? | 16:51 |
| * coldtom needs to build on a separate machine but not lose his cache | 16:52 | |
| tlater[m] | coldtom: Keep a copy around. I suspect the directory formats are ever so slightly different, but most of it should be compatible. | 16:52 |
| tlater[m] | The both use buildbox-casd, it's just some legacy stuff that might not entirely work | 16:52 |
| tlater[m] | juergbi: The branch seems to fail invoking local_cas.FetchMissingBlobs | 16:53 |
| tlater[m] | That wouldn't go through bst-artifact-server, right? | 16:53 |
| juergbi | if it's purely local, not via a CasRemote, then no | 16:54 |
| tlater[m] | It's going through casremote.py | 16:54 |
| tlater[m] | FetchMissingBlobs isn't even implemented on bst-artifact-server, so I wouldn't see how it could invoke that | 16:55 |
| tlater[m] | OTOH, that would explain the error | 16:56 |
| juergbi | tlater[m]: FetchMissingBlobs is in casd. if that's done in a context with a remote, casd will talk to bst-artifact-server | 16:56 |
| juergbi | with a regular CAS method | 16:56 |
| juergbi | (which will then proxy it to the 'remote' instance of casd, of course) | 16:57 |
| * tlater[m] wonders if he accidentally removed a FetchTree method somewhere | 16:57 | |
| juergbi | only failing with remote execution seems odd, though | 16:59 |
| juergbi | unless the issue is that it breaks compatibility with buildgrid's cas server | 17:00 |
| juergbi | I'm actually not sure whether that is used or bst-artifact-server for the CAS part | 17:00 |
| tlater[m] | This seems to be specifically workspace related | 17:01 |
| tlater[m] | All the other RE tests pass | 17:01 |
| tlater[m] | Ah, looks like what changed is that I rebased against master | 17:04 |
| tlater[m] | Which changed some stuff regarding workspaces | 17:04 |
| juergbi | hm, but RE CI job passed in that branch, right? | 17:06 |
| juergbi | so the server changes break the new test. that seems possible | 17:07 |
| tlater[m] | It's annoyingly hard to debug locally :( | 17:08 |
| *** lachlan has joined #buildstream | 17:11 | |
| *** lachlan has quit IRC | 17:28 | |
| benschubert | tlater[m]: you were playing a lot with update_cache_key, do you remember whether the state can change when we get cached? | 17:32 |
| tlater[m] | benschubert: Let me double check | 17:32 |
| tlater[m] | `__update_artifact_sate` should be called | 17:33 |
| tlater[m] | But we'd already know the cache key, I assume? | 17:33 |
| *** narispo has quit IRC | 17:34 | |
| tlater[m] | The main state that will change is the state of reverse dependencies - some builds will be scheduled | 17:34 |
| *** narispo has joined #buildstream | 17:34 | |
| benschubert | right, thanks :) | 17:37 |
| benschubert | I *think* I have a clear path on untangling that, finally | 17:39 |
| tlater[m] | benschubert: Oh? I'd love to see that :D | 17:40 |
| benschubert | I should soon have a WIP | 17:40 |
| *** jonathanmaw has quit IRC | 17:46 | |
| *** tme5 has quit IRC | 17:52 | |
| *** misterwhatever has joined #buildstream | 17:52 | |
| *** narispo has quit IRC | 17:58 | |
| *** misterwhatever has quit IRC | 17:58 | |
| *** narispo has joined #buildstream | 18:02 | |
| *** tiagogomes has quit IRC | 18:13 | |
| benschubert | tlater[m]: !1739 is a start :) | 18:14 |
| gitlab-br-bot | MR !1739: WIP: Optimize consistency and state handling https://gitlab.com/BuildStream/buildstream/merge_requests/1739 | 18:14 |
| benschubert | early reviews are appreciated given the size this thing is going to be | 18:15 |
| benschubert | I'm not entirely sure about the intent of those two tests: https://gitlab.com/BuildStream/buildstream/blob/master/tests/frontend/track.py#L206-224 . Is it to make sure that when we check for consistency, we do catch errors on plugins? Or is there something more? Because I would then just use a 'bst show', which should be sufficient... | 18:22 |
| benschubert | tristan: I think you wrote them? might be a long time ago though ^ | 18:22 |
| *** narispo has quit IRC | 18:28 | |
| *** narispo has joined #buildstream | 18:28 | |
| *** slaf_ has joined #buildstream | 18:30 | |
| *** slaf_ has joined #buildstream | 18:30 | |
| *** slaf_ has joined #buildstream | 18:30 | |
| *** slaf_ has joined #buildstream | 18:30 | |
| *** slaf has quit IRC | 18:32 | |
| *** slaf_ is now known as slaf | 18:32 | |
| tlater[m] | benschubert: I'll take a peek in a bit | 18:39 |
| tlater[m] | Food first :D | 18:39 |
| tlater[m] | benschubert: Oh, I had to do things with those tests recently | 18:39 |
| tlater[m] | It's a bit more than that | 18:40 |
| tlater[m] | It's also to find bugs when running methods of plugins that are only invoked when tracking | 18:40 |
| tlater[m] | Essentially whether we'll get a stacktrace if you run one particular method in a plugin | 18:41 |
| tlater[m] | It definitely covers some things `bst show` wouldn't - there are sister methods for `fetch` as well | 18:41 |
| tlater[m] | s/methods/tests/ | 18:41 |
| benschubert | well the track one only triggers on get_consistency, which is done as soon as the element is instantiated... :D | 18:53 |
| *** traveltissues has quit IRC | 19:19 | |
| *** rdale has quit IRC | 23:20 | |
| *** kapip has quit IRC | 23:23 | |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!