*** mohan43u has quit IRC | 00:14 | |
*** Prince781 has joined #buildstream | 04:08 | |
*** tristan has joined #buildstream | 04:49 | |
*** Prince781 has quit IRC | 05:49 | |
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 | 06:16 |
---|---|---|
laurence | juergbi, jmac can we create gitlab issues for the remote execution tasks? | 06:25 |
juergbi | sure, that makes sense | 06:25 |
laurence | Nexus, could you add a link in the description here to say 'depends on #21' https://gitlab.com/BuildStream/buildstream/issues/311 | 06:25 |
laurence | I keep finding myself getting lost in those 3 related issues. | 06:26 |
laurence | juergbi, tvm | 06:26 |
laurence | Nexus, the 3 related issues I mention above being the fact that https://gitlab.com/BuildStream/buildstream/issues/21 couldn't be implemented without the functionality outlined in https://gitlab.com/BuildStream/buildstream/issues/311 | 06:31 |
laurence | but then that they are both now blocked on https://gitlab.com/BuildStream/buildstream/issues/376 | 06:32 |
laurence | let's just make the links between them a bit clearer on the tickets | 06:32 |
tristan | Yeah gitlab does not have the best facilities for issue dependencies | 06:32 |
tristan | unfortunately we're stuck just making links, bugzilla was nice in that you could mark an bug as blocking on another, and render dependency graphs | 06:33 |
laurence | So the work on remote cache expiry that jennis has done is waiting on the scheduler changes in the CAS branch, which is expected very soon. I'm slightly concerned about the final test that jennis has mentioned re an artifact being too large. | 07:22 |
laurence | tristan, to confirm, can the branch land without that test included? | 07:22 |
laurence | because jennis has only another week to be able to focus on this. | 07:23 |
laurence | before some holiday and then an assignment in NY for a week | 07:23 |
tristan | laurence, we discussed that A.) We dont want to spend too much effort making the existing cache that robust... B.) Should there be an error encountered on the server while pushing an artifact, the client really need only report that there was an error (the nature of which is not important for now), and the client simply must not crash as a result | 07:26 |
laurence | tristan, right, thanks, I'll let jennis answer later this morning on whether his patch meets those needs. | 07:27 |
laurence | I'm keen to get this tied up and I think jennis needs some reassuring about this final bit | 07:28 |
laurence | (we are all keen to get this one tied up of course) | 07:28 |
*** toscalix has joined #buildstream | 07:55 | |
*** finn has quit IRC | 08:05 | |
*** finn has joined #buildstream | 08:12 | |
*** WSalmon has joined #buildstream | 08:14 | |
*** aday has joined #buildstream | 08:21 | |
*** jonathanmaw has joined #buildstream | 08:30 | |
jennis | laurence, tristan, yes I remember this discussion however I've been unable to implement this to fulfil both requirements: i) Without the client side crashing and ii) the expiry working properly, so far, when I've fixed one of these problems, the other has broken | 08:33 |
jennis | However, now i'm considering whether simply raising an ArtifactError once we see that an artifact is too big will suffice | 08:35 |
tristan | jennis, "working properly" is a matter of degrees perfectionism from what I understand, as such point (A) above means; dont be too perfect, just dont crash and report that there was an error (probably due to the server side doing a hangup) | 08:38 |
tristan | from what I recall, the only thing about that subject was that the client was not responding "well" to a hangup | 08:38 |
tristan | This probably means catching the specific kind of exception at a high enough level and turning it into an ArtifactError() in the client | 08:39 |
*** WSalmon has quit IRC | 08:40 | |
*** WSalmon has joined #buildstream | 08:41 | |
jennis | tristan, ok thanks, I'll see what I can do this morning :) | 08:45 |
*** bethw has joined #buildstream | 08:45 | |
*** aday has quit IRC | 08:45 | |
jennis | juergbi, regarding the CAS cache (client side), are you going to be updating !337? | 08:53 |
*** tiago has joined #buildstream | 08:54 | |
jennis | https://gitlab.com/BuildStream/buildstream/merge_requests/337 | 08:54 |
*** aday has joined #buildstream | 08:56 | |
Nexus | laurence: done | 09:00 |
juergbi | jennis: yes, I will but I'll first open a separate MR for dynamic push/pull handling and then rebase CAS on top of that | 09:00 |
*** aday has quit IRC | 09:03 | |
*** aday has joined #buildstream | 09:05 | |
*** dominic has joined #buildstream | 09:11 | |
jennis | tristan, turning the error into an ArtifactError will cause the client to crash? | 09:15 |
jennis | Well, that's what seems to be the case for me | 09:16 |
tristan | This is in the context of a push task no ? | 09:16 |
tristan | jennis, this is what I expect... while pushing... if you dont handle the exception, we get BUG messages | 09:16 |
tristan | with stack traces | 09:17 |
gitlab-br-bot | buildstream: issue #387 ("Google CAS-based artifact cache") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/387 | 09:17 |
tristan | jennis, when turning it into an ArtifactError, we get a message that pushing failed | 09:17 |
jennis | http://termbin.com/essv | 09:17 |
jennis | ^^ yes tristan, exactly that | 09:18 |
jmac | laurence: The 404s I mentioned are being returned when I try to create an MR, and are still happening | 09:18 |
laurence | ah, annoying, but no huge rush jmac | 09:23 |
tristan | jennis, it's not horrible as it stands... currently we are however seeing the stack trace which occurred on the remote... as the stderr of the remote task is getting shown locally | 09:23 |
tristan | jennis, is it possible to not have the remote crash in that case ? | 09:24 |
tristan | jennis, in fact it would seem to me very important to fix that... the client side is doing the right thing btw | 09:26 |
tristan | jennis, if the remote crashes this way however and the exceptions remain unhandled, I suspect that means that it will fail to do the expected cleanup job when there is a failure on the receiver (remote) end | 09:26 |
tristan | which means when you push an artifact which is too big for the cache... you are using the amount of sent data in disk space, in a temp directory that never gets cleaned up | 09:27 |
tristan | jennis, actually looks like not the case... the close() method which does the cleanup will be run unconditionally, and the exception is re-raised | 09:29 |
jennis | 1 second, let me show you where this broken pipe error occurs | 09:29 |
tristan | jennis, however... that crash is happening because while receiving objects... it probably intentionally hangs up the connection | 09:29 |
jennis | yes, because I force a break | 09:29 |
tristan | and then it goes on to say "self.receive_done()" | 09:29 |
jennis | tristan, here: https://gitlab.com/BuildStream/buildstream/merge_requests/421/diffs#1a607748038846ed9a81a1ae319de4df329ce8e6_305_323 | 09:30 |
tristan | that part looks like a final handshake which it should not expect to succeed after having intentionally hungup | 09:30 |
tristan | jennis, right, so we do that and then after receive_putobjects(), we expect something unreasonable to happen after | 09:31 |
tristan | i.e. self.reader.receive_done() expects to go ahead and read the DONE message | 09:32 |
jennis | # Ensure that pusher has sent all objects | 09:32 |
jennis | self.reader.receive_done() | 09:32 |
jennis | ahh you beat me to it | 09:32 |
tristan | jennis, maybe that "break" should turn into an explicit exception | 09:32 |
jennis | Well this is also something I wanted to discuss | 09:32 |
tristan | which can be handled instead of blind, and also result in close() / cleanup | 09:32 |
jennis | Changing that break to a continue, will make all things go smoothly | 09:32 |
tristan | jennis, have a meeting starting right now... | 09:33 |
jennis | tristan, too engrossed... will continue in a minute :) | 09:36 |
gitlab-br-bot | buildstream: merge request (jmac/virtual_directories->master: WIP: Abstract directory class and filesystem-backed implementation) #445 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/445 | 09:40 |
jmac | ^ There we go | 09:40 |
jennis | tristan, there is another point I would like to discuss before I proceed | 09:45 |
jennis | Changing that break to a continue statement... | 09:46 |
jennis | This will run *almost* fine, however bst still thinks the large artifact is in the share, BUT when I cd into the share and check the diskspace `du -sh`, it's clearly not wholly been added | 09:47 |
jennis | `ostree refs` lists the refs in the share and we see "large_artifact" in there | 09:47 |
jennis | Do you think this could possibly open up another way to deal with this, or shall we stick with break (i.e. terminating the connection) and handling errors? | 09:48 |
tristan | jennis, basically with "continue" you insist that the client goes ahead and continues to upload the enormous artifact which we're going to ignore | 10:05 |
tristan | jennis, I think you should try `raise ArtifactIsTooDamnBig()` instead of `break` or `continue` | 10:06 |
jennis | Not in my implementation | 10:06 |
jennis | https://gitlab.com/BuildStream/buildstream/merge_requests/421/diffs#1a607748038846ed9a81a1ae319de4df329ce8e6_305_323 | 10:07 |
jennis | see the if else statement | 10:07 |
jennis | It won't extract it | 10:07 |
tristan | jennis, and in OSTreeReceiver.run(), you explicitly handle ArtifactIsTooDamnBig in the try block... and move self.close() to a finally: clause instead of repeating it | 10:07 |
jennis | This makes a bit more sense | 10:08 |
tristan | jennis, that if / else block is in the *receiver*, doing continue fails to interrupt the upload | 10:09 |
jennis | (Sorry, I've not had much experience with exception handling) | 10:09 |
tristan | jennis, so basically you would remove the "BLIND EXCEPT" (in allcaps in that comment) completely, and you would just print "Artifact is too damn big" to stderr when handling the ArtifactIsTooDamnBig exception | 10:10 |
tristan | the result will be that server side will cleanly exit immediately after cleaning up when it wants to, without a stack trace, other exceptions will still be raised if they happen (they will fall right through the try block, but finally: will do the cleanup) | 10:11 |
tristan | And on the client side, we will see "Artifact is too damn big" in the error log | 10:11 |
jennis | tristan, thank you, I'll get on that now | 10:14 |
laurence | tristan, not urgent, but for the redirection of workspace commands, do you expect major re-factoring of jonathanmaw's branch being required? | 10:15 |
laurence | tristan, for context, i was reading some of the scroll back from a few days ago, and this is about the pipeline refactoring, and changes to the initialization process | 10:15 |
tristan | laurence, I expect that for some parts of the branch which touch the higher levels, we will have to look at what was done, and logically re-apply it | 10:20 |
tristan | from what I understood, the branch was dangerously misguided by an already weird Pipeline structure | 10:21 |
jennis | tristan, raising an ArtifactTooBig error will terminate the connection and lead to a BrokenPipeError | 10:35 |
jennis | So we're back to the same problem | 10:36 |
tristan | jennis, not back to the same problem, why ? | 10:37 |
tiago | :q | 10:37 |
tristan | jennis, http://termbin.com/essv <-- this is what you are saying happens when the client receives a nice BrokenPipeError which you've elegantly caught to detect the error on the client side, is that not correct ? | 10:39 |
jennis | yes thats correct, apologies, I thought I was barking up the wrong tree so reverted those changes | 10:41 |
tristan | so looking at that paste, we're going to see a nice message in that log file which came from the remote, instead of the stack trace which occurred in the remote | 10:42 |
tristan | jennis, it's a bit confusing because it's hard to "just know" that the stack trace comes from the remote | 10:42 |
tristan | but that stack trace does, it's just in the log by virtue of the stdout/stderr being captured by the client (which is nice for debugging) | 10:43 |
*** aday has quit IRC | 10:47 | |
*** tristan has quit IRC | 11:21 | |
*** aday has joined #buildstream | 11:26 | |
*** aday has joined #buildstream | 11:27 | |
jennis | ok... I've reverted a lot of the changes and gone back to just dealing with the BrokenPipeError | 11:37 |
jennis | and all the tests are passing | 11:37 |
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 | 11:49 |
*** tristan has joined #buildstream | 11:53 | |
tlater | jennis: It might be worth double checking that your MR is up-to-date with mine | 12:28 |
tlater | I.e., the functions you borrow from my patch might have changed since you first cherry-picked them | 12:28 |
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 | 13:00 |
jennis | tlater, I'll have a look, thanks | 13:05 |
jmac | tests/frontend/push.py::test_push_all | 13:06 |
jmac | ^ seems to be hanging for me occasionally, anyone else seen that? | 13:06 |
jennis | ? | 13:06 |
jennis | Locally? or on Gitlab? | 13:07 |
jmac | Locally. | 13:07 |
jennis | It's fine for me | 13:07 |
*** xjuan has joined #buildstream | 13:15 | |
tristan | jmac, Happens to me also, haven't had time to debug it | 13:15 |
jmac | Right, I'll see if I can see anything | 13:16 |
Nexus | i'm a bit confused by "needed_commits(" in pushreceive.py | 13:23 |
Nexus | won't the last entry ref in remote never get added to needed? | 13:23 |
jennis | tristan, is this the kind of push log you would expect to see: http://termbin.com/nqa9 ? | 13:29 |
jennis | Obviously there is that fact that it's still saying "Pushing artifact" was a success | 13:29 |
tristan | jennis, nope, but it looks close | 13:30 |
jennis | So now, I have everything going as planned, except from the fact we have this in the push log | 13:30 |
tristan | success looks wrong but, maybe the client cannot know that if it managed to push before seeing a hangup | 13:30 |
tristan | jennis, ... this part I suspect you missed: | 13:31 |
tristan | <tristan> jennis, so basically you would remove the "BLIND EXCEPT" (in allcaps in that comment) completely, and you would just print "Artifact is too damn big" to stderr when handling the ArtifactIsTooDamnBig exception | 13:31 |
jennis | Yes, so instead I used ArtifactError as this is a general error for artifacts | 13:31 |
jennis | raise ArtifactError("Artifact was too big for the cache, so has not been pushed.") | 13:31 |
jennis | So you'd be happy if I just replaced that with ArtifactTooBigError? | 13:32 |
tristan | Uhhh | 13:32 |
tristan | jennis, Ok so two things... for now, artifacts failing to be pushed is an error you might not catch | 13:32 |
tristan | which seems fine... you push it... and that's all, there is no handshake at the end for the client to know | 13:33 |
tristan | but that is not a big deal | 13:33 |
tristan | jennis, it's important that you understand that this is not an error about failing to push it on the client side | 13:33 |
tristan | this is an error about failing to receive it (or rather refusing to receive it) on the server | 13:33 |
tristan | ArtifactError by itself works differently in the client... BstError is handled specially by core mechanics | 13:34 |
tristan | jennis, on the receiver, you need an error to indicate that you have stopped reading the incomming TarStream | 13:34 |
tristan | jennis, at the toplevel, ... I'll then repeat again: | 13:35 |
tristan | <tristan> <tristan> jennis, so basically you would remove the "BLIND EXCEPT" (in allcaps in that comment) completely, and you would just print "Artifact is too damn big" to stderr when handling the ArtifactIsTooDamnBig exception | 13:35 |
jennis | Oh so a new type of Error that does not inherit from BstError? | 13:35 |
tristan | jennis, this means... you *print* to stderr in the receiver that you dropped it... you do not raise yet another error | 13:35 |
tristan | Actually that output looks even more confusing | 13:36 |
tristan | it looks like | 13:36 |
tristan | you did not actually raise an error instead of the "break" statement we were talking about | 13:36 |
jennis | No because I thought this workaround looked right | 13:36 |
tristan | jennis, you have an error coming from receive_done() | 13:37 |
tristan | jennis, which means... even after you stopped reading the TarStream ... you keep reading the stream and expect a DONE message there | 13:37 |
tristan | jennis, follow my instructions from before | 13:37 |
tristan | Raise a specialized error *instead* of "break", at the line we were discussing | 13:37 |
jennis | and then catch that error and print to stderr? | 13:38 |
tristan | Handle that error specially, dont keep reading incomming TarStream and expecting it to be a DONE message | 13:38 |
tristan | Printing it to stderr in the receiver, will mean it will come to the client in the log | 13:38 |
tristan | instead of python's default exception handler printing a stack trace to stderr, and that stderr getting sent back to the client | 13:39 |
tristan | Remember, even though you are seeing a stack trace, the client is not experiencing one. | 13:39 |
tristan | jennis, Yes: Catch that error and dont reraise it, print something informative to stderr | 13:40 |
jennis | Simply with sys.stderr.write() ? | 13:41 |
tristan | that can work, but it would be weird with the current codebase | 13:42 |
jennis | ahh we use logging for this purpose? | 13:42 |
tristan | jennis, notice that pushreceive.py uses the "logging" module for printing the things we're going to see in the client | 13:42 |
tristan | I suppose here logging.warning would be good | 13:42 |
jennis | ok, thank you tristan | 13:44 |
Nexus | who's a good OSTree person to quiz? | 13:49 |
paulsherwood | jjardon: ^^ ? | 13:55 |
jjardon | Nexus: probably the best place is the #ostree channel | 13:55 |
Nexus | kk thanks | 13:56 |
*** tristan has quit IRC | 14:01 | |
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:42 |
toscalix | in prep for the release and guadec, do we have a twitter account? I see that @buildstream is taken | 14:45 |
*** tristan has joined #buildstream | 14:52 | |
toscalix | I take this as a no. I will propose something through the ML | 15:06 |
finn | Hey, stupid question time from me | 15:14 |
finn | Looking at a .bst file, it has this URL in: | 15:14 |
finn | url: dockerhub:buildstream/image-tools/ | 15:14 |
finn | should that final "/" matter? | 15:14 |
finn | Better question: Should that final forward slash be included or not or should it not matter? | 15:16 |
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:22 |
jennis | No finn, I thought I patched that | 15:22 |
jennis | It shouldn't be there | 15:22 |
jennis | Otherwise we end up with a url with // in there somewhere | 15:23 |
finn | exactly ;) | 15:23 |
jennis | aha https://gitlab.com/BuildStream/buildstream-examples/merge_requests/4 | 15:23 |
finn | Just checking | 15:24 |
finn | I'd like to get that build-86image working again | 15:27 |
* jennis also thinks that would be a good first task | 15:28 | |
tristan | Is there a reason we are adding merge requests to the buildstream-examples repo ? | 15:35 |
tristan | Would we not do better to delete, or at least archive it as properly 'dead' for posterity ? | 15:35 |
tristan | as I recall, I created that to show some people how things worked at very early stages, and we've moved on and have pushed whatever relevant stuff that was there to downstream projects | 15:36 |
tristan | seems like baggage not worth carrying | 15:36 |
finn | Is there currently a simple example one can follow which doesn't take hours to build? | 15:41 |
tristan | finn, not which one can follow no, jennis is busy but has a plan in place to add a getting started guide + a first example which sets a precedent for how we add examples in the docs | 15:43 |
finn | What will the first example be? | 15:43 |
tristan | finn, to go "from scratch", you might look at one of the tests in tests/integration | 15:43 |
finn | Thanks | 15:43 |
tristan | which will use the alpine image which is the minimal image we currently build on | 15:44 |
tristan | (it's the smallest runtime we know of, but we'll probably eventually switch that for a buildstream-built image from freedesktop-sdk once we can hone it down to a small enough size) | 15:44 |
tristan | jennis, finn seeing your above topic, I'm not sure if this is relevant for docker urls but we do usually include a trailing slash in alias definitions (although BuildStream doesnt care) | 15:49 |
tristan | i.e. it allows one to specify in a .bst file something like: foo:bar.git | 15:49 |
tristan | instead of foo:/bar.git | 15:49 |
albfan[m] | tristan: Did some ever try buildstream on windows? like on top of msys2. Do you think it should work? | 16:02 |
gitlab-br-bot | buildstream: merge request (jmac/virtual_directories->master: WIP: Abstract directory class and filesystem-backed implementation) #445 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/445 | 16:07 |
tlater | Hrm, can we avoid requiring two-factor auth for buildstream? I understand the point, but it seems my only option is to go through some Google application through my phone, which would require me to install gapps... | 16:11 |
tlater | This seems like an even bigger issue for people who don't have a smartphone. | 16:12 |
jennis | tlater, are you referring to the examples-MR? | 16:12 |
tlater | No, I get a popup from gitlab forcing me to enable two-factor auth when I try to open the buildstream group. | 16:13 |
tlater | Probably because of my permissions... | 16:13 |
jmac | ! | 16:13 |
* tlater suspects toscalix changed the group settings | 16:13 | |
*** bochecha_ has joined #buildstream | 16:13 | |
toscalix | tlater: you have to go over it the very first time, not later. It would only affect the new subgroup, not the current repos. | 16:15 |
toscalix | but yes, I can skip it | 16:16 |
toscalix | tlater: I only changed the subgroup settings | 16:16 |
toscalix | double checking | 16:16 |
toscalix | exactly, the two-facto auth is only required in the nosoftware subgroup, so no changes in the current repos | 16:18 |
albfan[m] | tristan: it depends on bubblewrap so I suppose it is not possible | 16:18 |
toscalix | tlater: disabled. I really like the 2FA | 16:21 |
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:21 |
*** cs_shadow has joined #buildstream | 16:25 | |
tlater | toscalix: Yeah, no worries, it's just very usable if you don't sell your soul to Google ;) | 16:25 |
toscalix | haven't we all aready? | 16:26 |
toscalix | s/aready/already | 16:26 |
tristan | albfan[m], I'm hoping for a WSL based sandbox implementation for building linux stuff on win32, but it's a bit ambitious | 16:30 |
tristan | albfan[m], building native win32 can be interesting, needs a lot of investigation though | 16:30 |
*** Prince781 has joined #buildstream | 16:33 | |
albfan[m] | someone is looking for develop (not only build on windows gtk3) and I though on buildstream. https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-gtk3-git or https://github.com/wingtk/gvsbuild are the best options by now | 16:36 |
*** bochecha_ has quit IRC | 16:49 | |
*** Prince781 has quit IRC | 16:57 | |
*** toscalix has quit IRC | 17:02 | |
*** bethw has quit IRC | 17:03 | |
*** aday has quit IRC | 17:07 | |
*** solid_black has joined #buildstream | 17:13 | |
*** aday has joined #buildstream | 17:33 | |
gitlab-br-bot | buildstream: merge request (jmac/virtual_directories->master: WIP: Abstract directory class and filesystem-backed implementation) #445 changed state ("opened"): https://gitlab.com/BuildStream/buildstream/merge_requests/445 | 17:44 |
*** solid_black has quit IRC | 17:51 | |
*** jonathanmaw has quit IRC | 17:54 | |
*** aday has quit IRC | 18:09 | |
*** tristan has quit IRC | 18:19 | |
*** Prince781 has joined #buildstream | 18:24 | |
*** xjuan has quit IRC | 18:27 | |
*** Prince781 has quit IRC | 18:46 | |
*** xjuan has joined #buildstream | 19:02 | |
*** xjuan has joined #buildstream | 19:02 | |
*** bochecha_ has joined #buildstream | 19:37 | |
*** xjuan has quit IRC | 20:29 | |
*** aday has joined #buildstream | 21:22 | |
*** aday has quit IRC | 21:25 | |
*** bochecha_ has quit IRC | 21:43 |
Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!