IRC logs for #buildstream for Wednesday, 2018-05-02

*** mohan43u has quit IRC00:14
*** Prince781 has joined #buildstream04:08
*** tristan has joined #buildstream04:49
*** Prince781 has quit IRC05:49
gitlab-br-botbuildstream: 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/42106:16
laurencejuergbi, jmac can we create gitlab issues for the remote execution tasks?06:25
juergbisure, that makes sense06:25
laurenceNexus, could you add a link in the description here to say 'depends on #21' https://gitlab.com/BuildStream/buildstream/issues/31106:25
laurenceI keep finding myself getting lost in those 3 related issues.06:26
laurencejuergbi, tvm06:26
laurenceNexus, 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/31106:31
laurencebut then that they are both now blocked on https://gitlab.com/BuildStream/buildstream/issues/37606:32
laurencelet's just make the links between them a bit clearer on the tickets06:32
tristanYeah gitlab does not have the best facilities for issue dependencies06:32
tristanunfortunately we're stuck just making links, bugzilla was nice in that you could mark an bug as blocking on another, and render dependency graphs06:33
laurenceSo 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
laurencetristan, to confirm, can the branch land without that test included?07:22
laurencebecause jennis has only another week to be able to focus on this.07:23
laurencebefore some holiday and then an assignment in NY for a week07:23
tristanlaurence, 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 result07:26
laurencetristan, right, thanks, I'll let jennis answer later this morning on whether his patch meets those needs.07:27
laurenceI'm keen to get this tied up and I think jennis needs some reassuring about this final bit07:28
laurence(we are all keen to get this one tied up of course)07:28
*** toscalix has joined #buildstream07:55
*** finn has quit IRC08:05
*** finn has joined #buildstream08:12
*** WSalmon has joined #buildstream08:14
*** aday has joined #buildstream08:21
*** jonathanmaw has joined #buildstream08:30
jennislaurence, 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 broken08:33
jennisHowever, now i'm considering whether simply raising an ArtifactError once we see that an artifact is too big will suffice08:35
tristanjennis, "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
tristanfrom what I recall, the only thing about that subject was that the client was not responding "well" to a hangup08:38
tristanThis probably means catching the specific kind of exception at a high enough level and turning it into an ArtifactError() in the client08:39
*** WSalmon has quit IRC08:40
*** WSalmon has joined #buildstream08:41
jennistristan, ok thanks, I'll see what I can do this morning :)08:45
*** bethw has joined #buildstream08:45
*** aday has quit IRC08:45
jennisjuergbi, regarding the CAS cache (client side), are you going to be updating !337?08:53
*** tiago has joined #buildstream08:54
jennishttps://gitlab.com/BuildStream/buildstream/merge_requests/33708:54
*** aday has joined #buildstream08:56
Nexuslaurence: done09:00
juergbijennis: yes, I will but I'll first open a separate MR for dynamic push/pull handling and then rebase CAS on top of that09:00
*** aday has quit IRC09:03
*** aday has joined #buildstream09:05
*** dominic has joined #buildstream09:11
jennistristan, turning the error into an ArtifactError will cause the client to crash?09:15
jennisWell, that's what seems to be the case for me09:16
tristanThis is in the context of a push task no ?09:16
tristanjennis, this is what I expect... while pushing... if you dont handle the exception, we get BUG messages09:16
tristanwith stack traces09:17
gitlab-br-botbuildstream: issue #387 ("Google CAS-based artifact cache") changed state ("opened") https://gitlab.com/BuildStream/buildstream/issues/38709:17
tristanjennis, when turning it into an ArtifactError, we get a message that pushing failed09:17
jennishttp://termbin.com/essv09:17
jennis^^ yes tristan, exactly that09:18
jmaclaurence: The 404s I mentioned are being returned when I try to create an MR, and are still happening09:18
laurenceah, annoying, but no huge rush jmac09:23
tristanjennis, 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 locally09:23
tristanjennis, is it possible to not have the remote crash in that case ?09:24
tristanjennis, in fact it would seem to me very important to fix that... the client side is doing the right thing btw09:26
tristanjennis, 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) end09:26
tristanwhich 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 up09:27
tristanjennis, actually looks like not the case... the close() method which does the cleanup will be run unconditionally, and the exception is re-raised09:29
jennis1 second, let me show you where this broken pipe error occurs09:29
tristanjennis, however... that crash is happening because while receiving objects... it probably intentionally hangs up the connection09:29
jennisyes, because I force a break09:29
tristanand then it goes on to say "self.receive_done()"09:29
jennistristan, here: https://gitlab.com/BuildStream/buildstream/merge_requests/421/diffs#1a607748038846ed9a81a1ae319de4df329ce8e6_305_32309:30
tristanthat part looks like a final handshake which it should not expect to succeed after having intentionally hungup09:30
tristanjennis, right, so we do that and then after receive_putobjects(), we expect something unreasonable to happen after09:31
tristani.e. self.reader.receive_done() expects to go ahead and read the DONE message09:32
jennis        # Ensure that pusher has sent all objects09:32
jennis        self.reader.receive_done()09:32
jennisahh you beat me to it09:32
tristanjennis, maybe that "break" should turn into an explicit exception09:32
jennisWell this is also something I wanted to discuss09:32
tristanwhich can be handled instead of blind, and also result in close() / cleanup09:32
jennisChanging that break to a continue, will make all things go smoothly09:32
tristanjennis, have a meeting starting right now...09:33
jennistristan, too engrossed... will continue in a minute :)09:36
gitlab-br-botbuildstream: 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/44509:40
jmac^ There we go09:40
jennistristan, there is another point I would like to discuss before I proceed09:45
jennisChanging that break to a continue statement...09:46
jennisThis 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 added09:47
jennis`ostree refs` lists the refs in the share and we see "large_artifact" in there09:47
jennisDo 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
tristanjennis, basically with "continue" you insist that the client goes ahead and continues to upload the enormous artifact which we're going to ignore10:05
tristanjennis, I think you should try `raise ArtifactIsTooDamnBig()` instead of `break` or `continue`10:06
jennisNot in my implementation10:06
jennishttps://gitlab.com/BuildStream/buildstream/merge_requests/421/diffs#1a607748038846ed9a81a1ae319de4df329ce8e6_305_32310:07
jennissee the if else statement10:07
jennisIt won't extract it10:07
tristanjennis, and in OSTreeReceiver.run(), you explicitly handle ArtifactIsTooDamnBig in the try block... and move self.close() to a finally: clause instead of repeating it10:07
jennisThis makes a bit more sense10:08
tristanjennis, that if / else block is in the *receiver*, doing continue fails to interrupt the upload10:09
jennis(Sorry, I've not had much experience with exception handling)10:09
tristanjennis, 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 exception10:10
tristanthe 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
tristanAnd on the client side, we will see "Artifact is too damn big" in the error log10:11
jennistristan, thank you, I'll get on that now10:14
laurencetristan, not urgent, but for the redirection of workspace commands, do you expect major re-factoring of jonathanmaw's branch being required?10:15
laurencetristan, 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 process10:15
tristanlaurence, 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 it10:20
tristanfrom what I understood, the branch was dangerously misguided by an already weird Pipeline structure10:21
jennistristan, raising an ArtifactTooBig error will terminate the connection and lead to a BrokenPipeError10:35
jennisSo we're back to the same problem10:36
tristanjennis, not back to the same problem, why ?10:37
tiago:q10:37
tristanjennis, 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
jennisyes thats correct, apologies, I thought I was barking up the wrong tree so reverted those changes10:41
tristanso 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 remote10:42
tristanjennis, it's a bit confusing because it's hard to "just know" that the stack trace comes from the remote10:42
tristanbut 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 IRC10:47
*** tristan has quit IRC11:21
*** aday has joined #buildstream11:26
*** aday has joined #buildstream11:27
jennisok... I've reverted a lot of the changes and gone back to just dealing with the BrokenPipeError11:37
jennisand all the tests are passing11:37
gitlab-br-botbuildstream: 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/42111:49
*** tristan has joined #buildstream11:53
tlaterjennis: It might be worth double checking that your MR is up-to-date with mine12:28
tlaterI.e., the functions you borrow from my patch might have changed since you first cherry-picked them12:28
gitlab-br-botbuildstream: 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/42113:00
jennistlater, I'll have a look, thanks13:05
jmactests/frontend/push.py::test_push_all13:06
jmac^ seems to be hanging for me occasionally, anyone else seen that?13:06
jennis?13:06
jennisLocally? or on Gitlab?13:07
jmacLocally.13:07
jennisIt's fine for me13:07
*** xjuan has joined #buildstream13:15
tristanjmac, Happens to me also, haven't had time to debug it13:15
jmacRight, I'll see if I can see anything13:16
Nexusi'm a bit confused by "needed_commits(" in pushreceive.py13:23
Nexuswon't the last entry ref in remote never get added to needed?13:23
jennistristan, is this the kind of push log you would expect to see: http://termbin.com/nqa9 ?13:29
jennisObviously there is that fact that it's still saying "Pushing artifact" was a success13:29
tristanjennis, nope, but it looks close13:30
jennisSo now, I have everything going as planned, except from the fact we have this in the push log13:30
tristansuccess looks wrong but, maybe the client cannot know that if it managed to push before seeing a hangup13:30
tristanjennis, ... 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 exception13:31
jennisYes, so instead I used ArtifactError as this is a general error for artifacts13:31
jennisraise ArtifactError("Artifact was too big for the cache, so has not been pushed.")13:31
jennisSo you'd be happy if I just replaced that with ArtifactTooBigError?13:32
tristanUhhh13:32
tristanjennis, Ok so two things... for now, artifacts failing to be pushed is an error you might not catch13:32
tristanwhich seems fine... you push it... and that's all, there is no handshake at the end for the client to know13:33
tristanbut that is not a big deal13:33
tristanjennis, it's important that you understand that this is not an error about failing to push it on the client side13:33
tristanthis is an error about failing to receive it (or rather refusing to receive it) on the server13:33
tristanArtifactError by itself works differently in the client... BstError is handled specially by core mechanics13:34
tristanjennis, on the receiver, you need an error to indicate that you have stopped reading the incomming TarStream13:34
tristanjennis, 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 exception13:35
jennisOh so a new type of Error that does not inherit from BstError?13:35
tristanjennis, this means... you *print* to stderr in the receiver that you dropped it... you do not raise yet another error13:35
tristanActually that output looks even more confusing13:36
tristanit looks like13:36
tristanyou did not actually raise an error instead of the "break" statement we were talking about13:36
jennisNo because I thought this workaround looked right13:36
tristanjennis, you have an error coming from receive_done()13:37
tristanjennis, which means... even after you stopped reading the TarStream ... you keep reading the stream and expect a DONE message there13:37
tristanjennis, follow my instructions from before13:37
tristanRaise a specialized error *instead* of "break", at the line we were discussing13:37
jennisand then catch that error and print to stderr?13:38
tristanHandle that error specially, dont keep reading incomming TarStream and expecting it to be a DONE message13:38
tristanPrinting it to stderr in the receiver, will mean it will come to the client in the log13:38
tristaninstead of python's default exception handler printing a stack trace to stderr, and that stderr getting sent back to the client13:39
tristanRemember, even though you are seeing a stack trace, the client is not experiencing one.13:39
tristanjennis, Yes: Catch that error and dont reraise it, print something informative to stderr13:40
jennisSimply with sys.stderr.write() ?13:41
tristanthat can work, but it would be weird with the current codebase13:42
jennisahh we use logging for this purpose?13:42
tristanjennis, notice that pushreceive.py uses the "logging" module for printing the things we're going to see in the client13:42
tristanI suppose here logging.warning would be good13:42
jennisok, thank you tristan13:44
Nexuswho's a good OSTree person to quiz?13:49
paulsherwoodjjardon: ^^ ?13:55
jjardonNexus: probably the best place is the #ostree channel13:55
Nexuskk thanks13:56
*** tristan has quit IRC14:01
gitlab-br-botbuildstream: 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/42114:42
toscalixin prep for the release and guadec, do we have a twitter account? I see that @buildstream is taken14:45
*** tristan has joined #buildstream14:52
toscalixI take this as a no. I will propose something through the ML15:06
finnHey, stupid question time from me15:14
finnLooking at a .bst file, it has this URL in:15:14
finnurl: dockerhub:buildstream/image-tools/15:14
finnshould that final "/" matter?15:14
finnBetter question: Should that final forward slash be included or not or should it not matter?15:16
gitlab-br-botbuildstream: 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/42115:22
jennisNo finn, I thought I patched that15:22
jennisIt shouldn't be there15:22
jennisOtherwise we end up with a url with // in there somewhere15:23
finnexactly ;)15:23
jennisaha https://gitlab.com/BuildStream/buildstream-examples/merge_requests/415:23
finnJust checking15:24
finnI'd like to get that build-86image working again15:27
* jennis also thinks that would be a good first task 15:28
tristanIs there a reason we are adding merge requests to the buildstream-examples repo ?15:35
tristanWould we not do better to delete, or at least archive it as properly 'dead' for posterity ?15:35
tristanas 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 projects15:36
tristanseems like baggage not worth carrying15:36
finnIs there currently a simple example one can follow which doesn't take hours to build?15:41
tristanfinn, 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 docs15:43
finnWhat will the first example be?15:43
tristanfinn, to go "from scratch", you might look at one of the tests in tests/integration15:43
finnThanks15:43
tristanwhich will use the alpine image which is the minimal image we currently build on15: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
tristanjennis, 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
tristani.e. it allows one to specify in a .bst file something like: foo:bar.git15:49
tristaninstead of foo:/bar.git15: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-botbuildstream: 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/44516:07
tlaterHrm, 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
tlaterThis seems like an even bigger issue for people who don't have a smartphone.16:12
jennistlater, are you referring to the examples-MR?16:12
tlaterNo, I get a popup from gitlab forcing me to enable two-factor auth when I try to open the buildstream group.16:13
tlaterProbably because of my permissions...16:13
jmac!16:13
* tlater suspects toscalix changed the group settings16:13
*** bochecha_ has joined #buildstream16:13
toscalixtlater: 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
toscalixbut yes, I can skip it16:16
toscalixtlater: I only changed the subgroup settings16:16
toscalixdouble checking16:16
toscalixexactly, the two-facto auth is only required in the nosoftware subgroup, so no changes in the current repos16:18
albfan[m]tristan: it depends on bubblewrap so I suppose it is not possible16:18
toscalixtlater: disabled. I really like the 2FA16:21
gitlab-br-botbuildstream: 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/42116:21
*** cs_shadow has joined #buildstream16:25
tlatertoscalix: Yeah, no worries, it's just very usable if you don't sell your soul to Google ;)16:25
toscalixhaven't we all aready?16:26
toscalixs/aready/already16:26
tristanalbfan[m], I'm hoping for a WSL based sandbox implementation for building linux stuff on win32, but it's a bit ambitious16:30
tristanalbfan[m], building native win32 can be interesting, needs a lot of investigation though16:30
*** Prince781 has joined #buildstream16: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 now16:36
*** bochecha_ has quit IRC16:49
*** Prince781 has quit IRC16:57
*** toscalix has quit IRC17:02
*** bethw has quit IRC17:03
*** aday has quit IRC17:07
*** solid_black has joined #buildstream17:13
*** aday has joined #buildstream17:33
gitlab-br-botbuildstream: 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/44517:44
*** solid_black has quit IRC17:51
*** jonathanmaw has quit IRC17:54
*** aday has quit IRC18:09
*** tristan has quit IRC18:19
*** Prince781 has joined #buildstream18:24
*** xjuan has quit IRC18:27
*** Prince781 has quit IRC18:46
*** xjuan has joined #buildstream19:02
*** xjuan has joined #buildstream19:02
*** bochecha_ has joined #buildstream19:37
*** xjuan has quit IRC20:29
*** aday has joined #buildstream21:22
*** aday has quit IRC21:25
*** bochecha_ has quit IRC21:43

Generated by irclog2html.py 2.15.3 by Marius Gedminas - find it at mg.pov.lt!