| fss | niemeyer: ptal https://codereview.appspot.com/6823060/ | 01:12 |
|---|---|---|
| niemeyer | fss: Will do, but tomorrow! :) | 01:12 |
| fss | niemeyer: np | 01:12 |
| fss | :-) | 01:12 |
| fss | niemeyer: time to sleep | 01:12 |
| niemeyer | fss: Yeah, here too | 01:13 |
| fss | niemeyer: see you | 01:13 |
| fss | niemeyer: adios | 01:13 |
| niemeyer | fss: Have a good one | 01:13 |
| fss | niemeyer: thanks, you too | 01:13 |
| wallyworld_ | jam: hi, got time for a chat? | 05:42 |
| jam | sure wallyworld_ | 05:42 |
| jam | wallyworld_: here or on mumble? | 05:42 |
| wallyworld_ | mumble easier | 05:42 |
| TheMue | Morning. | 07:17 |
| fwereade_ | TheMue, heyhey | 07:18 |
| TheMue | fwereade_: Hello. | 07:18 |
| fwereade_ | TheMue, how's it going? | 07:18 |
| TheMue | fwereade_: Fine, having first progress with LXC. | 07:19 |
| TheMue | fwereade_: And you? | 07:19 |
| fwereade_ | TheMue, not bad, kinda still drawing together the threads of everything I have to do, but getting there | 07:21 |
| TheMue | fwereade_: Oh, reminds me of Marks mail. Have to continue with it. | 07:21 |
| fwereade_ | TheMue, yeah, me too :) | 07:22 |
| TheMue | fwereade_: At least I already entered my vacation. But not yet my Copenhagen travel costs. | 07:22 |
| rogpeppe | fwereade_, TheMue: morning | 07:58 |
| TheMue | rogpeppe: Hiya. | 07:59 |
| fwereade_ | rogpeppe, heyhey | 08:03 |
| rogpeppe | fwereade_: i saw niemeyer mention consolidate-entity-watcher last night. is that a branch of yours? i couldn't see it anywhere. | 08:04 |
| fwereade_ | rogpeppe, yeah, it also went in last night | 08:05 |
| fwereade_ | rogpeppe, unit/machine/service watchers are now all just EntityWatcher | 08:05 |
| rogpeppe | fwereade_: cool | 08:07 |
| rogpeppe | fwereade_: i searched for "consolidate-entity-watcher" and it didn't find anything | 08:07 |
| fwereade_ | rogpeppe, hum, I think it's -watchers | 08:11 |
| fwereade_ | rogpeppe, https://codereview.appspot.com/6856060 | 08:11 |
| rogpeppe | fwereade_: yeah, that's my point - i found it just now, but i didn't find it last night | 08:11 |
| rogpeppe | fwereade_: gmail isn't so good at approximate searches | 08:11 |
| fwereade_ | rogpeppe, ah, got you | 08:12 |
| rogpeppe | fwereade_: really nice to see that go in, thanks for doing it! | 08:13 |
| fwereade_ | rogpeppe, cheers, a pleasure :) | 08:13 |
| fwereade_ | bah, I said I'd take over a branch of aram's but it's got a prereq I didn't spot | 08:35 |
| TheMue | fwereade_: Which one? | 08:42 |
| fwereade_ | TheMue, https://codereview.appspot.com/6814108/ depends on https://codereview.appspot.com/6820112/ | 08:42 |
| TheMue | fwereade_: Will take a look. | 08:43 |
| fwereade_ | TheMue, I think they may actually be independent enough that I can just focus on the one I need; we'll see | 08:43 |
| fwereade_ | TheMue, I don't think they need anyone else to look, I'm just grumbling :0 | 08:44 |
| TheMue | fwereade_: Hehe | 08:44 |
| TheMue | fwereade_: I've been involved in discussions about the second one. | 08:44 |
| TheMue | fwereade_: The changing of the firewaller lead to a breaking test. | 08:45 |
| TheMue | fwereade_: It's the test for the global mode. | 08:46 |
| fwereade_ | TheMue, ah, yes, I overheard some of them but wasn't following closely | 08:46 |
| fwereade_ | TheMue, that just increases my determination to avoid that branch if I can ;p | 08:46 |
| TheMue | fwereade_: :D | 08:46 |
| jam | wallyworld_: we're on mumble whenever you're back around | 09:00 |
| fwereade_ | ok, this is really just starting to drive me mad | 09:26 |
| fwereade_ | can anyone think of any problematic consequences to just changing the type of machine and relation id to string? | 09:26 |
| fwereade_ | rogpeppe, TheMue: ^? | 09:32 |
| rogpeppe | fwereade_: there are few places where the relationship between machine id and instance id might become less clear | 09:32 |
| rogpeppe | fwereade_: but in general i'm +1 | 09:33 |
| TheMue | fwereade_: Do we use them numerical, e.g. by auto-incrementing it for new instances? | 09:33 |
| fwereade_ | TheMue, ah, hmm | 09:33 |
| fwereade_ | TheMue, yes, but that shouldn't be a problem | 09:34 |
| TheMue | fwereade_: Then I see no problem. Typically I prefer alphanumerical identifier. | 09:34 |
| fwereade_ | TheMue, rogpeppe: cool, thanks, I think I'll give it a go then :) | 09:35 |
| TheMue | fwereade_: old school for me is, that numbers should only be used for calculating :) | 09:35 |
| rogpeppe | fwereade_: it'll be a fairly substantial job - juju status might be awkward | 09:35 |
| fwereade_ | TheMue, sounds like a solid rule to me :) | 09:36 |
| TheMue | fwereade_: cheers | 09:36 |
| rogpeppe | fwereade_: i'd be tempted to add a prefix that indicates the machine-id-ness of the id | 09:36 |
| rogpeppe | fwereade_: but that would be bad for backward compatibility | 09:36 |
| fwereade_ | rogpeppe, yeah, I don't *really* like the pure-number id but at least it makes it easy to distinguish in the places where there's ambiguity | 09:37 |
| fwereade_ | rogpeppe, eg juju status, actually :) | 09:37 |
| fwereade_ | rogpeppe, (I don't think the type change should be a big issue there... biggest consequences seems to be that we'll drop the jsonify func, which feels like a hack itself) | 09:39 |
| rogpeppe | fwereade_: yeah, i think it should end up cleaner | 09:39 |
| fwereade_ | rogpeppe, (although I might hold off until I have a word with niemeyer...) | 09:40 |
| rogpeppe | fwereade_: maybe a good plan | 09:40 |
| niemeyer | Morning all! | 10:16 |
| TheMue | niemeyer: Morning. | 10:17 |
| niemeyer | TheMue: Yo | 10:18 |
| dimitern | niemeyer: morning | 10:21 |
| fwereade_ | niemeyer, heyhey | 10:26 |
| fwereade_ | niemeyer, what would be your reaction to an attempt to just turn all machine/relation ids into strings? | 10:26 |
| niemeyer | fwereade_: Hmm | 10:30 |
| niemeyer | fwereade_: I'd of course resist before you actually explain the idea :-) | 10:30 |
| fwereade_ | niemeyer, this was primarily motivated by observing that AFAICT the services and machines watchers are trying to do the same thing, but actually don't | 10:31 |
| niemeyer | fwereade_: Trying to do the same thing in which sense? | 10:31 |
| fwereade_ | niemeyer, watch a collection for life changes | 10:31 |
| niemeyer | fwereade_: So you're saying that if we made them the same type, we'd get rid of another watcher? | 10:33 |
| fwereade_ | niemeyer, yeah | 10:33 |
| fwereade_ | niemeyer, I also suspect that anther couple *might* collapse into nothingness as well but it's not entirely clear whether that's a winning move | 10:34 |
| fwereade_ | niemeyer, I'd prefer not to get ahead of myself there ;) | 10:34 |
| niemeyer | fwereade_: I'd be fine with that.. it's been suggested before, but it was more of a gut feeling than a practical advantage | 10:34 |
| niemeyer | fwereade_: Although, hmm | 10:35 |
| niemeyer | fwereade_: There's some reasonable benefit in using ids in the database too, I guess | 10:35 |
| niemeyer | I mean, ints | 10:35 |
| fwereade_ | niemeyer, I wouldn't be too opposed to that in principle, but I'm not sure how significant the benefit will be in practice... would yu expand, I might be missing something? | 10:36 |
| fwereade_ | niemeyer, if it's significant then ofc it beats local convenience | 10:37 |
| niemeyer | fwereade_: Well, it's indeed hard to say how relevant it will be in practice, taking everything else into account, but there's some nice benefits of using ints in fields and indexes | 10:38 |
| niemeyer | fwereade_: 4B machines in four well-aligned bytes | 10:39 |
| fwereade_ | niemeyer, how about units? if the cost of string keys is significant, we should maybe be thinking about int ids for everything... | 10:44 |
| fwereade_ | niemeyer, we'll have roughly the same number of units as we do machines (at a minimum) | 10:44 |
| niemeyer | fwereade_: Units need composed keys? | 10:44 |
| niemeyer | fwereade_: Agreed | 10:44 |
| niemeyer | fwereade_: But is your argument that we should use the lowest denominator for all? :) | 10:44 |
| fwereade_ | niemeyer, my argument is that using different types for different entity ids is unhelpful at the state level, because we end up with unnecessary complexity that's almost certainly significant enough to harbour bugs | 10:46 |
| niemeyer | fwereade_: I was referring to this, specifically: | 10:46 |
| niemeyer | <fwereade_> niemeyer, how about units? if the cost of string keys is significant, we should maybe be thinking about int ids for everything... | 10:46 |
| fwereade_ | niemeyer, it may be that some aspect of some entity is enough at odds with this to sink the whole idea, but if so I don't yet see it | 10:47 |
| niemeyer | fwereade_: I'm still playing devil's advocate | 10:47 |
| niemeyer | fwereade_: Maybe we should change, but it's important to acknowledge those differences and take them into account | 10:47 |
| fwereade_ | niemeyer, my argument is that we should use any type we can find that has acceptable tradeoffs across the board, but that we should ideally go with a single id type | 10:48 |
| fwereade_ | niemeyer, sorry, that was poorly stated, hopefully still clearish | 10:48 |
| niemeyer | fwereade_: Yes, but I haven't seen many positive tradeoffs so far | 10:49 |
| niemeyer | fwereade_: The MachineWatcher has ~30 lines | 10:49 |
| niemeyer | fwereade_: ~40 maybe | 10:49 |
| niemeyer | fwereade_: What else are we obtaining by changing it? | 10:50 |
| fwereade_ | niemeyer, the Machine*s*Watcher has ~110 and Service*s*Watcher has nearly as much | 10:50 |
| fwereade_ | niemeyer, that's the only significant direct benefit I can see at the moment | 10:50 |
| fwereade_ | niemeyer, but I have a suspicion that it will also make it possible to consolidate ServiceRelationsWatcher, ServiceUnitsWatcher, MachinePrincipalUnitsWatcher and the forthcoming UnitSubordinatesWatcher | 10:52 |
| fwereade_ | niemeyer, all of which are/will do very similar things and have differences in implementation that I suspect are suboptimal | 10:52 |
| fwereade_ | er, is there a ServiceUnitsWatcher | 10:52 |
| fwereade_ | niemeyer, ah, there is, it's just not used | 10:53 |
| niemeyer | fwereade_: Okay, if you think it's useful, it's fine with me | 10:54 |
| niemeyer | fwereade_: I think rogpeppe had already raised that argument before for other reasons as well | 10:54 |
| fwereade_ | niemeyer, awesome, thanks :) | 10:54 |
| niemeyer | fwereade_: (reusing channels) | 10:55 |
| fwereade_ | niemeyer, there have certainly been little irritations related to it in the past | 10:55 |
| fwereade_ | niemeyer, ah, yes, nice | 10:55 |
| fwereade_ | niemeyer, (and then it would actually be trivial to reinstate EntityWatcher ids and thereby allow them to share channels if necessary | 10:56 |
| TheMue | lunchtime, biab | 10:58 |
| rogpeppe | niemeyer: morning! | 11:00 |
| niemeyer | rogpeppe: Heya | 11:01 |
| rogpeppe | niemeyer: i think this should be good to go now: https://codereview.appspot.com/6846066/ | 11:15 |
| niemeyer | rogpeppe: Cheers | 11:16 |
| niemeyer | It's about meeting time | 11:57 |
| niemeyer | I'll fire the hangout | 11:58 |
| jam | niemeyer: thanks, can you link it here? | 12:00 |
| niemeyer | https://plus.google.com/hangouts/_/500aec429367cf0688b60916ad5b255544d9bef8 | 12:00 |
| niemeyer | jam: ^ | 12:01 |
| jam | dimitern, wallyworld ^^ | 12:01 |
| rogpeppe | fwereade_: meeting? | 12:02 |
| fwereade_ | rogpeppe, joining | 12:02 |
| niemeyer | fwereade_: Coincidently, Matt (mattyw) has been doing some questions about how subordinates work this morning | 13:20 |
| niemeyer | fwereade_: I suggested hanging around here so we can add some freshness to our real-world use cases :) | 13:20 |
| mattyw | I'll try to remember to hang around here for the rest of the week as well | 13:22 |
| fwereade_ | niemeyer, ah, cool | 13:22 |
| niemeyer | rogpeppe: CL looks good | 13:49 |
| rogpeppe | niemeyer: cool | 13:49 |
| niemeyer | rogpeppe: One question: it looks like the generation of certs doesn't match the use of certs now on the config side of things? | 13:49 |
| rogpeppe | niemeyer: i'm working towards reconciling the two | 13:50 |
| niemeyer | rogpeppe: Super | 13:50 |
| rogpeppe | niemeyer: but i'm interested what particular aspect you're thinking about there | 13:50 |
| niemeyer | rogpeppe: The single pem vs. multiple pems | 13:51 |
| rogpeppe | niemeyer: that's actually the same in both state info and config. the place it's different is in Bootstrap | 13:51 |
| rogpeppe | niemeyer: and cloudinit.Config | 13:52 |
| rogpeppe | niemeyer: i'm talking about Environ.Bootstrap, not juju.Bootstrap BTW | 13:52 |
| niemeyer | rogpeppe: That's what I was talking about.. it's Bootstrap that generates the certs, isn't it | 13:52 |
| rogpeppe | niemeyer: juju.Bootstrap currently generates two certs, but we're moving towards making it generate only one - the cert and key for the bootstrap instance | 13:53 |
| rogpeppe | niemeyer: the root cert and key are going to be generated in environs, i think | 13:53 |
| niemeyer | rogpeppe: That's not what I was talking about, but why? | 13:54 |
| niemeyer | rogpeppe: Why would we change the place the cert is generated? | 13:54 |
| rogpeppe | niemeyer: that's what we were talking about yesterday | 13:54 |
| rogpeppe | niemeyer: (or was it the day before?) | 13:54 |
| niemeyer | rogpeppe: I don't know | 13:54 |
| niemeyer | rogpeppe: I don't recall about either | 13:54 |
| niemeyer | rogpeppe: Why would we change the place the cert is generated? | 13:54 |
| rogpeppe | [13:46:25] <niemeyer> rogpeppe: Why is that different from Bootstrap? | 13:56 |
| rogpeppe | [13:47:04] <niemeyer> rogpeppe: The whole thing is starting to feel a bit hackish, to be honest.. | 13:56 |
| rogpeppe | [13:47:21] <niemeyer> rogpeppe: We have a mechanism to read the environment data from disk abstracted away | 13:57 |
| rogpeppe | [13:47:34] <niemeyer> rogpeppe: and then we take that data, and go back to disk to look for more | 13:57 |
| rogpeppe | [13:47:46] * niemeyer looks at some code | 13:57 |
| rogpeppe | [13:48:18] <rogpeppe> niemeyer: we need to be able to save something to disk and then recover that, and that's what this is about | 13:57 |
| rogpeppe | [13:49:04] <rogpeppe> niemeyer: the thing that i think is most hackish is the interface in the juju package, which really shouldn't know about $HOME stuff really, probably. | 13:57 |
| rogpeppe | [13:49:12] <niemeyer> rogpeppe: This looks like the wrong place to be doing this | 13:57 |
| rogpeppe | [13:49:15] <niemeyer> rogpeppe: Exactly | 13:57 |
| niemeyer | rogpeppe: Indeed, thanks, the point is $HOME | 13:57 |
| niemeyer | rogpeppe: So, my original point wasn't about that, though | 13:57 |
| niemeyer | rogpeppe: I was referring to the fact Bootstrap seems to generate a single PEM file, while config uses two, of different names | 13:58 |
| niemeyer | rogpeppe: But I guess you'll sort that out at the same time | 13:58 |
| rogpeppe | niemeyer: yeah, that's because mgo uses a single one, and that's how the one in Bootstrap is used, but we can easily make bootstrap take two arguments and split the field in cloudinit.Config too | 13:58 |
| rogpeppe | niemeyer: then cloudinit will just join them together | 13:59 |
| niemeyer | rogpeppe: mgo doesn't do anything about either.. | 13:59 |
| rogpeppe | niemeyer: sorry mongod | 13:59 |
| rogpeppe | niemeyer: mongod expects a single file with them both in | 13:59 |
| niemeyer | rogpeppe: That's irrelevant as well, I think.. all of that logic sits at the client | 13:59 |
| rogpeppe | niemeyer: the whole point of the cert that's passed to bootstrap is that it's used by mongod, no? | 14:00 |
| rogpeppe | niemeyer: (i'm not saying that we shouldn't split it up for consistency's sake, though) | 14:00 |
| rogpeppe | niemeyer: to cert that's passed to Environ.Bootstrap, i mean, just to be clear | 14:01 |
| niemeyer | rogpeppe: It's irrelevant.. mongod never ever sees whatever we do at the client side | 14:01 |
| rogpeppe | s/to/the/ | 14:01 |
| niemeyer | rogpeppe: And mongod never ever sees the CA certificates | 14:01 |
| rogpeppe | niemeyer: i'm not talking about the CA certificates | 14:01 |
| niemeyer | rogpeppe: The private key and the cert file are handled as separate files in the configuration | 14:01 |
| rogpeppe | niemeyer: i'm not talking about those either | 14:02 |
| niemeyer | rogpeppe: Yes, but that conversation started because of what I was talking about, I think :-) | 14:02 |
| niemeyer | rogpeppe: If you want to go in other directions we can do that too, but then we have to agree to change subjects :) | 14:02 |
| niemeyer | <niemeyer> rogpeppe: One question: it looks like the generation of certs doesn't match the use of certs now on the config side of things? | 14:02 |
| rogpeppe | niemeyer: sorry, i thought i'd confirmed that you were talking about Environ.Bootstrap | 14:04 |
| rogpeppe | niemeyer: i agree about juju.Bootstrap - that argument is going away | 14:04 |
| rogpeppe | niemeyer: and it'll come from the Environ | 14:04 |
| rogpeppe | niemeyer: so all those []byte(testing.RootCertPEM + testing.RootKeyPEM) will go away too | 14:05 |
| niemeyer | rogpeppe: environ.Bootstrap doesn't generate certificates, so the whole discussion wouldn't make sense | 14:05 |
| niemeyer | rogpeppe: But cool, I guess we're in sync now | 14:05 |
| rogpeppe | niemeyer: yup | 14:05 |
| rogpeppe | niemeyer: i'd like to have a chat about how we want the generation of certs and environments.yaml to work from the user's p.o.v. | 14:07 |
| rogpeppe | niemeyer: at some point, hopefully soon | 14:07 |
| niemeyer | rogpeppe: Cool | 14:07 |
| rogpeppe | niemeyer: here's one possibility: http://paste.ubuntu.com/1372497/ | 14:13 |
| niemeyer | rogpeppe: I still prefer the plan we discussed yesterday | 14:13 |
| niemeyer | rogpeppe: On bootstrap, generate a sample file | 14:14 |
| niemeyer | rogpeppe: if it doesn't exist | 14:14 |
| niemeyer | rogpeppe: For now, ec2 info only, uncommented | 14:14 |
| rogpeppe | niemeyer: the problem i see with that is that the sample config is for one provider, which may not be the one that the user wants | 14:14 |
| niemeyer | rogpeppe: Some day, local uncommented, plus everything else commented | 14:14 |
| niemeyer | rogpeppe: There's only a single provider today | 14:14 |
| rogpeppe | niemeyer: and when they want another one, there's no way of getting that | 14:14 |
| rogpeppe | niemeyer: there will be many providers | 14:15 |
| niemeyer | rogpeppe: Either the user wants that, or she/he may go home | 14:15 |
| niemeyer | <niemeyer> rogpeppe: Some day, local uncommented, plus everything else commented | 14:15 |
| niemeyer | rogpeppe: We can have all of them | 14:15 |
| niemeyer | rogpeppe: Commented out | 14:15 |
| niemeyer | rogpeppe: Plus local uncommented | 14:15 |
| niemeyer | rogpeppe: Which is ubiquitous | 14:15 |
| rogpeppe | niemeyer: BTW the current python doesn't generate any commented lines | 14:15 |
| niemeyer | rogpeppe: Not sure how that's relevant | 14:16 |
| rogpeppe | niemeyer: just saying | 14:16 |
| niemeyer | rogpeppe: Me too :) | 14:16 |
| rogpeppe | niemeyer: what happens if a new provider is added and the user wants a sample config for that? | 14:17 |
| rogpeppe | niemeyer: it seems pretty hackish to require them to remove ~/.juju/environments.yaml in order to get a sample | 14:17 |
| niemeyer | rogpeppe: Jeshh.. can we please get something going? | 14:18 |
| niemeyer | rogpeppe: We have ONE PROVIDER TODAY | 14:18 |
| niemeyer | rogpeppe: WriteFile(sample) | 14:18 |
| niemeyer | done! | 14:18 |
| rogpeppe | niemeyer: ok. i'm just wondering whether to have it as an entry point to EnvironProvider or not | 14:18 |
| niemeyer | rogpeppe: We can improve the situation ad-infinitum | 14:18 |
| rogpeppe | niemeyer: e.g. EnvironProvider.SampleConfig() *config.Config | 14:19 |
| niemeyer | rogpeppe: I could literally have implemented the feature we want today and pushed for review in the time we took to discuss this so far | 14:19 |
| niemeyer | rogpeppe: We can improve APIs and make it super fancy once we have 10 providers and 10 million users | 14:20 |
| rogpeppe | niemeyer: so you're happy with this behaviour happening on any juju command? (i thought it might be better if it was only done on bootstrap) | 14:20 |
| niemeyer | <niemeyer> rogpeppe: On bootstrap, generate a sample file | 14:21 |
| rogpeppe | niemeyer: ok. python version does it on any command. | 14:21 |
| niemeyer | rogpeppe: I'm happy with either, though.. whatever users find the most convenient | 14:21 |
| niemeyer | rogpeppe: Ask Jorge | 14:21 |
| rogpeppe | niemeyer: will do | 14:22 |
| niemeyer | Alright, lunch time here | 14:23 |
| rogpeppe | niemeyer: can you remind me of his irc nick? | 14:23 |
| rogpeppe | niemeyer: enjoy! | 14:23 |
| niemeyer | rogpeppe: jcastro | 14:23 |
| rogpeppe | niemeyer: thanks | 14:23 |
| rogpeppe | niemeyer: submitted, thanks. which means i can propose the next in line: https://codereview.appspot.com/6855054 | 14:34 |
| rogpeppe | s/propose/re-propose/ | 14:35 |
| TheMue | *: anyone interested in the first lxc steps? https://codereview.appspot.com/6852065/ and https://codereview.appspot.com/6853075/ | 14:36 |
| rogpeppe | TheMue: looking | 14:39 |
| TheMue | thx | 14:39 |
| rogpeppe | TheMue: any particular reason that run returns bytes.Buffer rather than []byte ? | 14:43 |
| TheMue | rogpeppe: Oh, no, took it from the original command that I used before wrapping it. Yes, a change makes sense. | 14:44 |
| dimitern | why is an Instance Id string and a machine ID - int? | 15:06 |
| TheMue | dimitern: Can you scroll about 5h back? There's a discussion about this topic and possibly harmonizing them between fwereade_ and niemeyer. | 15:08 |
| fwereade_ | dimitern, I'm just making machine IDs into strings as we speak :) | 15:09 |
| dimitern | TheMue: I see | 15:09 |
| dimitern | fwereade_: Cool! :) | 15:09 |
| fwereade_ | it's rather uglier than I imagined, but hey ho | 15:09 |
| dimitern | fascinating! That seems like a massive simplification | 15:17 |
| niemeyer | dimitern, fwereade_: FWIW, having instance id and machine id having different types was actually a plus :) | 15:20 |
| rogpeppe | TheMue: you've got a review | 15:20 |
| dimitern | niemeyer: to define better, fine-grained architecture? | 15:20 |
| niemeyer | dimitern: No, because they're not interchangeable | 15:21 |
| TheMue | rocheers | 15:21 |
| TheMue | rogpeppe: cheers *grmpf* | 15:21 |
| rogpeppe | niemeyer: yeah, it's a pity. we *could* make instance ids have their own type actually. that might be better. | 15:22 |
| dimitern | niemeyer: I see. But nevertheless, the polymophism in this case still rocks.. | 15:22 |
| rogpeppe | type InstanceId string | 15:22 |
| rogpeppe | in environs | 15:22 |
| rogpeppe | then we'd get the best of both worlds | 15:23 |
| dimitern | rogpeppe: how about MachineId | 15:23 |
| rogpeppe | dimitern: no | 15:23 |
| rogpeppe | dimitern: machine != instance | 15:23 |
| rogpeppe | dimitern: and that's why it's a good idea to keep the types separate - it's a very easy category error to make | 15:24 |
| rogpeppe | fwereade_: what do you think? | 15:24 |
| dimitern | rogpeppe: like passing one instead of the other? | 15:24 |
| rogpeppe | dimitern: yeah. thinking they mean the same thing | 15:24 |
| dimitern | rogpeppe: that's sounds very useful, Go-ish :) | 15:25 |
| rogpeppe | dimitern: yeah, it's a useful thing to be able to do | 15:25 |
| fwereade_ | rogpeppe, honestly, I'm not sure the problem is really in the types | 15:25 |
| rogpeppe | fwereade_: why are you changing the types then? | 15:25 |
| dimitern | rogpeppe: my concern was the int/string distinction, while despite both being strings, having separate types is even better | 15:25 |
| fwereade_ | rogpeppe, the problem is that we have instance and machine ids, and they sound like the same thing until you come to understand the model | 15:26 |
| rogpeppe | fwereade_: exactly. which is why it's useful to distinguish them by giving them different types | 15:26 |
| rogpeppe | fwereade_: even if the underlying type of both of them is string | 15:26 |
| fwereade_ | rogpeppe, I dunno -- what's the distinction here between that and, say, giving a unit name its own type? | 15:27 |
| fwereade_ | rogpeppe, that will prevent people passing service names when they mean unit names | 15:27 |
| rogpeppe | fwereade_: because we want to be able to use entity ids in an interchangeable way | 15:27 |
| dimitern | fwereade_: probably it's not as easy to mix up unitids with others | 15:28 |
| rogpeppe | fwereade_: otherwise why are you making machine id into a string? | 15:28 |
| fwereade_ | rogpeppe, sorry [hone | 15:28 |
| rogpeppe | fwereade_ has an alien phone :-) | 15:28 |
| dimitern | the new iGotcha ]I[ | 15:29 |
| dimitern | ok, so in the environs/interface.go there is this comment: http://bazaar.launchpad.net/~gophers/juju-core/trunk/view/head:/environs/interface.go#L113 | 15:31 |
| dimitern | can somebody give me an example of the described inconsistency of the provider? | 15:31 |
| niemeyer | fwereade_: Slightly more sensitive in that case, since they both are seen as a "machine" in some senses.. not suggesting it's a show stopper at this point, though | 15:32 |
| rogpeppe | niemeyer: what do you think of giving instance id its own type? | 15:33 |
| rogpeppe | dimitern: for example, if you launch a machine, you might not be able to see any information on that machine for a while | 15:34 |
| dimitern | rogpeppe: you mean, e.g. calling StartInstance successfully, then calling AllInstances won't return the new machine in the result set? | 15:35 |
| rogpeppe | dimitern: or if you change something in a storage, that change might not be visible for a while. or it may be visible, then become invisible again. | 15:35 |
| rogpeppe | dimitern: yup | 15:36 |
| rogpeppe | dimitern: or it might | 15:36 |
| rogpeppe | dimitern: no way to know | 15:36 |
| rogpeppe | dimitern: it depends (presumably) on which server you're talking to. | 15:36 |
| dimitern | rogpeppe: but that's the eventual consistency thing of the cloud | 15:36 |
| rogpeppe | dimitern: yeah | 15:36 |
| niemeyer | rogpeppe: It's an interesting idea, but I'd rather see how we feel about the machine id change on itself first | 15:36 |
| rogpeppe | niemeyer: i'd prefer to keep the types consistently separate actually | 15:37 |
| rogpeppe | niemeyer: i think that's easier to do than merge then separate. | 15:37 |
| rogpeppe | dimitern: there's no way to get around it | 15:37 |
| niemeyer | rogpeppe: Let's please not change everything at once | 15:37 |
| niemeyer | rogpeppe: There's no reason to do that | 15:37 |
| niemeyer | rogpeppe: fwereade_ is suggesting a huge change | 15:37 |
| rogpeppe | niemeyer: i wasn't suggesting that | 15:37 |
| niemeyer | rogpeppe: Which doesn't require anything *else* in addition to it | 15:38 |
| rogpeppe | niemeyer: i'd do the int->InstanceId change first | 15:38 |
| niemeyer | Let's have a look at it, and ponder calmly | 15:38 |
| niemeyer | No hurry there | 15:38 |
| rogpeppe | niemeyer: i mean string->InstanceId of course | 15:38 |
| rogpeppe | niemeyer: then int->string | 15:38 |
| niemeyer | rogpeppe: Yes, and I mean int=>string, *stop*, *think* | 15:38 |
| rogpeppe | niemeyer: the problem is that makes it harder to move on from there because then we've got instance id and machine id being identical | 15:39 |
| niemeyer | rogpeppe: They'd have the same type, which is not a show stopper at all | 15:39 |
| rogpeppe | niemeyer: sure, it's not hard. it's just a little harder. | 15:40 |
| niemeyer | rogpeppe: It's no harder.. instance id is completely independent | 15:40 |
| niemeyer | Anyway, unfortunately I need to step out now for the daily errand I mentioned over email | 15:41 |
| niemeyer | Tomorrow is the last day | 15:41 |
| niemeyer | Back later to continue | 15:41 |
| fwereade_ | rogpeppe, sorry, back, that took longer than expected | 15:50 |
| fwereade_ | rogpeppe, I want to change ids to strings primarily so we can strip out some of the watcher duplication (within which bugs will lurk, I think) | 15:51 |
| rogpeppe | fwereade_: that's what i thought | 15:51 |
| fwereade_ | rogpeppe, I'm not necessarily opposed to a special InstanceId type but it is completely orthogonal AFAICT | 15:51 |
| rogpeppe | fwereade_: yeah - i just wondered if it might be better to keep the two types rigorously separate through the change process | 15:52 |
| fwereade_ | rogpeppe, and honestly it feels to me akin to having `type MaxInt int; type MinInt int; type Range{Max MaxInt; Min MinInt}` | 15:52 |
| rogpeppe | fwereade_: i.e. leverage the type system to make sure we don't mix 'em up at some point in this large change | 15:52 |
| fwereade_ | rogpeppe, they're two different things | 15:52 |
| rogpeppe | fwereade_: it's totally different from that | 15:53 |
| rogpeppe | fwereade_: we never want to operate on an instance id and a machine id together | 15:53 |
| fwereade_ | rogpeppe, they're both numbers; they both define one end of a range; if people don't understand ranges they might get it wrong | 15:53 |
| rogpeppe | fwereade_: which is not the case for MinInt and MaxInt | 15:53 |
| fwereade_ | we'd better separate them | 15:53 |
| fwereade_ | but people might get it wrong! ;p | 15:53 |
| rogpeppe | fwereade_: bad analogy | 15:54 |
| fwereade_ | rogpeppe, ok, GrossProfit and NetProfit? | 15:54 |
| rogpeppe | fwereade_: those are both numbers that can be used in similar ways | 15:54 |
| rogpeppe | fwereade_: instance id and machine id can never be used in similar ways | 15:54 |
| rogpeppe | fwereade_: it's more similar to the difference between rune and int | 15:55 |
| fwereade_ | rogpeppe, I dunno -- by that token, just about every string we use should actualy be typed | 15:56 |
| rogpeppe | fwereade_: i don't think so | 15:56 |
| rogpeppe | fwereade_: how many of the strings we use represent a distinctive thing that doesn't make sense to be combined with other strings? | 15:57 |
| fwereade_ | rogpeppe, don't get the "combined" bit -- is that a consideration? if so we'd better untype a bunch of things :) | 15:58 |
| rogpeppe | fwereade_: i think the main consideration should be: does it work well as a separate type? | 15:59 |
| fwereade_ | rogpeppe, I guess it will be pretty much opaque to juju itself, and basically only want to be translated on the way in/out | 16:00 |
| rogpeppe | fwereade_: exactly | 16:00 |
| rogpeppe | fwereade_: outside of the provider, it could be anything - it's entirely provider-defined. | 16:01 |
| dimitern | rogpeppe: well, if it's type InstanceId string it's not exactly provider defined | 16:01 |
| fwereade_ | rogpeppe, so, ok, we get a change at a small amount of extra safety if we add an InstanceId type; but this remains independent of entity id considerations, right? | 16:01 |
| rogpeppe | dimitern: the contents are, but yeah | 16:02 |
| fwereade_ | dimitern, (yeah, indeed, I was wondering about non-string ones, but they should be trivial to translate anyway) | 16:02 |
| rogpeppe | fwereade_: it does. except that i *think* that keeping the types separate throughout the change will make things easier. | 16:02 |
| fwereade_ | rogpeppe, ISTM that machine and instance IDs rarely actually come into direct contact, but you certainly know that stuff betterthan me | 16:03 |
| rogpeppe | fwereade_: i suspect there are a fair few places where it's only the type that makes it clear what's being returned (machine id or instance id) | 16:04 |
| rogpeppe | fwereade_: i'm sure i've written a few such things myself (i'm returning an int - it's obvious i must be returning a machine id not an instance id) | 16:04 |
| rogpeppe | afk for 10 mins | 16:09 |
| rogpeppe | back | 16:16 |
| TheMue | rogpeppe: Thx for your review, it has been very valuable and I simplified the whole stuff a lot. | 16:35 |
| rogpeppe | TheMue: cool | 16:35 |
| fwereade_ | gents, I need to pop out for a while; calling it a day for now, but I'll probably be around again laterthis evening | 16:43 |
| TheMue | Funny, just detected that Sidney has DST but Brisbane not. So Dave and Ian have one hour difference. | 16:43 |
| TheMue | fwereade_: Enjoy. | 16:43 |
| TheMue | rogpeppe: Thanks again. ;) | 16:51 |
| TheMue | So, I'm off for today, dinner time. CU tomorrow again. | 17:02 |
| rogpeppe | niemeyer: branch enabling on-demand certificate generation when certificate not found by config: https://codereview.appspot.com/6782094/ | 18:06 |
| rogpeppe | which brings me neatly to the end of the day | 18:07 |
| rogpeppe | g'night all | 18:07 |
| rogpeppe | see y's tomorrow | 18:07 |
| === rog is now known as Guest84948 | ||
| === negronjl` is now known as negronjl | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!