[07:04] Good morning. [07:09] TheMue, heyhey [07:09] fwereade: Hiya === TheMue_ is now known as TheMue [07:32] good morning gentle ment [07:32] gentlemen [07:32] davecheney, heyhey [07:33] TheMue, ok, I am looking in detail at the "firewaller bug" [07:33] TheMue, have you played around with that at all yourself? [07:33] davecheney: Good mroning [07:34] fwereade: No, only discussed it so far with Aram. [07:34] TheMue, can you recall anything specific about how he was characterising it? [07:34] https://codereview.appspot.com/6856120/ [07:34] ^ version III of cross series bootstapping [07:34] fwereade: It only happened after has change of the firewaller. So maybe it's better to wait until Monday to talk with him about those changes. [07:35] TheMue, so the answer to the question I asked is "no"? :) [07:35] fwereade: He talked about a kind of race situation, but only in global mode. [07:36] TheMue, ah, hmm, ok, it looks like I'm stuck on something different then [07:36] TheMue, I think I fixed that one [07:36] fwereade: Hey, not so fast. I need my time to write here in the bottom line while you're asking new questions above. ;) [07:36] TheMue, sorry :) [07:37] fwereade: NP, had a smiley at the end. :D [07:38] fwereade: His change after the watcher changed has been to extract a larger part of the firewallers loop code into an own method. But there it works differently, because the lifecycle states of the units returned by the watcher has to be checked. [07:39] TheMue, ok, I don't *think* that is currently an issue [07:39] TheMue, I'm pretty certain I have tweaked the firewaller such that it's responding to global mode events nicely [07:39] TheMue, now I'm confused that a unit watch appears not to be firing when it should [07:39] fwereade: Is it already submitted or a CL? [07:40] TheMue, thank you for the context though [07:40] TheMue, it's Arams 120 CL [07:40] TheMue, that niemeer asked me to finish off and land [07:40] fwereade: You're welcome, I'm interested in it too as the Firewaller has initially been my baby. ;) [07:41] fwereade: Do you please have the URL for me, to quickly jump in? [07:41] TheMue, I haven't even pushed it yet, just a mo [07:41] fwereade: OK [07:42] TheMue, hm, sorry, it'll be inconvenient to push right now [07:43] fwereade: OK, I think I get a notification when you'll push it, so then I can jump in again. [07:45] TheMue, I'm just pushed to lp:~fwereade/juju-core/aram-firewaller-sketch if you want to take a look (and, ideally, paste me the output of TestGlobalModeRestartPorts on your machine?) [07:45] TheMue, it has a load of random debug prints pooed in there [07:46] fwereade: Yes, one moment. [08:03] TheMue, heyyyyyyyyy I know what this looks like [08:03] TheMue, maybe [08:04] TheMue, if a client asks for a watch on a channel and isn't receiving, it just blocks the whole watcher [08:16] fwereade: So, after the mgo/mongodb update test is running and TestGlobalModeRestartPorts passes w/o any messages. Strange. [08:16] TheMue, gaah TestGlobalModeRestartPortCounts [08:16] TheMue, sorry [08:17] fwereade: Passes too, also sorry. ;) [08:19] TheMue, ok, that's weird :) [08:20] fwereade: Indeed. [08:51] davecheney, fwereade, TheMue: morning! [08:51] rogpeppe: Hi [08:51] rogpeppe, heyhey [08:52] morning all [08:54] davecheney: glad to hear the TLS update worked for you... [08:57] rogpeppe: it's fn' awesome [08:57] so much faster than ssh [08:58] davecheney: i'm a bit surprised by that actually [08:58] davecheney: the network latency should be approximately similar [08:58] much less round trips [08:58] davecheney: to set up the connection? [08:59] davecheney: i didn't realise ssh was inefficient that way [09:05] 3 round trips for tcp handshake, then a dozen for ssh handshake, then 2 for tcp channel, then the mgo setup on top of that [09:06] i'd bet pesos to dollars that tls handshaking is more effective than ssh tunneling [09:08] davecheney: TLS handshake is standard diffie hellman, i think, which is one round trip. there may be more too though, to negotiate the crypto. [09:09] depends on the size of the cert [09:09] but it is much more efficient [09:09] so, on high latency the difference is remarkable [09:10] also, shit: https://code.google.com/p/go/source/detail?r=697f36fec52ceaabc2208d28918fc34787b617bb [09:10] spent three evenings working on this one [09:23] davecheney: ah bugger. what a pity. have you worked out what the problem was yet? [09:25] issue 599 [09:25] 64bit atomics need to be 8 byte aligned [09:26] i should have remembered earlier [09:34] rogpeppe, TheMue: I had believed that the watcher would panic if you tried to unwatch something you hadn't watched... is that not the case? [09:35] fwereade: you're talking about state/watcher? [09:35] rogpeppe, yeah [09:36] fwereade: doesn't look like it from a brief glance at the source [09:36] fwereade: looks like it's idempotent [09:36] rogpeppe, indeed, I just could have sworn it was like that once [09:36] rogpeppe, probably just fever dreams [09:36] fwereade: it would be an easy change to make [09:37] rogpeppe, indeed, I may mention it to niemeyer [09:38] fwereade: One moment, daughter is here, will be back in a few seconds. [09:38] rogpeppe, TheMue: could I get a trivial +1 on https://codereview.appspot.com/6856122 please? [09:39] rogpeppe, that panicing would have made it *much* quicker to find that issue, I think [09:39] fwereade: *click* [09:41] fwereade: it's a pity that a document id is an interface{} [09:42] fwereade: in fact, where do we use a document id that's not a string? [09:43] fwereade: Has the Unwatch() changed to take an id instead of a Life. [09:44] fwereade: ha, the answer is "nowhere" [09:44] fwereade: probably that's only since you've changed machine id to string [09:45] fwereade: that would've caught the problem too [09:45] fwereade: no, it was just a mistake [09:45] s/fwereade/TheMue/ [09:45] rogpeppe: OK [09:47] rogpeppe, also, relations have an "Id" field that we use internally but a mgo "_id" called Key [09:47] rogpeppe, so, yes from the watcher POV we could still use strings [09:48] rogpeppe, but it's muddy [09:48] fwereade: all the tests run fine with strings [09:48] TheMue, the Unwatch never needed a life [09:48] fwereade: but i see what you mean [09:48] rogpeppe, cool [09:48] fwereade: could you make a test for this? [09:48] rogpeppe, I broached the subject with niemeyer, he seemed -1 on changing it [09:48] fwereade: fairy nuff [09:49] fwereade: Yep, I took a look and saw that it only hasn't been catched by the compiler due to the interface{}. [09:49] rogpeppe, that's an interesting philosophical question [09:49] rogpeppe, if I were to write a test for this I would also feel obliged to write a similar one for every watcher [09:49] fwereade: ha [09:50] fwereade: maybe you should [09:50] fwereade: maybe they're broken similarly... [09:56] rogpeppe: https://codereview.appspot.com/6856120/ [09:58] davecheney: looking [10:00] davecheney: is configTest.series ever going to be set to something other than version.Current.Series? [10:00] davecheney: (or "") [10:00] rogpeppe: yes [10:01] davecheney: how so? [10:01] bootstrapping from quantal -> precise [10:02] davecheney: so is the plan to add an argument to config.New? [10:03] davecheney: thinking about it, i'm not convinced that default-series should default to anything [10:04] davecheney: but i'm open to contrary arguments [10:04] rogpeppe: id' say it should always default to precise [10:04] (insert current LTS release) [10:04] this is all in aide of https://codereview.appspot.com/6851081/ [10:04] davecheney: ignoring version.Current.Series? [10:04] after a very long talk with gustavo [10:04] i still have no idea how to test this [10:04] other than the sledgehammer approaches I have previously suggested [10:06] davecheney: i don't think the configTest.series parameter is necessary. you can just test against whatever the default should be, in configTest.check. [10:06] s/parameter/field/ [10:07] davecheney: i'll have a think about how you might want to test cross-series bootstrapping [10:07] thanks rogpeppe, i'm really stuck [10:21] brb [10:22] hey, can I depend on deterministic ordering in slices? (when testing building a slice by appending) [10:23] and what's the best way to test this, given []Type as a result and having another - what type of assert is best? [10:25] dimitern: yes, slice are ordered [10:25] dimitern: yes [10:26] and DeepEquals [10:26] dimitern: assuming the types are not opaque [10:26] rogpeppe, davecheney: ok, 0x [10:27] dimitern: have you read this article? http://research.swtch.com/godata [10:29] rogpeppe: what do you mean opaque? no I'll read it now [10:29] dimitern: i mean you shouldn't use DeepEqual if the values have unexported fields [10:30] rogpeppe: right, that's the case [10:30] rogpeppe: but does it matter, since the're all in the same package? [10:30] dimitern: it depends what kind of equality you're interested in [10:31] dimitern: what are you actually trying to assert? [10:32] rogpeppe: well, I have AllFlavors() call, which returns []Flavor and error, and I called AddFlavor() twice with fl1, fl2, and now it seems []Flavor{fl1,fl2} != the result, but maybe I'm returning them in reverse order [10:33] dimitern: if your flavors live in a map [10:33] possibly [10:33] dimitern: i didn't realise a Flavor had unexported fields [10:33] davecheney: yes, I have map[string]Flavor [10:33] so if you're doing something lie [10:34] for k, v := range flavors { if k == "something" { append(...) } } [10:34] then the order your iterate over flavors is random [10:34] rogpeppe: it's a local type, Flavor has entity *nova.Entity and detail *nova.FlavorDetail - one or both can be specified [10:35] davecheney: ok, is there something similar to pythons's dict.values(), without iterating over the map? [10:35] dimitern: it sounds like DeepEqual might be appropriate in this particular case. [10:35] rogpeppe: isn't there a SliceEquals or something ? [10:36] for set equality ? [10:36] rogpeppe: tried DeepEquals, but does not work if the order is different [10:36] davecheney: not sure, I'll try [10:36] davecheney: i don't think so [10:37] shitter [10:37] dimitern: one conventional way of doing this is to sort the slices before comparing them [10:37] rogpeppe: how? [10:37] davecheney: no, SliceEquals does not exists [10:37] dimitern: look at the sort package, define Less, Swap and Len methods on a new type sortedFlavors []Flavor [10:38] dimitern: (one line each) [10:38] rogpeppe: ok, I'll check it out [10:38] dimitern: then sort each slice, by type-converting it to sortedFlavors, then use whatever equality operation you feel is appropriate [10:40] rogpeppe: that seems like a lot of work to compare 2 slices - how about just iterating over them and checking each [10:41] dimitern: it's probably more work to do that, tbh [10:42] dimitern: defining the sort methods is mechanical and easy [10:42] dimitern: because you don't just want to declare two slices [10:42] dimitern: you want to make sure they're exactly the same [10:43] rogpeppe: I'm looking at the sort pkg now [10:43] dimitern: regardless of order [10:44] dimitern: tbh, if you're adding your own flavors and checking them, it's probably overkill to check the whole Flavor [10:44] rogpeppe: the docs for Soft says: The sort is not guaranteed to be stable. - what's that? [10:44] dimitern: it means that equal items won't necessarily end up in the original order [10:44] dimitern: you could just check some aspect of it that you know about [10:44] dimitern: e.g. a name [10:45] rogpeppe: exactly, I have only 2 items and they can have at most 2 orderings [10:45] dimitern: in which case, you could just compare and swap, then DeepEqual [10:45] dimitern: same difference [10:45] rogpeppe: yep, that seems the easiest [10:46] dimitern: if even, just compare the first element of each slice, then swap one if they're different, then compare both slices [10:46] s/if even/or even/ [10:46] rogpeppe: yeah, this works and since it's a simple case I'll leave it to this [11:00] mgz: or w7z: ? [11:01] hey dimitern [11:01] mgz: hey :) i was not sure which one you're watching - mumble? [11:02] nearly there, left my headset somewhere silly... [11:05] wallyworld: ^^ [11:05] ok [11:17] rogpeppe, TheMue: I've updated https://codereview.appspot.com/6856122 -- it should still be pretty trivial [11:17] fwereade: Already lookin' ;) [11:17] rogpeppe, TheMue: the watcher package change means that the MUW tests fail reasonably comprehensibly when the dont-unwatch-life-values fix [11:18] ...is not there [11:19] fwereade: Aaaaah, that's what missing, just wanted to ask. [11:21] fwereade: LGTM [11:22] rogpeppe, awesome, thanks [11:24] rogpeppe, I'm printing the channels because it really helped me to be able to just print suspicious-looking channels when I watched, and that was very useful in the early stages when I was still zeroing in on a fix [11:24] rogpeppe, it may not be so useful now, i guess [11:24] rogpeppe, thanks [11:24] fwereade: i think that if you're debugging, you'll add prints where necessary. an isolated channel pointer print isn't much help really. [11:26] rogpeppe, the isolated panic is kinda handy though, because you can just put your prints in state/watcher and not have to worry about understanding the watcher package [11:26] fwereade: true, the channel is an external thing, isn't it. fair enough, why not leave it? [11:27] rogpeppe, cheers [11:28] TheMue, any comments? [11:28] fwereade: You'll get your LGTM in a few moments. ;) [11:29] TheMue, <3 [11:31] fwereade: You've got it. [11:35] TheMue, the channel pointer print is the difference between (1) debugging by sprinking a few prints in state/watcher.go (or whatever the client is) and (2) opening up and coming to understand the watcher package well enough to determine that you need to print the channels around the pointw where I panic [11:35] TheMue, and (2) also includes all of (1) [11:35] TheMue, as rogpeppe pointed out, the channel is a watch param just like coll and id [11:36] TheMue, revno is not interesting here, so I didn't print that [11:36] TheMue, but the specific channel *is* interesting, because the watcher can have N watches for a given key [11:36] TheMue, and the challenge becomes figuring out which watch is unmatched [11:37] TheMue, do you still object to panicing with the chan? [11:38] niemeyer, heyhey [11:39] Good morning! [11:39] fwereade: OK, reasonable. [11:39] TheMue, cheers [11:39] niemeyer: Hiya. [11:40] niemeyer: morning! [11:40] fwereade, TheMue, rogpeppe: Heyas! [11:54] mgz, dimitern, wallyworld: Something landed recently in trunk broke the build: https://pastebin.canonical.com/79484/ [11:54] wallyworld: I think it was your change to return 'nil' for objects when there has been an error. [11:55] yes :-( [11:55] If we want to keep 'nil', we need to change the return to "*Object" rather than "Object" [11:55] since pointers can be nil [11:55] but a struct cannot. [11:55] hmmm. i guess that's the correct thing to do [11:55] wallyworld: but also note, I'm not sure that adding the 'if' statement *improves* the code. [11:55] wallyworld: I tried sending an email this morning, and tried again just now. [11:55] i was doing what the code review called for [11:56] let me check, i missed the email [11:56] wallyworld: Oh, I understand that, I'm disagreeing with rogpeppe not you. I approved your change, but questioned the motive. [11:56] maybe i need to return an empty struct? [11:56] even with that, we should be careful to run 'go test ./...' before committing on trunk, since we don't have a bot yet. [11:56] yeah, sorry [11:56] i thought i had but clearly not [11:57] wallyworld: IMO, returning an empty struct is just as bad as a partial one. Since they have an object that is 'valid'. [11:57] In which case, just return what you have, and set err like you were doing. [11:57] Or, we change everything to pointers. [11:57] anyway, i'm not really working today [11:57] :L) [11:57] your preference? [11:57] :) [11:57] jam: was that the named-return-variable discussion? [11:57] wallyworld: *mine* is to not repeat the if err != nil, check in the function, that the callers are going to have to do anyway. [11:58] rogpeppe: sorry, I do want to discuss this, but my son needs to play Lego Batman... :) [11:58] Can we chat maybe later, or on Monday/Tues? [11:58] jam: nananananananana batman! [11:58] jam: sure [11:58] jam: i'll fix the build and we can discuss next week [12:15] wallyworld: If in doubt, I'd suggest returning a pointer [12:16] wallyworld: That's appropriate in most cases [12:16] wallyworld: Of course, and returning nil consistently when err != nil [12:16] niemeyer: for now, i'm just reverting the changes to fix the breakages, and will tackle properly next week once we have consensus [12:16] since it's late here now and i'm tired [12:16] wallyworld: Sure, I'm just saying this is a well known concept [12:17] wallyworld: If you go over existing code, both ours and upstream, you'll see a clear pattern [12:17] jam makes a good point though about the functions doing their own if err != nil checks [12:17] wallyworld: it's definitely conventional to return a zero value when err != nil, pointer or not. that was the reason for my comments. [12:17] agree pointer is best [12:18] Yes, and returning partial data when err != nil is not okay [12:18] i wonder, if err != nil, why does the returned value need to be zero? [12:18] wouldn't it be ignored anyway? [12:19] There are very rare exceptions to this rule, which confirms the rule.. Those are related to cases where it's important to observe the partial values, and the partial values are deterministic in such cases. [12:19] it seems like it's just adding complexity [12:19] lots of apis in other languages say, if there's an error, such and uch is undefined [12:19] wallyworld: It's one more layer to prevent bad usage [12:20] wallyworld: yes, that's not the case here [12:20] wallyworld: We consistently return a zero value when errors are found [12:20] ok. it costs to do that though [12:20] wallyworld: I haven't spent much so far :) [12:21] well, each api call needs repetitive blocks of code "if err != nil ..." [12:21] makes it messy, especially if the caller also does "if err != nul..." [12:21] wallyworld: We do err != nil all the time [12:21] wallyworld: It's pretty clear and cheap [12:21] so we have 2 places doing the error checking - the caller and the callee [12:22] wallyworld: you only need that if you're returning something that may not be zero [12:22] wallyworld: We have tons of places doing error checking [12:22] wallyworld: in the case we're talking about, we were returning a value that may or may not have been partially filled in [12:22] what if the semantics of the call say - "if there's an error, don't use the returned vsalue" [12:22] wallyworld: That's not even on the list of things that actually burned my own time over the past few years [12:23] wallyworld: That still wouldn't change the fact that there's a strong convention that is useful as it prevents crack from flowing through [12:24] ok, no problem. it does seem like code duplication for little gain, but that's just IMO :-) [12:24] s/code duplication/excessive boiler plate [12:24] wallyworld: I'd appreciate seeing an example [12:24] wallyworld: Of what you claim is code duplication [12:24] ok, i'll do a pastebin [12:25] wallyworld: Doing if err != nil { return nil, err }, or doing if err != nil { return foo, err } [12:25] wallyworld: Doesn't really cost much, and the former one is both less error prone, and more clear in intention [12:26] niemeyer: in this case, i think it was between: if err != nil { return nil, err }; return foo, nil; and a plain return. [12:27] https://pastebin.canonical.com/79488/ [12:27] see how the caller and calle both do the if err != nil [12:27] the function has no need to [12:27] if just clutters the code [12:27] why not just return data, err [12:28] who cares if data is partially populated, err will tell that something is wrong and not to use it [12:28] wallyworld: pastebin.ubuntu.com is a better one for that [12:28] wallyworld: As it's public [12:28] ok, old habits [12:28] wallyworld: Let me grab my second factor auth key :) [12:28] sorry [12:28] wallyworld: No worries [12:29] the private one is in my browser history [12:29] so, my view is that normally, the function semantics of such things normally are such that if an exception is raised, or an error returned, disregard any result data [12:30] wallyworld: It's hard to follow that example, as it contains completely invalid code [12:30] sorry, it's pseudo code cut and pasted from real code [12:30] wallyworld: Yeah, but it's completely broken, badly intended, and referring to variables that don't even exist [12:31] sure, but that doesn'tmatter for the sake of the argument [12:31] wallyworld: You won't convince me about how I'm wrong about things being error prone and trivial with such an example :-) [12:31] that take awaya is [12:31] if err != nil { [12:31] return nil, err [12:31] } [12:31] return resp.Images, nil [12:31] the above is from the function [12:31] resp doesn't exist on that context [12:31] why does it need to do the if err != nil [12:32] why not just return resp.IMages, err [12:32] and let the client do if err != nil [12:32] wallyworld: Sorry, I honestly can't suggest anything.. the example is completely broken. I'm arguing that this is trivial to write, and that it makes code less error prone. Hard to even make the point if the example is completely crackful. [12:33] here's the full function, i was hoping you could just see the intent from the pseudo code, http://pastebin.ubuntu.com/1398973/ [12:34] imagine a module with dozens of such functions, all doing if err != nil unnecessarily, sure adds a lot of bloat and boilerplate [12:34] when the caller just goes ahead and does the err != nil dance anyway [12:34] wallyworld: that's a pretty unusual case actually [12:35] wallyworld: because it's not returning something that something else returned. [12:35] wallyworld: it's returning something that something else might or might not have filled in [12:36] yes true [12:36] and the err tells us if the fill in worked [12:36] wallyworld: agreed. we *could* just allow arbitrary broken return values when err != nil [12:37] wallyworld: That's fine, as long as there's a promise from the underlying function to never touch the resp value if any errors are returned [12:37] that's a pretty standard semantic [12:37] wallyworld: but i think it's nicer if we do things more predictably [12:37] wallyworld: I'd be slightly surprised if that promise is in place, though [12:37] i agree it's nicer perhaps, but at the cost of a fair bit of bloat [12:37] wallyworld: Because it means you cannot return any errors after resp has been touched [12:38] wallyworld: That seems the heart of your concern [12:38] wallyworld: tbh, i think the "bloat" makes the code easier to read [12:38] wallyworld: And honestly bloat to me is something else.. [12:38] i guess i see cut and paste boilerplate as bloat [12:38] wallyworld: it means that i *know* instantly the set of possible values returned by ListImagesDetail [12:39] wallyworld: I see logic that is hard to read, poorly engineered, hard to maintain, as bloat [12:39] wallyworld: Error prone as well.. totally bloatful [12:39] sure, there's more than one type of bloat [12:40] i'll point jam to the scrollback when he is next in the office, since he also shared my concerns [12:40] wallyworld: That trivial line, makes things lighter to process.. I know I don't have to worry about the return value having bad data, and I know any callers couldn't *possibly* use a partial value coming out of it. It empties brain space rather than filling it up. [12:41] s/line/lines :-P [12:42] thanks for the discussion though [12:42] interesting points of view [12:42] wallyworld: Exactly my point.. 10 lines of code may be easier to read than 1. [12:43] maybe, depends on the reader i guess [12:43] wallyworld: Depends on the lines of code [12:44] or more so one's unerlying expections about the semantcs of the call [12:44] as a caller, i would never dream of using returned data if err != nil [12:44] wallyworld: I'm glad to hear that. [12:44] so it never would enter my head to expect the function to have to do anything about it [12:45] wallyworld: That still doesn't mean that you won't do that anyway, by mistake. [12:45] wallyworld: Either way, if the point isn't easy to agree with, I ask that we at least sustain that strong convention that we use both internally, and externally in the core Go development team. [12:46] sure, no problem [12:46] i was trying to understand the underlying rationale [12:46] var foo []Foo [12:46] foo, err = Func() [12:46] if err == ErrSomething { [12:47] // Ah, okay, that error is expected here. [12:47] } [12:47] Oops.. foo has bad data now. [12:47] yes, and so don't use it [12:47] wallyworld: Yeah, you're very smart and will never do that. I'm kind of dumb, and tend to make mistakes. [12:48] hey, i didn't mean it like that [12:48] tests are also part of the solution too [12:50] wallyworld: They surely are. Not rare we assert the fact we get a zero value, for example. [12:51] niemeyer, aram's firewaller change is up again at https://codereview.appspot.com/6843128 [12:51] anyways, i'll make the necessary changes to use pointers [12:51] fwereade: I'll get on it right now [12:51] niemeyer, the firewaller bug was a MachineUnitsWatcher bug, fixed this morning [12:51] so the pattern will work as intended [12:51] fwereade: Oh, what was it about? [12:51] niemeyer, we were unwatching the wrong key [12:51] wallyworld: Thanks a lot [12:51] wallyworld: and thanks for explaining your POV [12:51] np, thanks for the discussion [12:51] niemeyer, state/watcher now panics if you try any funny business like that [12:52] fwereade: Holy crap, the wrong key huh [12:53] niemeyer, `for _, unit := range map[string]Life{...} {` [12:53] fwereade: Woah [12:53] niemeyer, quite so [12:53] fwereade: I think I did that before too.. will pay more attention to ranges [12:53] niemeyer, took a while to track it down, it doesn't exactly leap out at you when the type is defined elsewhere [12:54] lunchtime, biab [12:56] fwereade: It didn't help that Unwatch takes an interface to support keys of multiple types [12:56] TheMue: Enjoy [12:56] niemeyer, yeah, indeed [12:56] niemeyer, rogpeppe pointed out that all our _ids are actually strings now [12:57] niemeyer, although I'm still really bothered by id/_id on Relation [12:57] fwereade: Well, we could have saved ourselves the trouble if the watcher was at least verifying basic typing [12:57]