| jw4 | davecheney: okay | 00:07 |
|---|---|---|
| jw4 | davecheney: I'll leave it as is for now, but we may want to revisit names package I guess | 00:07 |
| davecheney | i want to shoot that in the head | 00:07 |
| davecheney | having api types in a seperate repo is just a mistake | 00:08 |
| davecheney | tags are api types | 00:08 |
| davecheney | they belong in the api | 00:08 |
| davecheney | my rule of thumb is, if you have to import two pacakges to use the functionality from one package, then they should be combined | 00:08 |
| jw4 | davecheney: can't we just pull them back in to api now that we've cleaned up the api packages? | 00:08 |
| jw4 | davecheney: that names package is tiny and maintining it is a pita | 00:09 |
| davecheney | jw4: raise an issue | 00:11 |
| davecheney | id' +1 that PR if you did it | 00:11 |
| jw4 | kk | 00:11 |
| davecheney | in the past it wasn't clear that the names, or specificlly tags | 00:11 |
| davecheney | were an api type | 00:11 |
| davecheney | despite what fwereade keeps telling us | 00:12 |
| jw4 | :D | 00:12 |
| davecheney | but, now we're had the clue bat applied | 00:12 |
| davecheney | it is clear that tags are api params | 00:12 |
| davecheney | so if you were to attempt that, it would be great | 00:12 |
| davecheney | id' do it like this | 00:12 |
| davecheney | 1. delete juju/names from your GOPATH | 00:12 |
| davecheney | copy all the Tags types to apiserver/params | 00:12 |
| davecheney | keep hitting things with sed til it works | 00:13 |
| jw4 | davecheney: +1 | 00:13 |
| jw4 | davecheney: of course if we do that it seems even more important to not include the Tag types in the state package... | 00:21 |
| jw4 | issue raised: lp-1392537 | 00:22 |
| davecheney | urgh | 00:25 |
| davecheney | fuck | 00:25 |
| davecheney | forgot that | 00:25 |
| * davecheney throws a shoe | 00:26 | |
| jw4 | hehe | 00:26 |
| perrito666 | <austin powers> who throws a shoe? | 00:26 |
| perrito666 | http://www.youtube.com/watch?v=an0bVaTjF_Y | 00:27 |
| thumper | um... what's this about state not using names? | 00:28 |
| jw4 | oh, oh. now we're gonna catch it | 00:28 |
| thumper | tags aren't entirely api types | 00:29 |
| thumper | when an entity has a tag, and the entities come from state | 00:29 |
| thumper | then you need to pull them up out into a common library | 00:30 |
| thumper | which is what juju/names is | 00:30 |
| thumper | what's the problem? | 00:30 |
| thumper | ideally... | 00:30 |
| jw4 | so then the dependency graph of state has to include juju/names as an acceptable dependency (obviously) | 00:30 |
| thumper | tags would live closer to the business types | 00:30 |
| thumper | no, I think it is a necessary dependency | 00:31 |
| jw4 | thumper: as in 'shadow' or 'mirror' business types in the API layer? | 00:31 |
| thumper | jw4: later... | 00:31 |
| jw4 | thumper: kk | 00:32 |
| thumper | davecheney: please don't bother removing juju/names dep from state at this stage | 00:33 |
| thumper | davecheney: there are other fish to gut | 00:34 |
| jw4 | perrito666: just watched that clip - funny :) | 00:36 |
| davecheney | https://groups.google.com/forum/#!topic/golang-dev/sckirqOWepg | 00:47 |
| davecheney | thumper: understood | 00:48 |
| thumper | P.S. For those keeping score, this will be Go version control system number four. Go development started in Subversion, moved to Perforce, then Mercurial, and soon to Git. | 01:15 |
| thumper | at least we only used two | 01:15 |
| rick_h_ | thumper: :) | 01:20 |
| rick_h_ | thumper: aren't you glad you practiced your git and got ahead of the game now? | 01:20 |
| thumper | shuddup | 01:21 |
| * thumper is moving private project from bzr to git | 01:21 | |
| thumper | for reasons | 01:21 |
| davecheney | Moving DVCS hosting as a Service! | 01:25 |
| mwhudson | davecheney: launchpad used to do that! | 01:52 |
| mwhudson | well still does i guess | 01:53 |
| thumper | :-) | 02:01 |
| thumper | davecheney: so... I have a question | 02:01 |
| thumper | and I think the answer is "don't do that" | 02:01 |
| thumper | but... | 02:02 |
| thumper | if I have an error | 02:02 |
| thumper | but it is a typed nil | 02:02 |
| thumper | how do I check that the pointer is nil? | 02:02 |
| thumper | because I can't check nil | 02:02 |
| thumper | I'm trying to make the library robust to stupid people | 02:03 |
| * thumper is guessing reflect package | 02:03 | |
| thumper | reflect.ValueOf(err).IsNil() works | 02:06 |
| perrito666 | thumper: can you not test for stupid users? | 02:06 |
| davecheney | thumper: hmm | 02:07 |
| davecheney | gimme a sec | 02:07 |
| davecheney | reflect is one way | 02:07 |
| davecheney | can you show me the code path ? | 02:07 |
| thumper | I'm writing the code path :-) | 02:07 |
| thumper | lemmie pastebin it | 02:08 |
| davecheney | k | 02:08 |
| thumper | http://paste.ubuntu.com/8996904/ | 02:08 |
| thumper | so... I'm trying to make this robust for a nil error | 02:08 |
| thumper | even a typed nil | 02:08 |
| thumper | without the nil check, a typed nil supports the interface | 02:09 |
| thumper | so we need the nil check as well | 02:09 |
| thumper | perrito666: not easily | 02:09 |
| perrito666 | few things make me happier than trimming loads of lines of code... perhaps some medicine for my cold would | 02:14 |
| davecheney | thumper: | 02:15 |
| * thumper waits | 02:16 | |
| davecheney | err, ok := err.(ErrorStacker); if ok && err != ErrorStacker{} | 02:16 |
| davecheney | ^ guess | 02:16 |
| thumper | davecheney: but ErrorStacker is an interface | 02:16 |
| davecheney | then youre secrewed | 02:16 |
| thumper | reflect FTW | 02:16 |
| davecheney | sorry | 02:16 |
| davecheney | https://github.com/juju/juju/pull/1134 | 02:35 |
| davecheney | small, hoefully uncontraversial change | 02:35 |
| thumper | first of three error and logging methods: | 03:17 |
| thumper | https://github.com/juju/errors/pull/13 | 03:17 |
| thumper | https://github.com/juju/loggo/pull/6 | 03:29 |
| thumper | https://github.com/juju/testing/pull/38 | 03:30 |
| thumper | once these are merged, we can add dependencies to juju and take advantage of them | 03:31 |
| thumper | another friday, another kid off to brownie camp | 03:31 |
| * thumper away for now | 03:31 | |
| thumper | cheers folks | 03:31 |
| === kadams54 is now known as kadams54-away | ||
| davecheney | does anyone know how to do a stacked review on rbt ? | 03:40 |
| davecheney | can it do that ? | 03:40 |
| davecheney | ping, http://reviews.vapour.ws/r/436/ | 03:44 |
| axw | davecheney: you can do pass "--parent" to rbt post | 03:55 |
| davecheney | axw how does that work with the bot that auto generates PRs ? | 03:55 |
| axw | not 100%, but I *think* you can do it after it's already been proposed | 03:55 |
| davecheney | i'll give it a go | 03:56 |
| axw | davecheney: also, LGTM on your rename branch | 03:56 |
| davecheney | i'll just merge then | 03:56 |
| davecheney | ta | 03:56 |
| davecheney | axw the next branch is far more intersting that a search + replace | 03:57 |
| davecheney | axw: fyi https://github.com/juju/juju/pull/1137 | 04:00 |
| axw | a little more than I have time to review right now, but the description sounds nice | 04:05 |
| davecheney | kk | 04:05 |
| davecheney | that's about it from me for today | 04:05 |
| davecheney | one or two more cleanup branches then things are looking a lot better | 04:05 |
| ericsnow | davecheney: were you going to sort out the dependency issues causes by state/backups.go? | 04:08 |
| ericsnow | davecheney: I figured the most straight-forward thing would be to basically move it into state/backups. | 04:09 |
| davecheney | ericsnow: yes, the issue is assigned to me | 04:11 |
| jw4 | name package refactor for beginning migration of Actions to using UUID identifiers : https://github.com/juju/names/pull/31 | 04:11 |
| ericsnow | davecheney: okay | 04:11 |
| wallyworld | davecheney: multiwatcher refactor lgtm | 04:11 |
| davecheney | wallyworld: ta | 04:12 |
| davecheney | just waiting for the other branch to land | 04:12 |
| wallyworld | refactoring is good :-) | 04:13 |
| wallyworld | fixing dependencies is good :-) | 04:13 |
| davecheney | wallyworld: it's made thigns a lot cleaner | 04:13 |
| jw4 | I'm unclear on api breaking changes with regards to Actions since they're not in use anywhere yet | 04:13 |
| davecheney | api/ no longer depends on state | 04:13 |
| wallyworld | \o/ | 04:13 |
| wallyworld | and not should it | 04:13 |
| wallyworld | nor | 04:13 |
| davecheney | indeed | 04:14 |
| wallyworld | jw4: my view is go ahead with your changes, so long as they are truly not used anywhere | 04:14 |
| jw4 | wallyworld: yep not yet, but that window closure is rapidly approaching | 04:14 |
| wallyworld | time to get it right then | 04:15 |
| wallyworld | before you ship a working solution | 04:15 |
| jw4 | yeah, and this PR is groundwork for some realy good improvements :) | 04:15 |
| wallyworld | \o/ | 04:16 |
| davecheney | jw4: names review done | 04:22 |
| jw4 | tx davecheney | 04:22 |
| davecheney | wallyworld: axw https://github.com/juju/juju/pull/1137 | 04:34 |
| davecheney | good to review now | 04:34 |
| wallyworld | looking | 04:34 |
| davecheney | just merged the prereq | 04:34 |
| wallyworld | davecheney: i just review that one did't i? | 04:35 |
| davecheney | yeah | 04:35 |
| davecheney | lgtm still stands ? | 04:35 |
| jw4 | updated https://github.com/juju/names/pull/31 (davecheney ?) | 04:43 |
| * davecheney looks | 04:44 | |
| davecheney | jw4: what about the panic in Join... ? | 04:45 |
| jw4 | davecheney: what should it do? | 04:45 |
| jw4 | davecheney: if someone calls it with invalid input? | 04:46 |
| davecheney | i think we have to apply the same logic as we do with tags | 04:46 |
| davecheney | NewXXTag will panic | 04:46 |
| davecheney | so you're not uspposed to call it outisde of a test without validating the input first | 04:46 |
| davecheney | ie, validating that it wont panic | 04:46 |
| jw4 | davecheney: I see, so assume good input? | 04:47 |
| davecheney | not sure what you mean ? | 04:48 |
| jw4 | well... since this is a public "constructor" it can be called with invalid input | 04:48 |
| jw4 | not sure what to do in that case | 04:48 |
| davecheney | the rule of thumb is | 04:49 |
| davecheney | don't panic in non test code | 04:49 |
| davecheney | that goes double when dealing with user input | 04:49 |
| jw4 | davecheney: so should JoinActionTag have an error return type? If I understood you correctly you're saying that non test code should never call the other NewXXTag variants that also panic and don't return error. | 04:52 |
| jw4 | davecheney: I might have misunderstood you before too - maybe the only use of this Join.. method is in Test code... | 04:53 |
| davecheney | if the method is only used in tests, then there is nothing to do | 04:53 |
| davecheney | sorry, i think i asked that | 04:53 |
| davecheney | but i wasn't clear enough | 04:53 |
| davecheney | panicing in tests is fine | 04:54 |
| jw4 | davecheney: no you were clear, I just wasn't thinking right | 04:54 |
| davecheney | ericsnow: you're good to go on that PR | 04:54 |
| ericsnow | davecheney: thanks | 04:54 |
| ericsnow | davecheney: I'm hoping to land http://reviews.vapour.ws/r/402/ (the one where I get rid of the sub-packages) before any of my other patches | 04:55 |
| jw4 | davecheney: so JoinActionTag is used entirely in test code except for closely coupled calls that I will fix in state | 04:55 |
| davecheney | i'll have a look | 04:56 |
| davecheney | jw4: ok | 04:56 |
| davecheney | ericsnow: oh, that was that two page review | 04:56 |
| davecheney | i'll have a look now | 04:56 |
| ericsnow | davecheney: k | 04:56 |
| === kadams54 is now known as kadams54-away | ||
| ericsnow | davecheney: thanks for the followup review | 05:43 |
| ericsnow | davecheney: I'll look closely tomorrow | 05:43 |
| davecheney | np | 05:50 |
| jw4 | Smallish initial refactor of Actions to use UUID instead of Sequence for suffix of _id : http://reviews.vapour.ws/r/441/ | 05:54 |
| jw4 | preparation for consolidating Actions and ActionResults into one entity, and improving of the watcher and notification design of actions | 05:55 |
| davecheney | jw4: reviewed | 06:02 |
| jw4 | speedy gonzalez! | 06:02 |
| jw4 | :) | 06:02 |
| jw4 | davecheney: couple questions... | 06:07 |
| davecheney | shoot | 06:07 |
| jw4 | 1) names.ActionTag is in an intermediate stage right now, where it has prefix and suffix, but the suffix is just a string, not guaranteed to be an UUID | 06:08 |
| jw4 | after step two or three I hope to rid actions of the prefix/suffix id altogether | 06:08 |
| davecheney | jw4: ok, add a TODO | 06:08 |
| jw4 | and the ActionTag will just be kind-UUID | 06:08 |
| jw4 | kk | 06:08 |
| davecheney | and don't use dud data in tests | 06:08 |
| jw4 | :-S | 06:09 |
| jw4 | wait... what's the emoticon for embarrassed? | 06:09 |
| davecheney | i'm only know the ones for smashing things | 06:09 |
| jw4 | 2) the Action.ActionTag() method returns an ActionTag but I don't know how to guarantee that the internal state will be correct | 06:10 |
| jw4 | notwithstanding our earlier discussion, is it appropriate to panic in the case where an Action entity has invalid _id that will cause an invalid ActionTag? | 06:11 |
| davecheney | yes, but you should also provide a validation function | 06:11 |
| davecheney | so callers can know that they call that method, they'll get valid data | 06:12 |
| davecheney | or return an error | 06:12 |
| jw4 | Ah! | 06:12 |
| jw4 | derp ... so Action.ActionTag() should return names.Tag, bool | 06:12 |
| davecheney | yup | 06:12 |
| jw4 | kk | 06:12 |
| davecheney | the key is we never create an invalid tag | 06:12 |
| davecheney | if we do that at the points where thigns are converted to tags | 06:12 |
| davecheney | then we have less error checking | 06:13 |
| jw4 | makes sense | 06:13 |
| davecheney | and we _know_ that when someone talks about a tag, it is valid data | 06:13 |
| davecheney | that is the difference between a tag and string | 06:13 |
| jw4 | pedantically speaking; since the return is names.ActionTag, bool not *names.ActionTag, bool then we will be returning an invalid tag when the bool is false | 06:14 |
| jw4 | (parenthesis would have helped that sentence) | 06:14 |
| davecheney | yes, that is why names.NewXXXTag returns a tag or panics | 06:14 |
| davecheney | so there is no chance, even if you ignore the error, that you can use bad data | 06:14 |
| davecheney | so my preference is for somehing like | 06:15 |
| davecheney | action.ValidateTag() bool | 06:15 |
| davecheney | which does the check | 06:15 |
| davecheney | and action.Tag() whcih returns a tag or panics | 06:15 |
| davecheney | then, you can ditch validateTag once it is not needed | 06:15 |
| jw4 | hmm; interesting. I see. I'll go that route in this case | 06:15 |
| davecheney | its pretty gross | 06:16 |
| davecheney | but that is why tags are so strict | 06:16 |
| davecheney | once you have a tag | 06:16 |
| davecheney | you know whereever you use that data it is valid | 06:16 |
| davecheney | pretty gross, having to add that validate method | 06:16 |
| jw4 | hmm; methinks I should keep that in mind for the next refactor of the names package too | 06:17 |
| davecheney | basically the rule is | 06:17 |
| davecheney | if you have a tag value, you _KNOW_ it is valid | 06:17 |
| davecheney | so any method you call on that will also return valid dta | 06:18 |
| davecheney | the cost of this is we need to be pedantic when parsing data to return a tag | 06:18 |
| davecheney | so, one option could be to relax the checks that action.ActionTag does | 06:18 |
| davecheney | thereby making more things valid action tags | 06:18 |
| jw4 | not a terrible choice, especially in transition | 06:19 |
| davecheney | a valid tag is pretty much defined by it's consumers | 06:21 |
| davecheney | as long as there is nothing like | 06:21 |
| davecheney | if tag.String() == "" { return error } | 06:21 |
| davecheney | then it's fine | 06:21 |
| jw4 | kk | 06:22 |
| jw4 | updated http://reviews.vapour.ws/r/441/ | 06:33 |
| jw4 | thanks davecheney ; I'll need to update the names package again to make sure invalid UUID's cause the JoinActionTag to panic | 06:41 |
| jw4 | also, interestingly utils.NewUUID() failed and returned an error in my tests a few minutes ago... | 06:41 |
| jw4 | I'm going to head to bed, but I'll pick it up again in the morning. | 06:42 |
| rogpeppe | davecheney: ping | 08:22 |
| davecheney | rogpeppe: ack | 08:22 |
| rogpeppe | davecheney: can you see what i'm doing wrong here? http://paste.ubuntu.com/9001604/ | 08:23 |
| rogpeppe | davecheney: in relation to this issue: https://code.google.com/p/go/issues/detail?id=9098 | 08:23 |
| rogpeppe | davecheney: i *think* i'm guaranteed to be running from go tip there, no? | 08:23 |
| * davecheney looks | 08:23 | |
| davecheney | rogpeppe: yes, thats go tip | 08:24 |
| davecheney | is the polyNNNN repo updated ? | 08:24 |
| davecheney | that repo has some asm which probalby needs to be updated | 08:24 |
| rogpeppe | davecheney: just making sure | 08:24 |
| mattyw | morning everyone | 08:25 |
| mattyw | rogpeppe, davecheney morning | 08:25 |
| rogpeppe | davecheney: ah, i guess that was the problem! | 08:26 |
| rogpeppe | davecheney: i'm sure i go get -u'd it but evidently not | 08:26 |
| davecheney | rogpeppe: i think the bug is in the poly repo | 08:28 |
| davecheney | or in a duff working copy | 08:28 |
| rogpeppe | davecheney: yeah | 08:28 |
| voidspace | morning all | 09:08 |
| === Spads_ is now known as Spads | ||
| voidspace | dimitern: morning | 10:03 |
| dimitern | voidspace, morning | 10:03 |
| voidspace | dimitern: http://reviews.vapour.ws/r/432/ | 10:08 |
| wallyworld | axw: re: the test | 10:24 |
| wallyworld | it won't work on windows will it? | 10:25 |
| axw | wallyworld: ah, crap. | 10:25 |
| axw | nope | 10:25 |
| wallyworld | i'd prefer not to rely on the pysical machine | 10:25 |
| wallyworld | mock out the func | 10:25 |
| wallyworld | sorry, i hould have been more insistent | 10:25 |
| axw | wallyworld: the best we could do then is to check that the worker is started | 10:25 |
| wallyworld | started and calls some func | 10:26 |
| wallyworld | i'll kill the landing job | 10:26 |
| axw | thanks | 10:26 |
| axw | wallyworld: "and calls some func" ? | 10:26 |
| wallyworld | mock out the func that the worker calls to check for block devices, can't remember the details off hand | 10:27 |
| axw | wallyworld: I'd prefer doing that inside the worker tests | 10:27 |
| wallyworld | return a made up set of block devices | 10:27 |
| wallyworld | ok | 10:27 |
| wallyworld | maybe checking the worker had started is enough | 10:27 |
| axw | I'll move the current test inside worker/diskmanager as a linux-only test | 10:27 |
| wallyworld | but checking the worker has started doesn't check that it's wired up right | 10:28 |
| axw | faking bits out doesn't really either | 10:28 |
| axw | e.g. pretending that it'd work on Windows, when in reality it won't | 10:29 |
| axw | wallyworld: alternatively, we could just skip the test on non-Linux...? | 10:30 |
| axw | eventually it should work on Windows too | 10:30 |
| wallyworld | sure, but if we mock out listBlockDevices(), we can check that the worker calls that func | 10:30 |
| wallyworld | i guess yeah skip that test on windows | 10:31 |
| wallyworld | but we should have a test for the no op worker on windows? | 10:31 |
| wallyworld | ie test that the right worker is started on the given platform | 10:32 |
| axw | ok | 10:32 |
| perrito666 | morning | 10:38 |
| axw | wallyworld: I've pushed again, with more tests and refactored to work better on Windows | 11:32 |
| wallyworld | ta, looking | 11:33 |
| * fwereade extended lunch to do parent teacher thing at school | 11:40 | |
| wallyworld | axw: one comment - still about that machine agent test that relies on actually querying the machine hardware | 11:46 |
| voidspace | dimitern: I take it we *don't* have to account for the fact that a machine (maas node) may have multiple network interfaces with overlapping CIDRs (on different networks) | 11:52 |
| voidspace | dimitern: but we can assume that if a required address falls within a CIDR for a network then we should request the address allocation for that interface | 11:53 |
| dimitern | voidspace, hold on, how can the CIDRs be overlapping? | 11:53 |
| voidspace | dimitern: i.e. the algorithm is "list networks for instance", "find the first one where the requested IP falls within the CIDR" | 11:53 |
| voidspace | dimitern: you can have the same IP address on different networks, right? | 11:54 |
| dimitern | voidspace, nope, I'd be surprised if you could | 11:54 |
| voidspace | dimitern: hmmm... ok, fair enough | 11:54 |
| voidspace | dimitern: I won't worry about it | 11:54 |
| dimitern | voidspace, how will the routing work otherwise? | 11:54 |
| voidspace | dimitern: so you can have two different networks both using the same address space - but not routable between each other | 11:55 |
| dimitern | voidspace, that's not possible in maas | 11:55 |
| voidspace | dimitern: cool | 11:55 |
| dimitern | voidspace, and in ec2 it's not an issue, as everything falls inside the vpc super-range | 11:56 |
| voidspace | dimitern: yep, great | 11:56 |
| voidspace | but for manual provider :-p | 11:56 |
| voidspace | which we're not worrying about now | 11:56 |
| dimitern | voidspace, for the manual provider and others we'll have to think, but not now, yeah :) | 11:58 |
| axw | wallyworld: the whole thing is not a unit test, it's a functional test. but yes, I can mock lsblk. | 12:04 |
| axw | will come back to it on monday | 12:04 |
| axw | thanks for the review | 12:04 |
| wallyworld | yeah, those jujud tests suck | 12:05 |
| wallyworld | nice not to propogate the badness just a little | 12:05 |
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| === kadams54 is now known as kadams54-away | ||
| === kadams54-away is now known as kadams54 | ||
| voidspace | dimitern: if you're happy with http://reviews.vapour.ws/r/432/ could you add a shipit? | 14:22 |
| voidspace | dimitern: oh | 14:25 |
| voidspace | dimitern: you did :-) | 14:25 |
| voidspace | dimitern: thanks | 14:25 |
| voidspace | hadn't refreshed page | 14:25 |
| perrito666 | natefinch: ? | 14:37 |
| dimitern | voidspace, :) | 14:48 |
| fwereade | perrito666, ping | 14:52 |
| fwereade | perrito666, actually just when you're around: http://reviews.vapour.ws/r/298/ is in the queue, but I think I'll need a guiding hand to be useful with it: do you need another reviewer, or can I leave it to people who alreayd have relevant state in mind? | 14:53 |
| perrito666 | hey hey | 14:55 |
| perrito666 | fwereade: the people involved already have a state of mind and have been involved somehow in the changes made since initial proposal I think we can manage for now :) | 14:56 |
| fwereade | perrito666, thanks | 14:58 |
| fwereade | ericsnow, similar questions re http://reviews.vapour.ws/r/346/ -- ISTM that dave/tim/jesse have already been looking into it in some detail, do you want my 2c as well or shall I leave it in their hands? | 14:59 |
| ericsnow | fwereade: I think we're good, but thanks (feel free to look things over though) | 15:04 |
| fwereade | ericsnow, it all looks fine as code, but the trouble is I'm a bit behind on necessary context for sane judgment | 15:04 |
| ericsnow | fwereade: no worries | 15:05 |
| perrito666 | sinzui: I have to admit that my jaw dropped when I saw your mail not have an issue about restore :p | 15:22 |
| sinzui | perrito666, I hope you didn't jinx the tests | 15:23 |
| * sinzui looks | 15:23 | |
| jw4 | fwereade: thanks for the review - I think you're right about slashes versus hyphens... I know there was a reason, but I don't think it was insurmountable | 15:31 |
| jw4 | fwereade: I don't think you've lost much context except maybe that switching from Sequence ID to UUID is a good initial step to consolidating Actions and ActionResults | 15:32 |
| jw4 | fwereade: in fact I think both slashes and hyphens are supported | 15:32 |
| fwereade | jw4, so, the thing about uuids is that they're meant to be uu -- and so the leading service/unit bit would now seem to be redundant? | 15:33 |
| === tvansteenburgh1 is now known as tvansteenburgh | ||
| jw4 | fwereade: exactly | 15:33 |
| fwereade | jw4, hmm, I would be happier if we didn't have alternative spellings of the "same" tag | 15:34 |
| jw4 | fwereade: I was telling davecheney last night that this is an intermediate step | 15:34 |
| jw4 | fwereade: +1 | 15:34 |
| jw4 | fwereade: I'll fix that too | 15:34 |
| fwereade | jw4, well, if it's intermediate towards "action-<uuid>" then the alternate spelling quibble is redundant too :) | 15:34 |
| jw4 | fwereade: within a couple hops the ActionTags should be as clean as some of the other tags without the Prefix/Suffix mumbo jumbo | 15:34 |
| fwereade | jw4, <3 | 15:35 |
| jw4 | fwereade: yep end-state action-<uuid> | 15:35 |
| fwereade | jw4, fwiw, I think that NotNil in the review is redundant | 15:35 |
| fwereade | jw4, a nil error will fail the subsequent ErrorMatches | 15:36 |
| jw4 | fwereade: derp | 15:36 |
| jw4 | fwereade: *thats* why my test was failing, and why davecheney told me to drop it | 15:36 |
| jw4 | :) | 15:36 |
| fwereade | jw4, and btw I'm not quite following the NewUUID thing? | 15:36 |
| jw4 | fwereade: confused rambling | 15:36 |
| fwereade | jw4, if it's broken we should fix it | 15:36 |
| jw4 | fwereade: it all hinges on the fact that my assert should have been gc.Nil | 15:37 |
| jw4 | i.e. I was *trying* to assert no error | 15:37 |
| jw4 | not that there *was* an error | 15:37 |
| jw4 | :) | 15:37 |
| jw4 | fwereade: in short there is no problem with NewUUID except in my formerly confused mind | 15:38 |
| fwereade | jw4, ok, I think that's an LGTM with trivials then | 15:40 |
| jw4 | kk | 15:40 |
| wwitzel3 | ericsnow, natefinch, perrito666 | 16:04 |
| perrito666 | mm, I seem to have some connection prob, be right there | 16:05 |
| rogpeppe | the proposal for the hook that runs if no other hook does is "default-hook", right? | 16:37 |
| rogpeppe | fwereade: ^ | 16:37 |
| rogpeppe | dimitern: ^ | 16:38 |
| dimitern | rogpeppe, I don't really know, sorry | 16:39 |
| rogpeppe | dimitern: ok, thanks | 16:39 |
| rogpeppe | dimitern: i'll go with default-hook for the time being... | 16:40 |
| dimitern | rogpeppe, +1 | 16:40 |
| sinzui | natefinch, mgz do you have a minute to review https://github.com/juju/juju/pull/1145 | 17:49 |
| mgz | sinzui: lgtm | 17:51 |
| wwitzel3 | is there a way to set the debug level before you bootstrap? | 17:52 |
| wwitzel3 | natefinch: found the issue with bug #1392390 | 18:29 |
| mup | Bug #1392390: maas zone selected for deployment that is occupied <cloud-installer> <landscape> <maas-provider> <placement> <regression> <juju-core:Triaged by wwitzel3> <juju-core 1.21:Triaged by wwitzel3> <https://launchpad.net/bugs/1392390> | 18:29 |
| wwitzel3 | natefinch: there are zero tests for that code is part of the problem, the for loop that looks at zones, if there was no error for a zone, it would keep process until the last zone instead of breaking out of the loop. | 18:30 |
| wwitzel3 | natefinch: resulting in the twilight zone (which is full) being the last attempt, and hence returning the error. | 18:31 |
| voidspace | g'night folks | 18:31 |
| wwitzel3 | nn voidspace have a good weekend | 18:31 |
| voidspace | wwitzel3: you too, thanks | 18:31 |
| natefinch | wwitzel3: interesting | 18:32 |
| wwitzel3 | natefinch: I added a break after the err check, and it works just fine now. | 18:32 |
| natefinch | .....and a test? :) | 18:33 |
| wwitzel3 | natefinch: I am probably going to break out the zone selection logic in to its own method, so it can actually be tested. | 18:33 |
| natefinch | huzzah! | 18:33 |
| wwitzel3 | natefinch: because right now, testing it would be a nightmare (probably why it doesn't have them) | 18:33 |
| natefinch | My thumbs aren't big enough for the thumbs up I want to give that :) | 18:34 |
| perrito666 | natefinch: get one of those gloves you guys use for sport games | 18:34 |
| natefinch | lol | 18:34 |
| wwitzel3 | haha | 18:35 |
| wwitzel3 | sinzui: bug #1392390 needs fixing in master as well, so once it is fixed there, what is the process of getting it in to 1.21 branch? | 18:35 |
| mup | Bug #1392390: maas zone selected for deployment that is occupied <cloud-installer> <landscape> <maas-provider> <placement> <regression> <juju-core:Triaged by wwitzel3> <juju-core 1.21:Triaged by wwitzel3> <https://launchpad.net/bugs/1392390> | 18:35 |
| sinzui | wwitzel3, branch 1.21, use git patch to apply changes from you master addition. request a pull...edit the destination from master to 1.21 | 18:37 |
| wwitzel3 | sinzui: got it, thanks | 18:38 |
| === kadams54 is now known as kadams54-away | ||
| natefinch | ericsnow: did you need me to look at something? | 19:09 |
| ericsnow | natefinch: in a few minutes | 19:09 |
| natefinch | ericsnow: kk | 19:09 |
| === kadams54-away is now known as kadams54 | ||
| ericsnow | natefinch: feel free to look over all the reviews or the whole patch, but I'd especially appreciate some feedback on the remaining (4) open issues on http://reviews.vapour.ws/r/402/. | 19:18 |
| ericsnow | natefinch: same with http://reviews.vapour.ws/r/346/ (which has no open issues) | 19:22 |
| natefinch | ericsnow: looking | 19:26 |
| ericsnow | natefinch: ta | 19:26 |
| perrito666 | rick_h_: I think I found the solution to my problem with having inexpensive cards with ipmi http://blog.michaelboman.org/2013/01/poor-mans-ipmi.html | 19:27 |
| wwitzel3 | ericsnow: is RB not auto pulling in PRs anymore? | 19:49 |
| ericsnow | wwitzel3: should be | 19:50 |
| ericsnow | wwitzel3: but now that you mention it | 19:50 |
| ericsnow | wwitzel3: github's API throttles your requests if you aren't properly authenticated | 19:50 |
| ericsnow | wwitzel3: I'm guessing that's the issue | 19:50 |
| ericsnow | wwitzel3: yep ("API rate limit exceeded"); I'm planning on fixing this today | 19:51 |
| ericsnow | wwitzel3: in the meantime you can still use rbt post | 19:52 |
| wwitzel3 | ericsnow: I haven't reinstalled it since my laptop reinstall, guess I should do that | 19:55 |
| ericsnow | wwitzel3: no worries | 19:55 |
| ericsnow | wwitzel3: you can still get a review on github if need be | 19:55 |
| wwitzel3 | ericsnow: https://github.com/juju/juju/pull/1146 fixes bug #1392390 | 19:57 |
| mup | Bug #1392390: maas zone selected for deployment that is occupied <cloud-installer> <landscape> <maas-provider> <placement> <regression> <juju-core:Triaged by wwitzel3> <juju-core 1.21:Triaged by wwitzel3> <https://launchpad.net/bugs/1392390> | 19:57 |
| natefinch | ericsnow: sorry, kids woke up early from their naps and wife just got home from midwife and has been instructed to take it easy for a few days, so I gotta run.... also means I didn't get to that review | 20:01 |
| ericsnow | natefinch: no worries | 20:01 |
| perrito666 | natefinch: tri a shot of scotch to each | 20:01 |
| natefinch | lol | 20:02 |
| * perrito666 would be a terrible parent | 20:02 | |
| perrito666 | ericsnow: did anything change on the way the mongo backup is done? | 20:23 |
| ericsnow | perrito666: a little | 20:23 |
| perrito666 | ok, what exactly? | 20:24 |
| ericsnow | perrito666: restore will be responsible for deleting the "ignored" databases after mongorestore runs | 20:24 |
| ericsnow | perrito666: should be trivial | 20:24 |
| perrito666 | ok, but what I am asking is, should I change the parameters I use for mongo when restoring? | 20:24 |
| perrito666 | besides that? | 20:25 |
| perrito666 | like, did you change the way you do the dump? | 20:25 |
| ericsnow | perrito666: nope | 20:25 |
| perrito666 | cool | 20:25 |
| ericsnow | perrito666: :) | 20:25 |
| fwereade | rogpeppe, yes, that is default-hook. but fwiw I think that something-changed is a better general approach, so we don't end up running the same code 500 times for no benefit | 21:42 |
| rick_h_ | perrito666: hah, that's one way to go | 22:03 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!