*** Prince781 has joined #buildstream | 00:15 | |
*** Prince781 has quit IRC | 01:22 | |
*** Prince781 has joined #buildstream | 02:58 | |
*** Prince781 has quit IRC | 04:13 | |
*** tristan has joined #buildstream | 04:19 | |
tristan | juergbi, since you are dancing in CAS land, it's like you to please review my comment here: https://gitlab.com/BuildStream/buildstream/issues/76#note_70558371 | 06:21 |
---|---|---|
juergbi | tristan: yes, CAS works pretty much the same in this regard | 06:23 |
juergbi | tristan: also note that with FUSE we don't have to extract at all anymore, though | 06:23 |
tristan | juergbi, Right but... we still need to inspect metadata from the artifacts in various places before a build commences | 07:08 |
tristan | I think this is irrelevant though, or I hope so | 07:09 |
juergbi | tristan: yes but that should similarly be possible with the virtual filesystem API | 07:09 |
tristan | If we have OSTree, we should just make extraction happen at cache insertion time | 07:09 |
juergbi | although we haven't planned any functionality to open a file for reading from there yet | 07:09 |
tristan | And if we have CAS, we should have a way to read the metadata from an artifact, no matter which way it is done | 07:09 |
juergbi | yes, just wanted to note it to avoid spending lots of time to change things with OSTree for minimal benefits and then throw it away when switching to CAS | 07:10 |
tristan | Right, that is sort of the goal of my comment anyway | 07:10 |
juergbi | (it's probably pretty simple but still) | 07:10 |
tristan | even if we *did* stick with OSTree, its still not desirable to try to force artifact caches to support partial extraction | 07:11 |
tristan | Yeah, extract at insert time should be very simple, and a performance gain on it's own | 07:11 |
juergbi | tristan: I don't understand your comment about partial extraction | 07:13 |
juergbi | direct access of metadata in CAS instead of extraction makes sense to me | 07:14 |
juergbi | (it would be no extraction at all, not partial extraction, to be precise) | 07:14 |
juergbi | (here CAS used as generic term) | 07:14 |
tristan | juergbi, I'm looking at #75 separately from CAS, some of the preceding comments there indicate that we would want to get information about the artifact at an earlier stage, without completely extracting the artifact | 07:15 |
tristan | which is something avoidable I think | 07:15 |
tristan | There is an interesting problem still to be solve there in the case that you do run into a failed artifact during a build | 07:16 |
juergbi | ah right, I agree we should not go towards extract jobs or anything like that | 07:17 |
tristan | If you have a PullQueue enabled, you can already know that the artifact you downloaded was a failed one before getting to building it's reverse dependencies | 07:17 |
*** bochecha_ has joined #buildstream | 07:17 | |
tristan | I'll comment again cause I have an idea | 07:18 |
tristan | juergbi, new comment about that, but not related to CAS anymore :) | 07:28 |
tristan | Anyway... lots of issue comments today... issue rampage | 07:28 |
tristan | juergbi, I'm getting a bit more concerned about caching of build trees, in light of big builds like webkit | 07:29 |
tristan | :-/ | 07:29 |
tristan | juergbi, I think that *even if* we are able to exclude the `.git` directory for such a thing, we're *still* going to end up with ~6GB artifacts | 07:30 |
tristan | My built WebKit directory is 5.8GB, excluding the resulting webkit output | 07:31 |
tristan | and this is from a tarball | 07:31 |
tristan | This is actually a really big deal | 07:31 |
gitlab-br-bot | buildstream: merge request (valentindavid/update_mirror->master: WIP: Valentindavid/update mirror) #440 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/440 | 07:31 |
tristan | juergbi, Do you think we should make this optional in some way ? | 07:31 |
tristan | I'm a bit scared of this, its a crap ton of data nobody is going to want to download | 07:32 |
tristan | Maybe we need a fragmented artifact cache for this ? | 07:32 |
tristan | poor Nexus | 07:32 |
tristan | he keeps getting the curveballs | 07:33 |
tristan | Maybe I should write the list on this topic | 07:33 |
*** toscalix has joined #buildstream | 07:36 | |
*** toscalix has quit IRC | 07:38 | |
*** toscalix has joined #buildstream | 07:39 | |
gitlab-br-bot | buildstream: merge request (valentindavid/update_mirror->master: WIP: Valentindavid/update mirror) #440 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/440 | 07:43 |
*** tristan has quit IRC | 07:51 | |
*** tristan has joined #buildstream | 08:37 | |
juergbi | tristan: I was voicing that concern as well and think it should definitely be optional. That said, deduplication should help a lot here with regards to multiple builds, as long as object files are also reproducible | 08:37 |
tristan | juergbi, yeah, but it should be either optional or, on demand perhaps | 08:38 |
juergbi | not sure how we could do on demand | 08:39 |
tristan | juergbi, artifact-cache split is an idea | 08:41 |
tristan | not super pretty, but totally plausible | 08:41 |
tristan | there are only a few cases where we actually *want* the cached build tree, so why not just downloaded when we want it ? | 08:42 |
tristan | If we split the artifact cache in two, locally and remotely, we can have that | 08:42 |
tristan | s/downloaded/download | 08:42 |
juergbi | ah, not making the caching part optional, just the downloading | 08:43 |
juergbi | with CAS it should be possible to do this even without splitting | 08:43 |
juergbi | i.e., download parts of an artifact | 08:44 |
juergbi | we anyway want this for remote execution where we often don't even need the build output | 08:44 |
*** dominic has joined #buildstream | 08:44 | |
juergbi | complete split would prevent effective deduplication (especially relevant when sources are stored in the cache as well) | 08:45 |
tristan | actually, even with ostree we could potentially do that | 08:45 |
tristan | i.e. we could have 2 keys to address it | 08:45 |
juergbi | yes, at least on the low level it's definitely possible | 08:45 |
tristan | ${project}/${element}/${key} and build/${project}/${element}/${key} | 08:46 |
tristan | just separate commits | 08:46 |
juergbi | yes | 08:46 |
*** jonathanmaw has joined #buildstream | 08:50 | |
tristan | juergbi, if we do this, do you think we should split logs and metadata as well for consistency ? | 08:56 |
juergbi | not sure. in CAS I would handle them all the same way but that would be via subdirectories, there would be no need for the separate refs | 08:58 |
*** bethw has joined #buildstream | 08:58 | |
Nexus | tristan: i saw poor Nexus, what's happening now‽ | 08:58 |
juergbi | I don't like the idea of moving metadata out of the object but for consistency it could make sense | 08:58 |
tlater | Hm, now that our fedora image isn't used for testing anymore, but just `bst-here`, should we consider only installing stable buildstream releases on it? | 08:58 |
juergbi | maybe only from 1.2 onward? | 08:59 |
tlater | We could also add an :unstable tag, since bst-here is growing "change the image we'll use" functionality | 08:59 |
tristan | juergbi, for consistency and also having the ability to access metadata might allow us to do more interesting client side things with less overhead | 09:01 |
tlater | Oh, apparently we haven't split off the testing image from the bst-here image | 09:02 |
* tlater thought that was what jjardon did a little while ago | 09:03 | |
gitlab-br-bot | buildstream: merge request (jennis/136-clean-remote-cache->master: WIP: jennis/136 clean remote cache) #421 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/421 | 09:07 |
tristan | Nexus, just sent mail to the list :) | 09:20 |
Nexus | yup, reading now | 09:20 |
tristan | Nexus, things are not going to work exactly as planned, it's hopefully not too huge of an obstacle; but I wanted to make a more public statement, hopefully this will help reduce stress and pressure directed at you :) | 09:21 |
Nexus | much appreciated :) | 09:21 |
*** noisecell has quit IRC | 09:40 | |
gitlab-br-bot | buildstream: issue #316 ("Assertion error when building in non-strict mode") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/316 | 09:47 |
*** aday has joined #buildstream | 09:52 | |
jennis | tristan, FYI remote cache expiry is *mostly* ready. All I am doing now is trying to work around the unlikely case of an artifact being bigger than the actual cache quota itself (which is turning out to be a real PITA) | 09:54 |
*** noisecell has joined #buildstream | 10:06 | |
* tlater considers writing to the ML about the structure of buildstream-docker-images | 10:20 | |
tlater | There's just about no strucutre to how we manage those images anymore | 10:20 |
tlater | And I think splitting up buildstream-fedora into a testing and runtime image is pretty important | 10:21 |
tristan | jennis, I guess you can abort bst-artifact-receive and hit the cleanup path in that case ? | 10:39 |
tristan | jennis, I mean... while it's in progress; as you may not be able to know until you fill up the quota | 10:39 |
tristan | jennis, then again, you dont know if there are deduplicated similar ones | 10:40 |
tristan | jennis, Ok well... *please* be forgiving and dont spend too much effort perfecting that case | 10:40 |
tristan | jennis, it's acceptable to let it grow beyond the quota and think about it after | 10:41 |
tlater | tristan: The problem is mostly that when you do try to close the pipe on the server end, the client complains about a broken pipe | 10:41 |
tristan | jennis, this is A.) An edge case that is not worth the hassle perfecting anyway... B.) Some hopefully short lived code which will be replaced by CAS | 10:41 |
tristan | Huh, I was sure we were handling hangups correctly | 10:42 |
* tlater was a bit confused too, perhaps someone else can shed some light there | 10:44 | |
tlater | I suspect we send some other data *after* the tar stream completes | 10:44 |
tlater | And that the client doesn't realize that the pipe closed in that case | 10:44 |
jmac | I didn't think we did send any more data after the tar stream... | 10:45 |
tristan | Ok so, maybe we should just define what "complain" is here, and handle that as an error ? | 10:45 |
tlater | jennis tried that, but it keeps complaining in various spots, so he'd have to litter everything in try/except clauses | 10:45 |
tristan | :-/ | 10:45 |
tristan | tlater, sounds like perhaps the try/except clauses were missing in the first place ? | 10:46 |
tlater | Yeah, agreed, there's something fundamentally wrong here | 10:46 |
jmac | Aha, no, we do wait for a PushCommandType.done | 10:46 |
tristan | I mean, remember that this code comes from a proof of concept toy for an upstream proposal to ostree | 10:46 |
tlater | jmac: Presumably that is something we define as a little protocl? | 10:47 |
tristan | we made it a bit more robust but it aint really fault tolerant so much | 10:47 |
tlater | Yeah, fair enough, it's about to be obsoleted anyway | 10:47 |
tlater | But it *does* make the edge case at hand painful to deal with | 10:47 |
jennis | I'm implemented it so that if the artifact is larger than the quota, it won't start deleting already-present artifacts | 10:48 |
jennis | So tristan, I guess what you suggested about letting it grow beyond the quota and deal with it after could be a possibility | 10:49 |
jmac | tlater: Yes, it's defined just by the implementation in pushreceive.py; there's no explicit definition | 10:49 |
jmac | But AFAIK it's just a 5-byte message with one byte set to PushCommandType.done | 10:50 |
jennis | however, if the quota is >= 50% of the servers available diskspace, we'll be adding an artifact greater than this... | 10:50 |
tlater | jmac: That's what I figured from my fairly limited exploration... I also assume that this means the client expects the tarfile to upload without the pipe ever closing, which causes the various BrokenPipeErrors | 10:51 |
tristan | Do we launch it in a subprocess ? | 10:51 |
jmac | tlater: Yeah, there's no feedback - the server can't tell the client to stop | 10:51 |
tristan | I dont think we do anymore | 10:51 |
jennis | tristan, yes | 10:51 |
tristan | we do ? | 10:51 |
tristan | So... we're just chasing after some error messages which are produced by a failed push... in a subprocess ? | 10:52 |
tristan | Like, do we really care how gracefully the subprocess exits ? | 10:52 |
jennis | https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_artifactcache/pushreceive.py#L441 | 10:52 |
tlater | Yep, we can probably just ignore those exceptions there | 10:53 |
tristan | ohhh you mean the ssh command | 10:54 |
tlater | Ah, nevermind | 10:54 |
tlater | I suppose we could ignore the exceptions in pushqueue? | 10:55 |
tristan | well, how painful can it be to handle the exceptions and abort the push ? | 10:55 |
* tristan a bit stumped, it's painful, it complains... | 10:55 | |
gitlab-br-bot | buildstream: merge request (jennis/136-clean-remote-cache->master: WIP: jennis/136 clean remote cache) #421 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/421 | 10:56 |
tristan | This *is* all happening in a child task, though | 10:56 |
tristan | I mean, handle it all and raise ArtifactError() for the push and we'll get a nice clean error message in the frontend | 10:56 |
tlater | jennis: Have you tried catching the exception in ostreecache.py instead of pushreceive.py? | 10:58 |
* tlater suspects that will give a nice, single spot to deal with | 10:58 | |
tlater | _push_to_remote() looks to be the place | 10:59 |
tristan | Right, *try* to be careful about generalizing too much on the exception, if it's specifically a broken pipe / hangup exception that we catch there then that is nice | 11:01 |
tristan | (because it's nice to keep unexpected exceptions reported as BUG with stacktrace) | 11:01 |
tristan | But dont try to distinguish between a general network failure broken pipe, and the remote cache having refused the artifact | 11:02 |
tristan | That is something we should have with CAS eventually, but not worthwhile here | 11:02 |
jennis | tristan, no, I will see where I can get with that | 11:03 |
tristan | abderrahim[m], thanks for the report ! ... looks like more whackamole to do in Element._update_state() | 11:06 |
abderrahim[m] | 😄 | 11:08 |
tlater | tristan: Hrm, I don't like how the cleanup jobs fit inside this queue thing we cooked up - yes, it's just another job for a queue to launch, but it also needs special casing because we want to run it exclusively. | 11:11 |
tlater | That is, when a cleanup job is launched we need to 1) wait for every current job to finish, 2) keep the scheduler from launching new jobs, 3) launch the cleanup job | 11:12 |
tlater | This is so we avoid changing the cache contents while we're busy cleaning up | 11:12 |
tlater | Actually, perhaps we could only stop launching build/pull jobs | 11:13 |
tlater | But anything that could add objects to the cache must not run, because the cleanup job could accidentally remove the committed objects before they are assigned a ref | 11:13 |
tlater | In either case, that means that the scheduler has to change what it's doing because we're running a cleanup job | 11:14 |
tlater | So it can't just be another job we submit | 11:14 |
tlater | Alternatively we can make sure the artifactcache is locked, but we'd need to do so across threads, which I think would require a lockfile? | 11:15 |
tristan | tlater, dont forget about the 3 value threshold | 11:19 |
tristan | which we discussed | 11:19 |
* tlater does recall that, but isn't sure how it solves the problem of cache cleanup needing to be atomic. | 11:20 | |
jennis | so basically there problem is with the PushCommandType, as jmac noticed earlier | 11:20 |
jennis | http://termbin.com/iyvo9 | 11:20 |
tristan | tlater, we launch the cleanup at the median, with a desire to bring it down to the first value, we only stop queueing more jobs when we reach the limit | 11:20 |
jennis | Seems to be returning a mu for byte-order | 11:20 |
tristan | tlater, it must not be atomic, it must be launched with a decision of exactly what it's going to do, which artifacts it's going to cleanup... it's execution is finite right ? | 11:21 |
tlater | Hrm, yeah, that works, but it still means that the cleanup job is special | 11:21 |
tristan | of course it's special | 11:21 |
tristan | It is *totally* special and nothing like the event notifications | 11:21 |
tristan | it needs business logic to caress it | 11:21 |
tristan | tlater, and it should not delete things which are deduplicated parts of ongoing concurrent caching of other things :) | 11:22 |
tristan | there is a big gotcha :) | 11:22 |
tlater | tristan: Could you explain that with a bit more detail? I feel I'm missing another thing buildstream does here. | 11:23 |
tristan | tlater, it caches artifacts | 11:23 |
tristan | while you are deleting them | 11:23 |
tristan | otherwise, it has to be atomic | 11:23 |
tristan | which sucks | 11:23 |
tlater | Yep, and that's a big problem | 11:24 |
tlater | Because ostree apparently only assigns a ref after the objects have been committed | 11:24 |
tristan | tlater, maybe we can solve that in CAS artifact caches only | 11:24 |
* tlater is afraid the cache deletion has to be atomic until we get CAS | 11:24 | |
tristan | tlater, if the atomic thing happens not very often (large thresholds), then it wont be too annoying for now | 11:24 |
tlater | I'll concede cleanup jobs their snowflake status then. | 11:26 |
tlater | jennis: That mu may very well just be a glitch | 11:26 |
tlater | What's important is that the client should handle erroneous data gracefully, and not crash | 11:26 |
tlater | Note that this might already happen due to network flukes | 11:27 |
tlater | Of course, we can't know whether a network fluke or a too-large-artifact caused this | 11:27 |
tlater | But that distinction doesn't matter enough that we should block a stopgap solution for cache expiry on it | 11:27 |
tlater | So if you just handle that error by warning the user that the push failed, it's fine | 11:27 |
Nexus | tristan: In your email, what do you mean by "Downloading the build tree of an artifact must only ever be done on demand"? | 11:28 |
tlater | jennis: Though... Why is it trying to figure out a byte order there? | 11:29 |
tlater | Does that exception happen during initialization? The byte order should be known by the time we have started uploading an artifact, no? | 11:29 |
tlater | Ah, right, every message has a byte order attached... Even the done message, and if the pipe doesn't contain data, that byte order is mu, for some reason. | 11:31 |
tristan | Nexus, I mean on demand, in the few corner cases that we need it, not in normal operation | 11:32 |
tristan | Nexus, I think I went over that in the email too | 11:32 |
Nexus | i'm a bit confused as to what you mean by "downloading the build tree" | 11:33 |
tristan | Nexus, for instance, I should have a choice when opening a workspace... Do I wanna wait and download 6GB of build tree so that I can have an incremental build of WebKit ? | 11:33 |
Nexus | is this assuming a remote cache? | 11:33 |
tristan | Yes | 11:33 |
Nexus | ok | 11:33 |
tristan | If it's cached locally that's a different story indeed | 11:33 |
Nexus | currently there is the option when opening a workspace to add the flag "--no-cache" | 11:33 |
Nexus | would you prefer that be the default? | 11:33 |
tristan | This is bigger than just that | 11:34 |
tristan | Currently you will cache the build trees and upload them unconditionally, that part is correct | 11:34 |
Nexus | Conditional on your not telling buildstream not to, yes | 11:34 |
tristan | Currently people want to `bst build epiphany.bst`, they dont need cached build trees of everything to do it, but we're forcing the downloads on users, that's really bad. | 11:35 |
Nexus | then you could set the variable `cache-build-tree` to false in your project.conf | 11:35 |
tristan | tlater, I'm gonna go now... but dont think of doing crazy things like autosuspending ongoing tasks; wait for tasks to complete | 11:35 |
gitlab-br-bot | buildstream: merge request (378-make-bst-here-more-flexible->master: Resolve "Make `bst-here` more flexible") #439 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/439 | 11:36 |
tlater | tristan: Don't worry, they'll gracefully end ;p | 11:36 |
tristan | Nexus, that wont stop you from downloading the entire cached artifact from the remote, where it resides | 11:36 |
Nexus | is that a thing that currently exists seperate to my code? | 11:36 |
tristan | Nexus, I want to `bst build epiphany.bst`, I dont wanna download 20GB of stuff I dont need | 11:36 |
jennis | So I guess to be safe, I should return a flag if we've encountered a too large artifact and then force the byte order to be something reasonable | 11:36 |
Nexus | because my code never uses the cache for a build | 11:37 |
tristan | Nexus, your code caches a build tree in the artifact yes ? | 11:37 |
tlater | Nexus: `cache-build-tree` only stops you from *uploading* cached build trees, but if someone has already uploaded one, you'll have to download a massive artifact. | 11:37 |
tristan | that's kind of the plan | 11:37 |
Nexus | tlater: yes, during opening a workspace only | 11:37 |
tristan | Nah, a remote artifact cache should always be complete, you dont get something that depends on build configuration of the uploader | 11:38 |
tristan | that'll never happen | 11:38 |
tristan | Nexus, if the uploader uploaded everything (which it should)... the downloader currently *has* to download everything (which is horrible) | 11:38 |
gitlab-br-bot | buildstream: merge request (valentindavid/update_mirror->master: WIP: Valentindavid/update mirror) #440 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/440 | 11:42 |
Nexus | tristan: kk i just chatted with tlater. just to make sure i have this correct. Currently, when you build something, buildstream looks to see if it has that artifact. Once my code pushes up a build tree, that process will ALSO pull down a potentially huge build tree | 11:42 |
Nexus | correct? | 11:42 |
*** tristan has quit IRC | 11:43 | |
gitlab-br-bot | buildstream: merge request (valentindavid/etag->master: Store etag along with cache.) #441 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/441 | 11:43 |
jennis | oh so it looks as if it just returns any random char for the byteorder if the pipe contains no data | 11:44 |
gitlab-br-bot | buildstream: merge request (valentindavid/update_mirror->master: WIP: Valentindavid/update mirror) #440 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/440 | 11:44 |
jennis | Do we think forcing the byteorder is the right think to do here? | 11:45 |
jennis | Seems like it could potentially disguise other problems | 11:45 |
tlater | jennis: That code is down to networking details | 11:45 |
tlater | If the byteorder fails to parse, we should handle the error gracefully (i.e., abort pushing, but give the user a warning) | 11:45 |
tlater | I don't think you want to force the byte order | 11:46 |
jennis | So you're saying, instead of raise the PushException, abort pushing...? | 11:48 |
jennis | https://gitlab.com/BuildStream/buildstream/blob/master/buildstream/_artifactcache/pushreceive.py#L82 | 11:48 |
tlater | What I'm saying is, keep that code as-is, but wherever we call it, we need to handle a PushException and warn the user instead of crashing | 11:49 |
tlater | Because you may receive an invalid byte order - that's fine | 11:49 |
tlater | What's not fine is crashing when we do | 11:49 |
jmac | Is the byte order actually used for anything other than the size field? | 11:50 |
tlater | jmac: I'm pretty sure this is to avoid problems with little/big endian | 11:50 |
tlater | Because that's not guaranteed when sending something over the network, iirc | 11:50 |
tlater | But juergbi should know more about that | 11:50 |
jmac | That would generally be the use for a byte order field | 11:50 |
tlater | Oh, you mean we don't actually act upon it? | 11:51 |
jmac | I'm not sure | 11:54 |
tlater | jennis: One thing to consider is if randomly receiving a correct byte order would break something | 11:54 |
jmac | The GLib.Variant thing is obscuring thing a bit | 11:54 |
jmac | Yeah, it looks like we have the capability to byteswap arguments but all the arguments we use are strings, bytes or an integer that's always 0 | 11:58 |
jmac | No big deal anyway | 11:59 |
*** tristan has joined #buildstream | 12:02 | |
* jennis is stumped | 12:05 | |
jennis | I'll see if I get any ideas after lunch :p | 12:06 |
Nexus | tristan: So in what i have written, the build tree is currently being saved into a new subdir of the artifact. Could we make a minor change to buildstreams current system, to make it exclude the build tree dir i added? | 12:06 |
jjardon | tlater: what is the bst-here image? | 12:37 |
tlater | jjardon: It's the image we'd actually like to be used exclusively for this script: https://gitlab.com/BuildStream/buildstream/blob/master/contrib/bst-here | 12:37 |
tlater | I.e., buildstream-fedora | 12:37 |
tlater | buildstream-fedora should be split into testsuite/fedora-27 and buildstream | 12:38 |
tlater | The one being used exclusively for tests, and the other exclusively for that script | 12:38 |
* tlater is considering writing to the ML on this topic, because I'm not sure how aware people are of what the images repo should look like... Or what various discussions here have thought it should look like. | 12:39 | |
cs_shadow | tlater: I was also thinking of updating the README of http://gitlab.com/buildstream/buildstream-docker-images/ to document what the different images are and when they should be used | 12:56 |
cs_shadow | if you wish to start a ML thread, that might be even better to gather various opinions about this | 12:56 |
jjardon | cs_shadow: +1 | 12:57 |
tlater | Yep, that's a great idea | 12:58 |
tlater | I'll send an email later today :) | 12:58 |
* jennis is unsure how to deal with these exceptions | 13:32 | |
jennis | I've jumped down a rabbit hole handling them, until I've hit a general exception, and blindly handling that will fail the build and provide no useful output | 13:33 |
jennis | useful debugging | 13:33 |
jennis | * | 13:33 |
jennis | I'm also dubious to add too many try/except clauses | 13:36 |
jennis | ^ for such an unlikely use-case | 13:36 |
gitlab-br-bot | buildstream: merge request (jennis/136-clean-remote-cache->master: WIP: jennis/136 clean remote cache) #421 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/421 | 14:04 |
*** Prince781 has joined #buildstream | 14:09 | |
tlater | jennis: You should be able to handle all or almost all of those exceptions in a single try/except clause | 14:14 |
jennis | yes, i've just done some rework, still had to use a bare-except though | 14:15 |
jennis | Which has resulted in the log being pretty useless | 14:15 |
tlater | Erk | 14:15 |
tlater | Are you sure you need a bare except? I really doubt that | 14:16 |
jennis | https://gitlab.com/BuildStream/buildstream/merge_requests/421/diffs#1a607748038846ed9a81a1ae319de4df329ce8e6_649_701 | 14:17 |
*** Prince781 has quit IRC | 14:17 | |
tlater | jennis: Are you aware that you can catch multiple exceptions? | 14:18 |
tlater | Yes you are... | 14:19 |
tlater | Why are you catching a bare except there? | 14:19 |
jennis | tlater, yes, but it's because the log contained a "raise Exception" | 14:20 |
tlater | You should be able to change that :) | 14:21 |
jennis | oh nvm, I haven't managed to obtain that again | 14:21 |
jennis | I had a return statement I forgot to remove earlier | 14:22 |
*** Prince781 has joined #buildstream | 14:22 | |
jennis | but now back to a log output with no debugging help | 14:22 |
gitlab-br-bot | buildstream: merge request (jjardon/debian-9->master: .gitlab-ci.yml: Run test in current Debian stable (stretch)) #425 changed state ("merged"): https://gitlab.com/BuildStream/buildstream/merge_requests/425 | 14:42 |
gitlab-br-bot | buildstream: issue #371 ("Run tests in Debian stable") changed state ("closed") https://gitlab.com/BuildStream/buildstream/issues/371 | 14:42 |
gitlab-br-bot | buildstream: merge request (jennis/136-clean-remote-cache->master: WIP: jennis/136 clean remote cache) #421 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/421 | 15:19 |
*** noisecell has quit IRC | 15:26 | |
jmac | I'm looking at _set_deterministic_user in utils.py. It uses the bst user's euid/egid, which doesn't seem very determinisitic to me. | 15:44 |
jmac | I'm wondering if it has some other intended function. | 15:54 |
*** dominic has quit IRC | 16:19 | |
tlater | Does anyone happen to know how glib variants work in a bit more detail? | 16:23 |
tlater | Specifically, ostree commit variants: https://lazka.github.io/pgi-docs/OSTree-1.0/classes/Repo.html#OSTree.Repo.load_commit | 16:23 |
* tlater is very confused as to what the object he gets returned contains... | 16:23 | |
*** bethw has quit IRC | 16:42 | |
juergbi | tlater: a (serialized) GVariant object is an immutable byte array that conforms to the GVariant format and individual parts of it can be retrieved using g_variant_get and co. | 16:43 |
juergbi | see https://developer.gnome.org/glib/stable/glib-GVariant.html for the full documentation | 16:43 |
*** xjuan has joined #buildstream | 16:46 | |
*** toscalix has quit IRC | 16:50 | |
gitlab-br-bot | buildstream: merge request (jennis/136-clean-remote-cache->master: WIP: jennis/136 clean remote cache) #421 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/421 | 16:51 |
jennis | I've handled the BrokenPipeError exception by simply forcing "pushed" to True, however this is now breaking the other expiry tests as the last element is not being pushed to the cache | 16:52 |
jennis | Which would only happen if the size of the tar_info object is greater than the quota | 16:53 |
jennis | So when we individually push three, 5 MB artifacts, by the time it comes to receiving the third one, is the tar_info object 15 MB? | 16:53 |
tlater | ty juergbi | 16:53 |
tlater | jennis: What you might be looking at is slight inaccuracies | 16:55 |
tlater | Depending on how compressible your data is you might get wildly varying actual artifact sizes in the cache | 16:55 |
jennis | Where wildly varying is on a scale of MBs? | 16:56 |
tlater | Yeah, could be, they can shrink quite a lot | 16:56 |
tlater | I don't think the tar_info object size will change... | 16:56 |
tlater | Can you inspect that? | 16:56 |
jennis | No, but will it represent the size of all compressed objects in the cache? | 16:57 |
tlater | Not afaik, I don't think all artifacts are pushed simultaneously, let alone in the same tar stream | 16:57 |
jennis | This is so bizarre | 16:57 |
tlater | jennis: A well-placed exception might help debug things ;) | 16:59 |
gitlab-br-bot | buildstream: merge request (214-filter-workspacing->master: Make workspace commands on a filter element transparently pass through to their build-dependency) #317 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/317 | 17:02 |
*** jennis has quit IRC | 17:04 | |
*** jonathanmaw has quit IRC | 17:09 | |
*** Prince781 has quit IRC | 17:49 | |
*** xjuan has quit IRC | 20:04 | |
*** mattis has joined #buildstream | 20:17 | |
mattis | hi | 20:17 |
mattis | Kein OP | 20:18 |
*** mattis has left #buildstream | 20:18 | |
*** mattis has joined #buildstream | 20:18 | |
*** mattis is now known as NotBanForYou | 20:22 | |
*** NotBanForYou has left #buildstream | 20:26 | |
*** Prince781 has joined #buildstream | 20:29 | |
*** xjuan has joined #buildstream | 20:47 | |
*** xjuan has quit IRC | 20:56 | |
*** cs_shadow has quit IRC | 21:16 | |
*** aday has quit IRC | 21:50 | |
*** bochecha_ has quit IRC | 21:54 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!