| bigjools | anyone know if the go-bot setup is documented anywhere? | 00:58 |
|---|---|---|
| wallyworld_ | bigjools: there was an email sent to juju-dev | 01:36 |
| bigjools | wallyworld_: archive-diving I go then, ta | 01:37 |
| wallyworld_ | it turned into a thread of about maube 10 emails | 01:37 |
| wallyworld_ | it may well be documented somewhere but that's all i know of | 01:37 |
| wallyworld_ | the thread is called Landing bot setup steps | 01:38 |
| bigjools | wallyworld_: when was it sent? | 01:40 |
| wallyworld_ | bigjools: july 30 | 01:40 |
| bigjools | ta | 01:41 |
| bigjools | aaand found it | 01:45 |
| * thumper hmmms | 01:55 | |
| * thumper wishes once again that go had generics | 01:56 | |
| thumper | copying boilerplate is error prone and annoying | 01:56 |
| * thumper wanders off to think a little | 01:56 | |
| axw | thumper: are we doing a code review today? | 03:07 |
| thumper | yeah, why not | 03:14 |
| thumper | I figure we all get on a hangout and look through the code | 03:14 |
| thumper | or do you think we should look first, then discuss on the call? | 03:14 |
| axw | I have looked already, so either way | 03:16 |
| thumper | wallyworld_: your opinion^^^ | 03:20 |
| wallyworld_ | i haven't looked | 03:20 |
| wallyworld_ | i guess i have 10 mins | 03:21 |
| * thumper goes to see what file it is again | 03:22 | |
| axw | provider/state.go | 03:23 |
| * thumper is in the hangout | 03:31 | |
| bigjools | wallyworld_: how long should I wait for a canonistack instance to bootstrap? it's been 15 minutes | 04:04 |
| wallyworld_ | bigjools: cnonistack is unusable most times for me | 04:04 |
| wallyworld_ | too slow | 04:04 |
| bigjools | \o/ | 04:04 |
| wallyworld_ | the servers are heavily overcommitted i think | 04:05 |
| bigjools | wallyworld_: I have an instance | 04:05 |
| bigjools | which I can't ssh into.... woo | 04:06 |
| bigjools | sigh | 04:06 |
| wallyworld_ | yay | 04:06 |
| bigjools | today is Fail Central | 04:07 |
| axw | bigjools: I had better luck with lcy02 than 01 - try that if you're not already on it | 04:40 |
| * thumper has had enough | 05:16 | |
| bigjools | axw: got it. The wiki page I found didn't mention I needed sshuttle ... | 05:55 |
| axw | ahh, right | 05:58 |
| axw | I got caught a few times by that | 05:58 |
| bigjools | how do I convince juju to destroy a service with a failed install hook? | 06:21 |
| bigjools | resolved I guess? | 06:22 |
| axw | bigjools: yep | 06:23 |
| bigjools | ah it leaves an empy machine - will that get used next deployment? | 06:23 |
| bigjools | guess I will find out soon | 06:23 |
| axw | so I'm told, but I haven't witnessed it | 06:23 |
| axw | otherwise you can --to | 06:24 |
| bradm | bigjools: it does use it again, but I'm pretty sure it doesn't clean it up - so its got all the config of the previous service on it | 06:27 |
| bradm | bigjools: which makes it pretty useless in reality, since you can't be sure of the config | 06:27 |
| bigjools | bradm: that's probably a bug I guess. Thanks. | 06:27 |
| bradm | bigjools: you can fake it out by doing a juju deploy ubuntu and then destroy the service | 06:28 |
| bradm | bigjools: we've used that in the past to pre-deploy juju instances | 06:28 |
| bigjools | ok cheers | 06:28 |
| === tasdomas_afk is now known as tasdomas | ||
| davecheney | bigjools: use resolved to get the machine to a started state (even if this is a lie) then destroy it | 06:51 |
| davecheney | bigjools: juju never reuses machines | 06:51 |
| davecheney | never | 06:51 |
| davecheney | --to isn't part of that statement | 06:51 |
| bigjools | davecheney: ok thanks - I am recalling what you did in the sprint | 06:51 |
| davecheney | 'cos it just takes off all the safety guards | 06:51 |
| * davecheney goes back to bd | 06:52 | |
| davecheney | bed | 06:52 |
| bigjools | ... | 06:52 |
| === _mup__ is now known as _mup_ | ||
| bradm | it must have been in pyjuju days when it did that, since I can't replicate it now | 07:01 |
| bradm | so thats good | 07:01 |
| jam | bradm: bigjools: we are supposed to suppot "Clean & Empty" which means nothing was actually deployed to the machine yet we will use it for a future deploy/add-unit. | 07:19 |
| jam | you can use "juju add-machine" (I think it works with a provider) to pre alloc machines. | 07:19 |
| bradm | jam: we did use the juju deploy ubuntu ; juju destroy-service ubuntu with pyjuju at a sprint to get the machines deployed to make training easier | 07:23 |
| bradm | jam: its debatable how much it actually helped, but anyway :) | 07:24 |
| jam | bradm: I'm pretty sure that doesn't work by default in juju-core, but you should be able to "juju deploy --to X" to force it, and | 07:24 |
| jam | juju add-machine should let you allocate machines ahead of time | 07:24 |
| bradm | jam: yes, it doesn't seem to work like it did in pyjuju, I tested it on canonistack | 07:24 |
| bradm | jam: honestly, I think its a good thing, I could imagine it biting us as we use juju more | 07:25 |
| jam | bradm: right, the default policy is intended to be "if a machine was made dirty by installing something directly, don't default to putting something onto that machine" | 07:25 |
| bradm | jam: totally makes sense. | 07:26 |
| jam | the problem that bigjools highlights, is that if you destroy a unit, what is the point of leaving the machine around (by default) | 07:26 |
| bradm | jam: it's really a hack what we were doing | 07:26 |
| bradm | jam: to recover logs, any data that you need | 07:26 |
| jam | bradm: sure, but I would say "allow a flag to destroy-unit that keeps the machine alive" not "create a flag to also destroy the machine" | 07:27 |
| jam | so if you need something, you can just remove the unit, but otherwise don't leave unusable machines around | 07:27 |
| bradm | jam: sure, or even a config option that says "keep the units after a service destroy" and default to destroying it, any of those | 07:28 |
| bradm | I'm still getting up to speed at how we're using juju | 07:28 |
| dimitern | morning | 07:48 |
| dimitern | fwereade_: hey | 07:48 |
| fwereade_ | dimitern, heyhey | 07:48 |
| dimitern | fwereade_: I did the changes as discussed (hopefully) - https://codereview.appspot.com/13494043/ | 07:49 |
| fwereade_ | dimitern, sweet, I'll take a look before I go out... I have more houses to look at | 07:50 |
| dimitern | fwereade_: can you take a closer look please, and I'd appreciate a stepwise testing plan :) (never used the local provider, and not sure how to trigger these hooks - should I write I charm?) | 07:50 |
| dimitern | fwereade_: thanks | 07:50 |
| fwereade_ | dimitern, I'd suggest writing two metadata-only charms that just define relations | 07:51 |
| fwereade_ | dimitern, that can relate to one another | 07:51 |
| fwereade_ | dimitern, deploy one unit of each | 07:51 |
| fwereade_ | dimitern, wait until they're started | 07:51 |
| fwereade_ | dimitern, then open 2 windows, and debug-hooks each of the units | 07:52 |
| fwereade_ | dimitern, then add a relation between them | 07:52 |
| dimitern | fwereade_: ok | 07:52 |
| fwereade_ | dimitern, and then you can basically do whatever you need -- it'll just pause and let you run whatever you want for each hook | 07:53 |
| dimitern | fwereade_: hmm | 07:53 |
| dimitern | fwereade_: that's what's unclear - what should I do while paused? | 07:53 |
| dimitern | fwereade_: call relation-set/get someting? | 07:54 |
| fwereade_ | dimitern, specifically you need I think to just keep on setting fresh unit data on either side of the relation and checking it arrives properly at the otherside | 07:54 |
| fwereade_ | dimitern, yeah exactly | 07:54 |
| fwereade_ | dimitern, set on one side | 07:54 |
| fwereade_ | dimitern, get on the other | 07:54 |
| dimitern | fwereade_: ok, sounds simple enough | 07:55 |
| fwereade_ | dimitern, if you want to be extra clever add a config to the charm | 07:55 |
| dimitern | fwereade_: how? | 07:55 |
| fwereade_ | dimitern, config.yaml | 07:55 |
| dimitern | fwereade_: aah :) ok | 07:55 |
| fwereade_ | dimitern, just a string setting you can change to trigger a hook and call relation-set in to get out of steady state again | 07:55 |
| dimitern | fwereade_: just a sec | 07:56 |
| fwereade_ | dimitern, cath is pointing out that we're going to be late... I will review as soon as I'm back | 07:56 |
| fwereade_ | dimitern, sorry :( | 07:56 |
| dimitern | fwereade_: ok | 07:56 |
| dimitern | fwereade_: will try it out | 07:56 |
| dimitern | fwereade_: tyvm | 07:57 |
| dimitern | TheMue: hey | 08:16 |
| dimitern | TheMue: can you point me out to the local provider docs you wrote? | 08:16 |
| TheMue | dimitern: sure, one moment | 08:17 |
| TheMue | dimitern: https://juju.ubuntu.com/docs/config-local.html | 08:17 |
| dimitern | TheMue: cheers | 08:20 |
| TheMue | dimitern: yw | 08:24 |
| === liam_ is now known as Guest751 | ||
| bigjools | I've got a charm that does an adduser in the installation hook, and it errors out with "Authentication token manipulation error" | 09:15 |
| TheMue | ah, great, now shows exactly the behavior i wanted | 09:50 |
| TheMue | dimitern: btw, if you discover errors or missing parts in the doc please let me know | 09:51 |
| dimitern | TheMue: well, it worked, but only as root | 09:51 |
| dimitern | TheMue: I had to copy my env.yaml to /root/.juju/ and also my ssh keys to /root/.ssh | 09:51 |
| dimitern | TheMue: and add juju binaries to root's PATH | 09:52 |
| dimitern | TheMue: sudo juju bootstrap failed with "juju not found" | 09:52 |
| dimitern | TheMue: so I had to do sudo su - and run as root from that point on | 09:53 |
| TheMue | dimitern: strange | 09:53 |
| TheMue | dimitern: i've done it as regular user and only bootstrapped with sudo | 09:54 |
| TheMue | dimitern: it's described in the code that way | 09:54 |
| dimitern | TheMue: perhaps some steps are missing | 09:54 |
| TheMue | dimitern: which release? i took 13.04 on a clean system | 09:55 |
| dimitern | TheMue: sudo x doesn't do a login shell session, so PATH is not set | 09:55 |
| dimitern | TheMue: raring | 09:55 |
| dimitern | TheMue: and my system is mostly clean (reinstalled a month ago, so most of the packages are missing) | 09:57 |
| dimitern | TheMue: didn't install mongodb-server, just lxc - I have mongo installed for juju tests already | 09:57 |
| TheMue | dimitern: i'm using a testing image in a parallel vm for those tests. here i installed juju, mongo and lxc as user (but with sudo) | 09:58 |
| TheMue | dimitern: and then i generated the config which has been in $HOME/.juju (as expected) | 09:58 |
| TheMue | dimitern: after that bootstrapping with sudo (so it uses my env) | 09:59 |
| TheMue | dimitern: et voila | 09:59 |
| dimitern | TheMue: well it didn't work for me | 09:59 |
| TheMue | dimitern: deployed our lovely pair wp/mysql then and it worked | 09:59 |
| TheMue | dimitern: service deployment has been as non-root w/o sudo | 10:00 |
| TheMue | dimitern: because the provisioner already works as root | 10:01 |
| dimitern | TheMue: I suppose if sudo juju bootstrap worked, the rest would've worked as well | 10:02 |
| dimitern | TheMue: but juju could not be found, so I had to do as described above | 10:03 |
| dimitern | TheMue: if I had juju installed from the archive/ppa it should work though, it'll be available for any user | 10:04 |
| dimitern | TheMue: so my point is - if you're running juju from source, the described steps should be different | 10:04 |
| TheMue | dimitern: sounds reasonable, on that system i had no juju source | 10:08 |
| jam | TheMue: thanks for the reviews. One more: https://codereview.appspot.com/13360044/ | 10:53 |
| TheMue | jam: looking | 10:54 |
| jam | TheMue: did it help to split this out into concrete steps? I sort of evolved the patch all at once, but I thought it might be easier to review in layers. | 10:55 |
| TheMue | jam: yes, helps a lot. would have been too large then | 10:56 |
| TheMue | jam: but i can't review the first one, get an error | 10:56 |
| jam | TheMue: one of them I submitted with a missing prereq so I deleted and resubmitted it. | 10:58 |
| TheMue | jam: ah, ok | 10:58 |
| jam | TheMue: do you mean https://codereview.appspot.com/13391047/ ? (the httpsuite-ssl change, vs the httpclient-ssl change, vs the final client/Client change) | 10:59 |
| jam | I thought you had reviewed 2 patches ,but it was just that I got your email 2 times (one from Reitveld, once from LP) | 11:00 |
| TheMue | jam: yes, that one. "old chunk mismatch" | 11:00 |
| jam | TheMue: for https://codereview.appspot.com/13391047/ the unified diff looks correct, I don't know what is wrong with the side-by-side diff | 11:01 |
| jam | I'll try to repropose | 11:02 |
| natefinch | morning all | 11:02 |
| TheMue | jam: thx | 11:02 |
| TheMue | natefinch: morning | 11:02 |
| jam | TheMue: reproposing seems to have fixed the bug | 11:03 |
| jam | morning natefinch | 11:03 |
| TheMue | jam: ah, yes, looks better now, will look at it in a few moments | 11:03 |
| TheMue | jam: LGTM, wow, all together a nice change | 11:12 |
| jam | TheMue: fwereade_, wallyworld_, dimitern, natefinch, mgz, jam: standup ? https://plus.google.com/hangouts/_/f497381ca4d154890227b3b35a85a985b894b471 | 11:30 |
| jam | fwereade_, mgz && | 11:33 |
| jam | ^^ | 11:33 |
| === wallyworld_ is now known as wallyworld | ||
| === wallyworld is now known as Guest33423 | ||
| dimitern | fwereade_: can you take a look at this please https://codereview.appspot.com/13523043/ | 11:42 |
| dimitern | fwereade_: I'll try the extra live testing steps you mentioned about the previous change | 11:43 |
| fwereade_ | dimitern, great, thanks | 11:43 |
| natefinch | jam: if you guys are still reviewing... it bugs me that loadState closes the reader it gets. It seems much clearer to have the callers close their own readers, even if it does mean one duplicated line. I think it's better to have the close method next to the method that creates the Closer | 13:17 |
| jam | natefinch: I did have that one in my list, but we're wrapping up and it wasn't huge | 13:17 |
| natefinch | jam: yeah, not really a big deal | 13:18 |
| jam | natefinch: it does eliminate some redundancy, but it makes it a bit harder to audit for the http.Get bug | 13:18 |
| jam | (you must call Close explicitly or http leaks file handles) | 13:18 |
| * TheMue has to step out, wife awaited me since 3 | 13:18 | |
| natefinch | jam: really, I think it's just a separation of concerns... loadstate doesn't care if the thing is closed, it just wants to be able to read. And if you then have something that wants to use loadState that doesn't have a close method, you have to manuifacture one | 13:19 |
| jam | natefinch: "is it better that LoadStateFromURL hand off the defer r.Close to the loadState | 13:20 |
| jam | helper? It eliminates redundancy, but makes it slightly harder. (I wouldn't | 13:20 |
| jam | directly guess that "Load*" would close the object it is given. though taking a | 13:20 |
| jam | ReadCloser does tell you that.) | 13:20 |
| jam | " | 13:20 |
| jam | was my notes | 13:20 |
| natefinch | yep | 13:20 |
| natefinch | jam: what's my business unit? None of these things seem to fit quite right (doing the travel form for October sprint) | 13:22 |
| jam | natefinch: you're under CDO | 13:24 |
| jam | that is the group underneath Robbie (Mark Ramm's direct manager) | 13:24 |
| natefinch | jam: ok, cool | 13:24 |
| dimitern | fwereade_: so when you can https://codereview.appspot.com/13523043/ | 13:25 |
| fwereade_ | I'm even doing it now :) | 13:25 |
| dimitern | fwereade_: sweet! | 13:26 |
| fwereade_ | dimitern, reviewed, LGTM with one extra test | 13:30 |
| dimitern | fwereade_: thanks | 13:31 |
| dimitern | fwereade_: am I getting this correctly - once the mysqlUnit has entered scope, leaving scope should generate a change in the watcher, right? | 13:41 |
| fwereade_ | dimitern, scope for unit X is unaffected by any change to unit X | 13:41 |
| dimitern | fwereade_: yeah, but unit X in this case is wordpressUnit | 13:42 |
| fwereade_ | dimitern, ah ok yes | 13:42 |
| fwereade_ | dimitern, mysqlUnit leaving scope should not generate a change, but should do a depart | 13:42 |
| dimitern | fwereade_: isn't that also a change? | 13:42 |
| dimitern | fwereade_: in Departed | 13:43 |
| dimitern | fwereade_: I'm not getting anything though.. strange | 13:43 |
| fwereade_ | dimitern, ah yes, got you, I had mismatched levels | 13:43 |
| fwereade_ | dimitern, missing SyncBackend somewhere? | 13:43 |
| dimitern | fwereade_: hmm perhaps, will dig into it | 13:43 |
| fwereade_ | dimitern, oh, and, I meant to say | 13:43 |
| fwereade_ | dimitern, you're written the WatcherC | 13:44 |
| fwereade_ | dimitern, would be really nice to use it in the state tests too | 13:44 |
| fwereade_ | dimitern, if you swap that in there you'll be able to tell whether the test harness works ;) | 13:44 |
| dimitern | fwereade_: ah, right! | 13:44 |
| dimitern | fwereade_: I'll start there then | 13:44 |
| dimitern | fwereade_: does this look ok to you? http://paste.ubuntu.com/6062708/ | 13:44 |
| sidnei | hi folks, can i get a pair of reviews? https://codereview.appspot.com/13242049 | 13:45 |
| fwereade_ | dimitern, hmm, might be worth a repeated sync on ShortWait timeout | 13:45 |
| dimitern | fwereade_: just Sync() ? | 13:46 |
| fwereade_ | dimitern, but check how the state tests do it, I might be wrong | 13:46 |
| fwereade_ | dimitern, ah, hmm | 13:46 |
| dimitern | fwereade_: do I need the StartSync() then? | 13:46 |
| fwereade_ | dimitern, just a sec, I should remind myself | 13:47 |
| dimitern | fwereade_: hmm there's no state.Sync() anymore it seems | 13:47 |
| dimitern | sidnei: now you need only a single LGTM btw | 13:49 |
| sidnei | even better :) | 13:49 |
| dimitern | sidnei: fwereade_ might be the right reviewer for this | 13:50 |
| fwereade_ | sidnei, I am now churning through reviews | 13:50 |
| sidnei | thanks! | 13:50 |
| dimitern | fwereade_: nevermind, found the issue :) | 14:04 |
| dimitern | fwereade_: one spurious for loop that never ends | 14:05 |
| teknico | wedgwood: you might find something useful in https://code.launchpad.net/~teknico/charm-helpers/lint-fixes/+merge/181859 | 14:07 |
| teknico | if not, no worries :-) | 14:07 |
| wedgwood | thanks. I normally do reviews on Wednesday afternoons. | 14:08 |
| wedgwood | teknico: taking a cursory look, I see one place where imports are commented out (rather than removed) (line 36-39) and one place where logic is changed (line 268-276) | 14:12 |
| teknico | wedgwood: the first one is because those imports are example placeholders, IIUC | 14:14 |
| teknico | wedgwood: the second is different logic but same behavior: the flow does not ever exit the for loop prematurely | 14:15 |
| wedgwood | teknico: ah, right you are. tbh I thought for ... else behaved differently. | 14:17 |
| teknico | wedgwood: that was a possibility I wanted to dig up by changing the code ;-) | 14:18 |
| wedgwood | it's definitely doesn't do what it seems like it should. Poor choice of keywords. | 14:18 |
| teknico | wedgwood: yes, it needs to be looked at in the right wavelength light to even begin making sense :-) | 14:21 |
| wedgwood | In any case, thanks for the cleanups :) | 14:23 |
| teknico | wedgwood: my pleasure, I hope I wasn't too much obsess... I mean, comprehensive about them :-) | 14:25 |
| wedgwood | teknico: improvements are improvements, I say | 14:26 |
| teknico | I actually removed a few more hundred lines of diffs that changed double-quote string delimiters into single-quote ones, it was becoming a bit too much. :-) maybe some other time | 14:27 |
| sidnei | fwereade_: addressed comments, first time using export_test, so hopefully it's good. :) | 14:43 |
| fwereade_ | sidnei, cheers | 14:43 |
| fwereade_ | sidnei, LGTM, just pull that duplicated code block out into a func and you're good | 14:46 |
| sidnei | fwereade_: a method of DeployerSuite or a standalone func? | 14:50 |
| dimitern | fwereade_: ping | 15:00 |
| fwereade_ | sidnei, standalone, I think, unless it's using fields on the type | 15:00 |
| fwereade_ | dimitern, pong | 15:01 |
| dimitern | fwereade_: so the txnRevno for settings I get from relUnit.Settings always seems to be -1 than the one I get from the watcher | 15:02 |
| * fwereade_ raises a somewhat surprised eyebrow | 15:02 | |
| dimitern | fwereade_: I'll push the branch as soon as the tests finish | 15:02 |
| dimitern | fwereade_: so originally I had c.Assert(actual.Changed, DeepEquals, expected.Changed) in AssertChange() | 15:03 |
| dimitern | fwereade_: but it seems all but the versions match (not sure if it's always off by one but it seems likely) | 15:03 |
| dimitern | fwereade_: I noticed that in relationunit_test after changing them to use the RelationUnitsWatcherC | 15:04 |
| dimitern | fwereade_: the versions were not checked before, just the len(changed), which seems weird | 15:04 |
| fwereade_ | dimitern, ahhhh-ha, that rings a bell, just a mo | 15:05 |
| dimitern | fwereade_: so I resorted to checking actual.Changed, HasLen, len(expected.Changed) and then making sure each key in actual is also in changed | 15:05 |
| dimitern | fwereade_: it seemed weird that on remoteUnit.EnterScope(nil) with a fresh relation the initial event reported txnRevno2 for unit settings (always) | 15:06 |
| fwereade_ | dimitern, so, just one thought... what are you basing expected versions on? | 15:07 |
| fwereade_ | dimitern, I think you'll find there is a reason | 15:07 |
| dimitern | fwereade_: no idea really - I thought txnRevno should be 1 on a fresh scope | 15:07 |
| fwereade_ | dimitern, try setting the unit's public address before entering scope ;p | 15:08 |
| dimitern | fwereade_: hmm, ok | 15:08 |
| fwereade_ | dimitern, (entering scope depends on the unit's life, so the txn revno will be 1 + max(scope doc, unit doc)) | 15:09 |
| dimitern | fwereade_: you mean instead of EnterScope(nil) to do myRelUnit.EnterScope(map[string]interface{}{"private-address": "blah"}) | 15:10 |
| dimitern | fwereade_: in both cases the initial event comes with version 2 | 15:10 |
| fwereade_ | dimitern, no, I mean make an extra change to the unit doc before joining | 15:10 |
| dimitern | fwereade_: aah | 15:10 |
| fwereade_ | dimitern, so basically settings versions are volatile | 15:11 |
| fwereade_ | dimitern, now I think of it you should be testing that they're increasing per unit, but should not be testing specific values | 15:11 |
| dimitern | fwereade_: I did mysqlUnit.SetPrivateAddress("1.2.3.4") and then myRelUnit, err := rel.Unit(s.mysqlUnit); myRelUnit.EnterScope(nil) - still | 15:11 |
| dimitern | fwereade_: still ver 2 | 15:11 |
| fwereade_ | dimitern, you sure private wasn't already set? | 15:12 |
| dimitern | fwereade_: positive | 15:12 |
| dimitern | fwereade_: I could try some other unit doc field.. | 15:12 |
| fwereade_ | dimitern, or just set it twice, even, I think | 15:12 |
| dimitern | fwereade_: did SetPrivateAddress() twice - the same ver 2 | 15:13 |
| dimitern | fwereade_: (with different values for private address) | 15:14 |
| dimitern | fwereade_: I could just ignore the version and make sure the changed field's keys are sane | 15:14 |
| fwereade_ | dimitern, hum, I'd expected that it would be the unit doc | 15:15 |
| fwereade_ | dimitern, ok, in any given test you *should* ignore the version | 15:16 |
| fwereade_ | dimitern, but we should *also* have a test that the version increases with each change of a unit | 15:16 |
| fwereade_ | dimitern, the value will not be stable as the codebase changes | 15:17 |
| fwereade_ | dimitern, but we depend on its always getting bigger | 15:17 |
| dimitern | fwereade_: when should it increase? | 15:17 |
| fwereade_ | dimitern, sorry: the settings revno should increase by at least 1 whenever the settings change | 15:21 |
| dimitern | fwereade_: where should this test reside? | 15:24 |
| fwereade_ | dimitern, this may be crazy, but... how about integrating it in the WatcherC? | 15:24 |
| fwereade_ | dimitern, the client says what should change | 15:24 |
| dimitern | fwereade_: but doesn't specify a version | 15:25 |
| fwereade_ | dimitern, the WatcherC validates the stream of data-per-unit coming out of the watcher by checking strictly increasing versions? | 15:25 |
| dimitern | fwereade_: the watcherC keeps the old txnRevno and compares it's greater | 15:25 |
| fwereade_ | dimitern, yeah, exactly | 15:25 |
| fwereade_ | dimitern, does that sound crazy? | 15:25 |
| dimitern | fwereade_: sgtm | 15:25 |
| fwereade_ | dimitern, cool | 15:25 |
| dimitern | fwereade_: so I need to keep track of each unit that's changed and make the check on each AssertChange | 15:29 |
| fwereade_ | dimitern, yeah, just keep a map of unit: last-change and whenever you get a new one check and set | 15:30 |
| sidnei | fwereade_: it's not, but neither the 'bundle' function is. i shall convert both then. | 15:34 |
| fwereade_ | sidnei, nice, thanks | 15:34 |
| dimitern | fwereade_: worked nicely | 15:39 |
| fwereade_ | dimitern, sweet | 15:39 |
| sidnei | fwereade_: so, on a similar subject. we need to get this fix into currently-deployed prodstack instances at some point. what's the process for that, once we have a built release (which at this point is 1.15.x?) to upgrade the existing deployments that are on 1.13.3-ish? | 15:42 |
| natefinch | mgz: can you take a look at this real quick? It's been mostly LGTM'd by john, but I added one more tweak to get the stuff working live with azure: https://codereview.appspot.com/13082044/ | 15:42 |
| mgz | natefinch: sure | 15:43 |
| mgz | hm, can't persuade rietveld to give me the 1->2 diff... | 15:44 |
| mgz | ah, pick a file, then change in the dropdown works | 15:44 |
| natefinch | mgz: yeah, I don't know what's going on there. The only addition was one line, adding DeploymentName to the struct we pass to GetDeploymentRequest | 15:44 |
| mgz | yeah, but is still blank... I'll branch the thing and look at the actual history | 15:45 |
| mgz | probably just the merges from trunk upseting it | 15:48 |
| natefinch | mgz: yeah | 15:49 |
| dimitern | fwereade_: would like you to have a final look though https://codereview.appspot.com/13523043 | 15:50 |
| TheMue | fwereade_: thx for review | 16:21 |
| === tasdomas is now known as tasdomas_afk | ||
| === wedgwood is now known as Guest57531 | ||
| === Guest33423 is now known as wallyworld | ||
| === wallyworld is now known as Guest52886 | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!