| hazmat | SpamapS, the versions vary across the env | 00:00 |
|---|---|---|
| hazmat | sorry poor naming choice | 00:00 |
| SpamapS | versions of juju, but as long as provisioning agent doesn't pass a zk secret.. you just wouldn't have a zk secret, right? | 00:00 |
| hazmat | mixed mode == multiple versions extant in the env | 00:00 |
| SpamapS | sure ok I don't care what you call it, I just want it to work :) | 00:00 |
| hazmat | SpamapS, yup.. no otp identity token, nothing to do | 00:00 |
| SpamapS | Its the only thing I really think we have to get into 12.04.1 | 00:01 |
| SpamapS | everything else we can pull people to PPA I think | 00:01 |
| hazmat | SpamapS, my timeline is bit stretched | 00:01 |
| SpamapS | if we merged it now | 00:01 |
| SpamapS | would it just break all add-units ? | 00:01 |
| hazmat | SpamapS, its not done, i still need to propogate identities to all the agents | 00:02 |
| hazmat | SpamapS, so with the move to august for 12.04.1 what's the date for last merge | 00:02 |
| * SpamapS makes the FAIL face | 00:02 | |
| hazmat | you wanted 6 weeks | 00:02 |
| SpamapS | oh dude | 00:02 |
| SpamapS | I don't care about *actual* 12.04.1 | 00:02 |
| SpamapS | June 6 man. I don't want to hold onto trunk like this for 3 months | 00:02 |
| SpamapS | If we can't get it in, we can't get it in. | 00:03 |
| SpamapS | c'est la vie. | 00:03 |
| hazmat | actually i take that back, there is a branch that does the agent identity thing | 00:03 |
| hazmat | SpamapS, let's just branch then | 00:03 |
| hazmat | SpamapS, you want 9 weeks of no changes ? | 00:04 |
| hazmat | for 12.04.1? | 00:04 |
| SpamapS | No stress. I just think, priority wise, thats the only one that really needs to get back to the conservative "hey whats this thing juju" users that will use it from the distro. | 00:04 |
| hazmat | june 6th isn't going to happen, i'm fully booked till june 14th | 00:04 |
| SpamapS | hazmat: alright, then we won't ship stuff to those users. | 00:04 |
| hazmat | i'd have to pull a resource to make other stuff move | 00:04 |
| SpamapS | or we'll branch a stable branch. | 00:04 |
| SpamapS | Don't stress, thats the key. | 00:05 |
| SpamapS | It happens, or it doesn't, but no heroic efforts are needed. | 00:05 |
| hazmat | SpamapS, too late.... my post uds bliss has passed | 00:05 |
| SpamapS | hazmat: thats why we have deadlines | 00:05 |
| SpamapS | so we *don't* have to stress | 00:05 |
| SpamapS | immovable deadlines even | 00:05 |
| hazmat | SpamapS, that works when you only have one deadline maybe, but when you have four concurrently, its a bit different | 00:05 |
| SpamapS | thats why stuff needs to be done *before* feature freeze | 00:05 |
| SpamapS | even in universe/unsupported .. we shouldn't target to go nuts the last week of the cycle. | 00:06 |
| SpamapS | hazmat: meh, it still works. You have a deadline, you know how much time you have available for this one project. Its not enough.. resource starvation communicated. No stress. | 00:06 |
| hazmat | ack | 00:06 |
| SpamapS | And I can set expectations for the precise-updates version appropriately as well. | 00:07 |
| SpamapS | I think instead I'll just try and make an 'unstable' PPA on June 6, and make sure there's some way to get juju-origin to use it. | 00:08 |
| SpamapS | then we can just freeze ppa:juju/ppa onto the galapgos release, which I hope we can call 0.5.1, and then just start building trunk into ppa:juju/unstable and let users know they should get on that one if they want the new stuff from honolulu | 00:09 |
| hazmat | SpamapS, so 12.04.1 got moved back a month, why doesn't our deadline move with it? | 00:12 |
| === flaviamissi_ is now known as flaviamissi | ||
| hazmat | amusing.. https://bugs.launchpad.net/edubuntu/+bug/1000000 | 00:48 |
| jimbaker | yes, a nice variant on https://bugs.launchpad.net/ubuntu/+bug/1 | 01:03 |
| SpamapS | hazmat: because I want trunk to open up for features | 03:52 |
| hazmat | SpamapS, then let's cut a precise branch, but that should still change june 6th to july 6th | 03:59 |
| SpamapS | hazmat: we can just make the precise packaging branch the precise branch :) | 04:01 |
| SpamapS | hazmat: in fact, with the Low priority stuff thats been landing in galapagos, I think I may give up on trying to ship "galapagos" into precise-updates | 04:03 |
| SpamapS | hazmat: the idea was extreme discipline so we could adhere to SRU policy. That has basically gone out the window. | 04:06 |
| hazmat | SpamapS, i think we have different notions of extreme.. anyways.. the question to me at the moment is what in the current set of revs doesn't meet that notion, we can revert, cut the branch, and reapply to trunk | 04:10 |
| SpamapS | hazmat: Well the SRU policy is pretty clear that only high impact bugs will be SRU'd | 04:21 |
| SpamapS | hazmat: so, cosmetic fixes are generally rejected, and with the heavy testing requirement, are kind of a waste of time. | 04:22 |
| hazmat | SpamapS, but things aren't tested in isolation? | 04:22 |
| SpamapS | hazmat: plus we have to extract each and every one into its own quilt patch so SRU team can review each change. It just becomes a lot of red tape and b.s. for cosmetic fixes. | 04:22 |
| hazmat | SpamapS, this also applies to universe? | 04:22 |
| hazmat | or that thing formerly known as | 04:23 |
| hazmat | hm | 04:23 |
| SpamapS | hazmat: yes, the only difference is that with universe, the community is expected to do the fixes (which, since we're juju devs.. we are considered the community) | 04:23 |
| hazmat | amusing | 04:23 |
| SpamapS | the policy is identical, just the expectation of response is different. | 04:23 |
| hazmat | SpamapS, so outside of 536, i could probably a valid case for each of the other 4 commits | 04:23 |
| hazmat | SpamapS, also we need the output formating fix that bcsaller is working on wrt to not having python stringification of values | 04:25 |
| hazmat | ie. one more | 04:25 |
| SpamapS | hazmat: we also have the problem of not being able to test from -proposed easily. ;) | 04:26 |
| hazmat | yeah.. i got lost in the weeds trying to make local use cloud-image so i didnt have to implement that twice | 04:27 |
| hazmat | lp:~hazmat/juju/proposed-support ;-) | 04:28 |
| SpamapS | so many branches | 04:29 |
| SpamapS | so little time :) | 04:29 |
| SpamapS | hazmat: I'm willing to do all the test verification on even the Low priority bugs .. as long as the SRU team is willing to review the uploads. | 04:32 |
| SpamapS | hazmat: I figure if we ship 3 - 5 in an upload every 2 weeks the SRU team should be ok with that. | 04:33 |
| hazmat | SpamapS, any reason not to go ahead and cut the precise branch now | 04:36 |
| SpamapS | hazmat: we have.. its at lp:ubuntu/juju | 04:38 |
| SpamapS | actually lp:ubuntu/precise/juju ;) | 04:38 |
| hazmat | gotcha | 04:38 |
| hazmat | cool | 04:38 |
| SpamapS | hazmat: so I'll just manage the flow of things from lp:juju -> lp:ubuntu/precise/juju :) | 04:42 |
| SpamapS | hazmat: and you guys can keep doing trivials as long as they don't cause me any conflicts. :) | 04:42 |
| davecheney | rogpeppe: you in the house ? | 06:37 |
| fwereade | davecheney, heyhey | 07:12 |
| fwereade | davecheney, instance id is not the same as machine id; machine id is juju-internal but instance id is provider-supplied | 07:13 |
| fwereade | davecheney, the providers don't have any reason I'm aware of to know about the details of juju state, so I don't think they should know about instance ids | 07:14 |
| fwereade | davecheney, gaah, providers should not know about *machine* ids | 07:14 |
| fwereade | davecheney, they should deal purely with *instance* ids | 07:15 |
| fwereade | davecheney, the machine state should contain the corresponding instance id which you can use to shut down the machine | 07:16 |
| rogpeppe | mornin' to anyone who's around... | 09:46 |
| Aram | hi there. | 10:12 |
| rogpeppe | Aram: if you don't type my irc alias, i don't see your remark! | 11:32 |
| Aram | :). | 11:32 |
| rogpeppe | lunch | 12:32 |
| niemeyer | Hello! | 13:01 |
| niemeyer | I suspect my Thinkpad's battery has just died on me | 13:01 |
| niemeyer | Quick reboot.. | 13:02 |
| rogpeppe | niemeyer: hiya | 13:09 |
| niemeyer | rogpeppe: Heya | 13:10 |
| niemeyer | Looks like battery is no more indeed | 13:10 |
| rogpeppe | niemeyer: bummer | 13:10 |
| rogpeppe | niemeyer: about List: i've gone with directories, but said that it should return the entire (recursive) contents of the directory. that means you can find out the entire contents of a bucket without explicitly recurring. | 13:12 |
| niemeyer | rogpeppe: Ok | 13:13 |
| niemeyer | rogpeppe: Where do I get a replacement bat in the UK? | 13:14 |
| rogpeppe | niemeyer: also, you said you had a couple of suggestions around that paste. do you remember another? | 13:14 |
| rogpeppe | niemeyer: in the UK i'd order it on the internet | 13:14 |
| rogpeppe | niemeyer: why UK? | 13:14 |
| niemeyer | rogpeppe: ROTFL.. *really*!? You'd use the *internet*! | 13:15 |
| niemeyer | :-) | 13:15 |
| rogpeppe | niemeyer: i can't be more specific - i'd froogle it. | 13:15 |
| niemeyer | rogpeppe: Yeah, got it.. I just had a good laugh :) | 13:15 |
| niemeyer | rogpeppe: I may be going there in a couple of weeks | 13:15 |
| rogpeppe | niemeyer: cool. whereabouts? | 13:15 |
| niemeyer | rogpeppe: London | 13:16 |
| niemeyer | rogpeppe: There's a sprint being planned and I was invited to help out as possible | 13:17 |
| rogpeppe | niemeyer: would it be helpful if i sourced a battery for you? | 13:17 |
| niemeyer | rogpeppe: Not sure.. if I can find a good spot to buy it, I might just send it to the office. | 13:18 |
| niemeyer | rogpeppe: I'd appreciate your help in case the place doesn't take international credit cards, though | 13:18 |
| rogpeppe | niemeyer: sounds like a good plan. that also sounds like a possibility. | 13:18 |
| niemeyer | rogpeppe: I'll have a look later at Amazon UK to see if I can find anything | 13:19 |
| rogpeppe | niemeyer: usually they do, but you might find the exchange rate isn't great. | 13:19 |
| niemeyer | For now, just put my battery in the fridge for it to relax for a moment | 13:19 |
| niemeyer | rogpeppe: The exchange rate with credit cards is controlled by the government.. they're forced to use the official rates | 13:19 |
| rogpeppe | chill out that battery man | 13:19 |
| niemeyer | rogpeppe: I'll just have to pay 6% extra of tax, unfortunately | 13:20 |
| rogpeppe | niemeyer: really? i never knew that. | 13:20 |
| rogpeppe | niemeyer: on top of our 20% VAT? | 13:20 |
| niemeyer | rogpeppe: Yeah.. financial operations tax in Brazil | 13:20 |
| niemeyer | rogpeppe: To discourage people from buying stuff abroad, is my guess | 13:20 |
| niemeyer | rogpeppe: Silly | 13:20 |
| rogpeppe | niemeyer: sounds like you *do* want me to buy it for you :-) | 13:21 |
| niemeyer | rogpeppe: Hehe, thanks, but don't worry for now.. | 13:21 |
| rogpeppe | niemeyer: just let me know if you need it | 13:21 |
| niemeyer | This is the second time this happens to me, btw | 13:22 |
| niemeyer | Last Thinkpad I had, the exact same happened | 13:22 |
| niemeyer | Looks like Lenovo has issues with batteries | 13:22 |
| Aram | what model is this thinkpad? | 13:23 |
| niemeyer | Aram: T410 | 13:41 |
| Aram | I have one as well. | 13:41 |
| Aram | curious that you had problems, I never had battery problems with any thinkpad. | 13:42 |
| Aram | had a lot of problems with dells and hps though | 13:43 |
| niemeyer | Aram: It's the second time | 13:52 |
| niemeyer | Aram: Last one died in the same way | 13:52 |
| Aram | strange. | 13:52 |
| niemeyer | Aram: Maybe I use it too much? :) | 13:52 |
| niemeyer | rogpeppe: My current battery costs £164.16 at Lenovo (!!) | 14:07 |
| rogpeppe | niemeyer: yeah, they're way expensive. | 14:07 |
| rogpeppe | niemeyer: i was looking at getting a spare recently. | 14:07 |
| niemeyer | It's almost the price of a whole laptop these days | 14:07 |
| rogpeppe | niemeyer: you can get an OEM battery | 14:07 |
| rogpeppe | niemeyer: but who knows how good it'll be... | 14:07 |
| niemeyer | Oh, found the same one on Amazon uk for £84.56 | 14:09 |
| rogpeppe | niemeyer: that's cool. link? | 14:10 |
| niemeyer | I wonder how long it's been in stock though.. that's always an issue with batteries | 14:10 |
| niemeyer | rogpeppe: http://www.amazon.co.uk/Lenovo-ThinkPad-Battery-55-battery/dp/B003FXQ3ZK/ref=sr_1_1?ie=UTF8&qid=1337177298&sr=8-1 | 14:10 |
| rogpeppe | niemeyer: given that they're the cheapest supplier, maybe they'll be clearing stock quickly... | 14:11 |
| rogpeppe | niemeyer: interesting to see how much of a markup lenovo direct has though. | 14:12 |
| niemeyer | rogpeppe: Yeah.. even if the cheap one lasts half as long, it's still ok :) | 14:13 |
| rogpeppe | niemeyer: some bizarre behaviour from gnu tar: http://paste.ubuntu.com/990736/ | 14:21 |
| niemeyer | Reading | 14:22 |
| rogpeppe | niemeyer: i can work around it with tar xzf /dev/fd/0, but.... | 14:22 |
| niemeyer | rogpeppe: Weird indeed | 14:22 |
| rogpeppe | niemeyer: ah! i've found the problem. | 14:23 |
| niemeyer | rogpeppe: Oh? | 14:23 |
| rogpeppe | niemeyer: i had a bogus version of tar in my path | 14:23 |
| rogpeppe | niemeyer: it's a pity our tests will fail if someone has an odd environment. but i don't want to hard-code /usr/bin/tar ... | 14:25 |
| rogpeppe | pwd | 14:25 |
| niemeyer | rogpeppe: Yeah, this is fine IMO | 14:37 |
| niemeyer | rogpeppe: All kinds of crazy things can happen if you has a buggy env | 14:38 |
| niemeyer | s/you/one | 14:38 |
| niemeyer | rogpeppe: Btw, the battery death reminded me that I started the mgo zk in the trip to Brazil | 14:38 |
| niemeyer | rogpeppe: Looking really nice so far | 14:38 |
| rogpeppe | niemeyer: it's a bit weird really, because one of the original advantages of unix is that you can customise your commands. | 14:38 |
| niemeyer | rogpeppe: Next time I have a moment I'll start on watches | 14:39 |
| rogpeppe | niemeyer: cool. have you proof-of-concept-ed the watchers? | 14:39 |
| rogpeppe | oh, ok | 14:39 |
| niemeyer | rogpeppe: Not yet, but I have a pretty good idea about how to implement them now | 14:39 |
| niemeyer | rogpeppe: I started the research on the trip | 14:39 |
| niemeyer | rogpeppe: It's quite racy, so has to be carefully thought out | 14:39 |
| rogpeppe | niemeyer: yeah. it's a critical component. | 14:40 |
| niemeyer | rogpeppe: I'm planning to use the internal replication mechanism to do it.. I *think* it'll end up quite nice | 14:40 |
| Aram | maybe we should do like the web guys that try to detect browser capability and serve custom pages depending on that, that way we could support user customized environments :P. | 14:40 |
| * Aram hides. | 14:40 | |
| rogpeppe | niemeyer: it'll be interesting to see how the benchmarks (network bandwidth & latency particularly) work out | 14:40 |
| niemeyer | rogpeppe: Quite optimistic on that regard | 14:41 |
| rogpeppe | Aram: ./configure :-) | 14:41 |
| niemeyer | rogpeppe: Don't be silly.. we should start with autoconf.sh | 14:41 |
| rogpeppe | niemeyer: good to hear | 14:41 |
| rogpeppe | niemeyer: of course! | 14:41 |
| rogpeppe | not | 14:42 |
| niemeyer | :-) | 14:42 |
| niemeyer | autoconf.sh => configure => make.. it's crazy how people end up finding that normal | 14:42 |
| rogpeppe | the whole notion of probing a system to find out what kind of thing it is is bogus | 14:42 |
| Aram | yeah, that worked so well for cross compiling :). | 14:43 |
| rogpeppe | tbh, i wouldn't mind setting PATH=/bin:/usr/bin | 14:43 |
| Aram | that's what I do most of the time in my scripts. | 14:43 |
| rogpeppe | Aram: it's a reasonably sane approach. | 14:44 |
| rogpeppe | niemeyer: question: do you think List should return the full path names of the files, or the pathnames with the dir prefix stripped off? | 14:51 |
| rogpeppe | niemeyer: i'm starting to question how much effort i want to put in to make S3 buckets look like directories. | 14:52 |
| rogpeppe | niemeyer: maybe we *should* just use a prefix model | 14:52 |
| niemeyer | rogpeppe: I'm happy with that too | 14:53 |
| rogpeppe | niemeyer: i think i'll go with that for the time being as the path of least resistance. | 14:53 |
| niemeyer | rogpeppe: Full path names sounds simpler | 14:53 |
| rogpeppe | niemeyer: then we don't have to worry about the impedance mismatch. and we can revisit the decision later if we need to. | 14:54 |
| niemeyer | rogpeppe: Yeah | 14:54 |
| niemeyer | rogpeppe: Btw, I'm thinking we'll end up having some kind of manifest file for the generic HTTP public provider | 14:55 |
| niemeyer | rogpeppe: This will also make that simpler | 14:55 |
| rogpeppe | niemeyer: no need to use the Separator stuff either. | 14:55 |
| niemeyer | rogpeppe: Definitely | 14:55 |
| rogpeppe | niemeyer: the manifest file should be auto-generated, otherwise we've got a problem with two clients uploading tools concurrently. | 14:56 |
| niemeyer | rogpeppe: This is about the public storage | 15:00 |
| niemeyer | rogpeppe: For the private storage we should be able to list our stuff | 15:01 |
| niemeyer | rogpeppe: Which will be guaranteed, given we're hopefully moving towards a internal storage mechanism | 15:01 |
| rogpeppe | niemeyer: ok, that seems reasonable. the interface can be the same in both cases (StorageReader) | 15:01 |
| niemeyer | rogpeppe: RIght | 15:02 |
| * rogpeppe quite likes the StorageReader/StorageWriter distinction. | 15:02 | |
| niemeyer | rogpeppe: Btw, I was going to suggest this before my ISP broke yesterday: StorageReadWriter may be spelled as Storage only | 15:02 |
| niemeyer | rogpeppe: StorageReader, StorageWriter, and Storage | 15:02 |
| rogpeppe | niemeyer: that works | 15:02 |
| rogpeppe | +1 | 15:02 |
| niemeyer | Supa' | 15:02 |
| niemeyer | :) | 15:02 |
| niemeyer | Ok, it's lunch time here | 15:04 |
| niemeyer | Back soon.. reviews on the afternoon today | 15:04 |
| rogpeppe | niemeyer: enjoy | 15:09 |
| Aram | niemeyer: why does your battery stats indicate 0 for the cycle count? | 16:01 |
| Aram | s/does/do/ | 16:02 |
| niemeyer | Aram: Probably side effect of death | 16:08 |
| rogpeppe | niemeyer: https://codereview.appspot.com/6145043 | 16:12 |
| rogpeppe | niemeyer: slightly worried that the branch is too big now and i'll need to spend some more hours splitting it up... :-| | 16:13 |
| rogpeppe | niemeyer: i'm going to have to go soon | 16:56 |
| niemeyer | rogpeppe: I'll have a careful look | 16:59 |
| rogpeppe | niemeyer: just replied to https://codereview.appspot.com/6215045/ . it would probably be good to have a chat about this tomorrow. | 17:15 |
| niemeyer | rogpeppe: Sure | 17:16 |
| niemeyer | rogpeppe: There's a leap in your description that I can't follow | 17:17 |
| niemeyer | rogpeppe: "by making Environ.Check operate directly on a map[string]interface{}, we make that possible." | 17:17 |
| niemeyer | rogpeppe: Nothing has to change in the way CheckConfig works to make that possible | 17:17 |
| rogpeppe | niemeyer: how do we merge the attributes generated when marshalling an environment with the other attributes held in the environment settings node? | 17:27 |
| rogpeppe | niemeyer: (i don't have to go immediately, BTW) | 17:27 |
| * rogpeppe is trying to recall all the discussion points that came up with fwereade and dfc at the time. | 17:32 | |
| niemeyer | rogpeppe: Hm | 17:33 |
| niemeyer | rogpeppe: I don't understand hte problem | 17:33 |
| niemeyer | rogpeppe: Check isn't about merging | 17:33 |
| niemeyer | rogpeppe: ConfigCheck is about validation and coercion | 17:33 |
| rogpeppe | niemeyer: Check has to parse the attributes that are created with Marshal (or whatever it would be called) | 17:33 |
| rogpeppe | niemeyer: so it helps if the two things read and write the same structure | 17:34 |
| niemeyer | rogpeppe: That's fine.. there's still no reason to change the function signature | 17:35 |
| rogpeppe | niemeyer: i don't see how it can work. what if Marshal returns a single int? | 17:35 |
| niemeyer | rogpeppe: I still don't get what problem you're trying to solve, sorry.. ConfigCheck returns a schema checker, which checks the schema by validating and coercing to the proper types in case it is valid | 17:36 |
| niemeyer | rogpeppe: What is Marshal? | 17:36 |
| niemeyer | rogpeppe: What problem are we trying to solve? | 17:37 |
| rogpeppe | niemeyer: ok, so... | 17:37 |
| rogpeppe | niemeyer: when we bootstrap, we need to ask the environment to save some of itself to the State. | 17:37 |
| rogpeppe | niemeyer: or at least that needs to happen at some point | 17:37 |
| niemeyer | Yep | 17:37 |
| niemeyer | I'm with you | 17:38 |
| rogpeppe | niemeyer: so the question is: how does that happen? and once we've got the settings inside the State, how do we create an Environ from them? | 17:38 |
| niemeyer | rogpeppe: We create an environment by retrieving the ConfigNode for the environment configuration, and providing the Map() onto a new environment via Open. | 17:39 |
| niemeyer | rogpeppe: Another Option is to provide the ConfigNode itself onto Open, and change Open's signature to take a Getter, for example. | 17:39 |
| niemeyer | rogpeppe: Neither of those options require any changes to ConfigChecker | 17:40 |
| rogpeppe | niemeyer: that doesn't work - Open expects a nicely formatted structure that has been produced through Check. | 17:40 |
| niemeyer | rogpeppe: No, it doesn't.. there's no Check today, and neither of us mentioned anything about it so far | 17:40 |
| rogpeppe | niemeyer: ok, i'm proposing to rename Coerce to Check. | 17:40 |
| niemeyer | rogpeppe: Yes, I can see *that* :-) | 17:40 |
| niemeyer | rogpeppe: But you didn't mention why, and I can't see any reason to | 17:40 |
| rogpeppe | niemeyer: but the point is: all the preconditions are checked there. if we start passing attributes directly to Open, then the checks will have to go there too. | 17:41 |
| niemeyer | rogpeppe: One thing isn't related to the other in any way.. | 17:41 |
| niemeyer | rogpeppe: If you pass a Getter into Open, there's no guarantee it's right either | 17:42 |
| rogpeppe | niemeyer: i wasn't proposing to pass a Getter into Open | 17:42 |
| rogpeppe | niemeyer: i was proposing to pass a Getter into *Check* | 17:42 |
| niemeyer | rogpeppe: Yeah, you're proposing something else, and I'm trying to guess why | 17:42 |
| rogpeppe | niemeyer: then Check can do all the checks it does right now. | 17:42 |
| niemeyer | rogpeppe: Yes, you were proposing that, but you didn't say why, and I can't see any reason to do that in the explanation above | 17:42 |
| niemeyer | rogpeppe: We seem to be a bit stuck on the lack of reasoning around htat | 17:43 |
| rogpeppe | niemeyer: it depends if you think the current division of labour works well or not | 17:43 |
| niemeyer | rogpeppe: It works very well today, and nothing I can see above seems to change that | 17:43 |
| rogpeppe | niemeyer: currently the Coerce method checks that all the fields are there and have sane values | 17:43 |
| rogpeppe | niemeyer: if we start passing unchecked values to Open, then that checking will have to move into Open. | 17:44 |
| niemeyer | rogpeppe: The values we pass today onto Open are checked. Renaming a method doesn't solve anything. | 17:44 |
| rogpeppe | [18:39:22] <niemeyer> rogpeppe: We create an environment by retrieving the ConfigNode for the environment configuration, and providing the Map() onto a new environment via Open. | 17:44 |
| niemeyer | rogpeppe: Yes!? | 17:44 |
| rogpeppe | niemeyer: so in that case, where is the checking done? | 17:45 |
| niemeyer | rogpeppe: I *really* hope we're not saving an invalid configuration onto the state!? | 17:45 |
| rogpeppe | niemeyer: who knows? we need to check it. | 17:45 |
| niemeyer | rogpeppe: No, sorry.. this is a broken design | 17:45 |
| rogpeppe | niemeyer: also, currently Check saves everything into a nice private structure. | 17:46 |
| niemeyer | rogpeppe: If you want to valid the state, it should be on its way in, not on its way out | 17:46 |
| rogpeppe | niemeyer: state is external. | 17:46 |
| rogpeppe | niemeyer: we might have got the version checking wrong | 17:46 |
| rogpeppe | niemeyer: we don't want to do unchecked type conversions. | 17:46 |
| niemeyer | rogpeppe: Besides that, validating the state would mean passing the result of Map() onto exactly the same validation & coercion function | 17:47 |
| niemeyer | rogpeppe: Which goes back onto the point that there's unjustified fuzz around that method | 17:47 |
| rogpeppe | niemeyer: i don't find it fuzzy. the Coerce and Open methods have a close relationship, but it's well defined. | 17:47 |
| niemeyer | rogpeppe: Yep. It's well define, works, and it's all good. You're suggesting we change the method name and signature and introduce an interface, because...? I don't know. | 17:48 |
| rogpeppe | niemeyer: i'm proposing passing the ConfigNode, in some form, to Coerce. | 17:48 |
| niemeyer | rogpeppe: Because...? | 17:48 |
| rogpeppe | niemeyer: so that Coerce can do exactly the same checking that it does when it gets stuff from environments.yaml | 17:49 |
| rogpeppe | niemeyer: with no code needing to be changed. | 17:49 |
| niemeyer | rogpeppe: ConfigCheck can do exactly that, today, with no changes to its signature, and no new interfaces. | 17:49 |
| rogpeppe | niemeyer: ConfigCheck? | 17:51 |
| rogpeppe | niemeyer: ConfigChecker, oh yes | 17:51 |
| rogpeppe | niemeyer: so what happens the other way around? | 17:52 |
| rogpeppe | niemeyer: i.e. how do we do the marshalling? | 17:52 |
| niemeyer | rogpeppe: the other way around we return.. :-) | 17:52 |
| rogpeppe | niemeyer: do we require that Marshal always return a map[string]interface{} ? | 17:53 |
| niemeyer | rogpeppe: env.Config() map[string]interface{} seems easy enough | 17:53 |
| rogpeppe | niemeyer: ok, so if we return map[string]interface{}, can't we guarantee that Coerce is called with that same type? | 17:54 |
| rogpeppe | niemeyer: and reflect that in the type signature | 17:54 |
| niemeyer | rogpeppe: That sounds fine to me.. I believe it's already called with that type today, right? | 17:54 |
| niemeyer | rogpeppe: We just don't enforce it by passing it via an interface{} | 17:54 |
| niemeyer | rogpeppe: But we could, and it sounds sane to do it | 17:55 |
| rogpeppe | niemeyer: to change the type signature? | 17:55 |
| niemeyer | rogpeppe: To be explicit that what we're passing in today is a map.. it's already a map | 17:55 |
| rogpeppe | niemeyer: explicit to the compiler, or just to document it? | 17:56 |
| niemeyer | rogpeppe: Explicit to the compiler.. | 17:56 |
| rogpeppe | niemeyer: ok. so... what we've ended up with there is *almost* the same as what i was proposing. | 17:57 |
| rogpeppe | niemeyer: the only difference being that if we use Getter and Setter we can pass in the ConfigNode directly. | 17:57 |
| niemeyer | rogpeppe: That's my point the whole time.. you're proposing a change that isn't necessary, because what we have already works | 17:57 |
| rogpeppe | niemeyer: fair enough. it saves some code doing to map copying, but we can use map[string]interface{} throughout if we like. | 17:59 |
| rogpeppe | s/doing to/doing the/ | 17:59 |
| niemeyer | rogpeppe: I don't think it saves any code.. the current scheme needs *less* code, actually | 17:59 |
| rogpeppe | niemeyer: (what's being passed to Coerce currently isn't actually map[string]interface{} BTW) | 17:59 |
| niemeyer | rogpeppe: Because we work natively with maps on both ends | 17:59 |
| rogpeppe | niemeyer: really? | 17:59 |
| rogpeppe | niemeyer: one side we've got map[interface{}]interface{}; the other we've got *ConfigNode | 18:00 |
| rogpeppe | (which already has Get and Set methods) | 18:00 |
| niemeyer | rogpeppe: What's below ConfigNode? | 18:00 |
| niemeyer | rogpeppe: What does ConfigNode.Map return? | 18:01 |
| niemeyer | rogpeppe: It's a map on both ends, really | 18:01 |
| niemeyer | rogpeppe: The fact it's a map[interface{}] is also irrelevant.. we'll never have non-strings on that first level | 18:01 |
| niemeyer | 1: WOAH!? | 18:02 |
| rogpeppe | niemeyer: it's not type-compatible with map[string], so we'll have to copy it to pass it in | 18:02 |
| niemeyer | :) | 18:02 |
| niemeyer | rogpeppe: Why can't we just have map[string]? | 18:02 |
| rogpeppe | niemeyer: because that's not what goyaml.Unmarshal returns.... or maybe we could make it... | 18:02 |
| niemeyer | rogpeppe: Coerce.. | 18:02 |
| * rogpeppe has a look | 18:02 | |
| rogpeppe | niemeyer: maybe ReadEnvironsBytes could unmarshal into map[string]map[string]interface{} | 18:04 |
| niemeyer | rogpeppe: Coerce is what has to manage it | 18:04 |
| rogpeppe | niemeyer: not necessarily, i think | 18:04 |
| * niemeyer waits for the light bulb to show up | 18:05 | |
| rogpeppe | niemeyer: the problem currently is that ReadEnvironsBytes produces a map[interface{}]interface{} which is then passed to Coerce | 18:05 |
| rogpeppe | niemeyer: but i *think* we can unmarshal into map[string]interface{} at the top level and save the Coerce checking | 18:06 |
| rogpeppe | niemeyer: (of course Coerce would still need to check other things, but we could make its signature map[string] and avoid the map copying. | 18:06 |
| rogpeppe | ) | 18:06 |
| niemeyer | rogpeppe: We can't.. | 18:07 |
| rogpeppe | niemeyer: oh. no? | 18:07 |
| niemeyer | rogpeppe: Coerce validates the values, and transforms them onto expected types | 18:07 |
| rogpeppe | niemeyer: go on | 18:07 |
| niemeyer | That's it.. | 18:07 |
| rogpeppe | niemeyer: what if the signature of Coerce was Coerce(map[string]interface{}) (interface{}, error) ? | 18:08 |
| niemeyer | rogpeppe: It would still not help.. coerce coerces | 18:08 |
| rogpeppe | niemeyer: i.e. that Coerce took the exact output of Marshal as input | 18:08 |
| rogpeppe | niemeyer: that's fine. | 18:08 |
| niemeyer | Ok then :) | 18:08 |
| niemeyer | I'll stop arguing and let you read the code | 18:08 |
| rogpeppe | niemeyer: ok, so i'm ok with using map[string]interface{} throughout rather than Getter/Setter, i think. | 18:09 |
| niemeyer | rogpeppe: Sounds great, thanks | 18:10 |
| rogpeppe | niemeyer: hmm, might not work very well with the schema package, which uses map[interface{}]interface{} throughout. | 18:12 |
| niemeyer | I see a light bulb starting to show up :-) | 18:12 |
| rogpeppe | niemeyer: i'm not convinced we gain much from using the schema package tbh | 18:14 |
| niemeyer | rogpeppe: Let's not throw the baby out with the bath please.. we have to walk forward | 18:14 |
| rogpeppe | niemeyer: it's higher-order code, but just a few type checks would be equivalent. | 18:14 |
| niemeyer | rogpeppe: There's zero reason to do by hand what the schema package does elegantly | 18:15 |
| niemeyer | rogpeppe: It's ready, works | 18:15 |
| niemeyer | rogpeppe: I believe FieldMap can return a map[string]interface{}, though | 18:15 |
| rogpeppe | niemeyer: nope. | 18:15 |
| niemeyer | rogpeppe: FieldMap is supposed to work with strings already.. we can enforce it in its return value I believe | 18:15 |
| rogpeppe | niemeyer: hence my v.(schema.MapType) conversion | 18:15 |
| niemeyer | rogpeppe: Don't know what you mean | 18:16 |
| rogpeppe | niemeyer: in fieldMapC.Coerce: out := make(MapType, l) | 18:16 |
| niemeyer | rogpeppe: can != does today | 18:17 |
| rogpeppe | niemeyer: that would make for a very different schema package. (one that i'd like to see, i agree, but not the one we have today, which outputs a small set of known types, rather than coercing to a specified type) | 18:17 |
| niemeyer | rogpeppe: Do you want me to code on that *very* different schema package? | 18:18 |
| niemeyer | rogpeppe: I bet the patch is less than 10 lines long | 18:18 |
| rogpeppe | niemeyer: all we're using schema for currently is the equivalent of: x, ok := m["field"].(string) | 18:18 |
| niemeyer | rogpeppe: and consists of removals | 18:18 |
| niemeyer | and a couple of replacements | 18:18 |
| rogpeppe | niemeyer: really? | 18:19 |
| * niemeyer writes it | 18:19 | |
| rogpeppe | niemeyer: but still, we've got all this higher-order machinery, but we're not using any of it in any particularly useful way. | 18:20 |
| niemeyer | rogpeppe: We are, it works, today, let's *please* move on! | 18:20 |
| niemeyer | rogpeppe: I really don't want to be moving backwards and doing checks by hand when we have that nice mechanism in place already | 18:20 |
| rogpeppe | niemeyer: it's because it was making it awkward that i was considering avoiding its use. changing its use is about 10 minutes work. | 18:21 |
| niemeyer | rogpeppe: I don't want to be checking whether it's int32 or int64 | 18:21 |
| niemeyer | rogpeppe: Or whether it's something else entirely | 18:21 |
| rogpeppe | niemeyer: will we actually have any int-valued keys? | 18:21 |
| rogpeppe | values i mean | 18:21 |
| niemeyer | rogpeppe: No, it's impossible.. | 18:22 |
| niemeyer | :-/ | 18:22 |
| rogpeppe | niemeyer: i mean, are there any int values that we actually *want*? | 18:22 |
| rogpeppe | niemeyer: because that's the only time that int32 vs int64 makes a difference. | 18:23 |
| niemeyer | rogpeppe: Please try to ponder about your question for a moment | 18:23 |
| niemeyer | rogpeppe: This is getting non-productive | 18:23 |
| rogpeppe | niemeyer: this is the kind of thing i was thinking of. i think it compares favourably to the current code. it actually reduces the overall line count (by 30 lines) and easier to understand IMHO, because you don't need to understand how schema works to follow the logic. i'll leave things as they are, but i think ewe should at least consider this possibility. | 19:02 |
| rogpeppe | http://paste.ubuntu.com/991155/ | 19:02 |
| rogpeppe | all tests pass BTW | 19:03 |
| rogpeppe | s/ewe/we/ | 19:03 |
| rogpeppe | right, i really should go now. | 19:05 |
| niemeyer | rogpeppe: How does it reduce the line count? Seems to have the same number of lines | 19:05 |
| rogpeppe | niemeyer: sorry for the distraction | 19:05 |
| rogpeppe | niemeyer: the line count goes down in other places (the schema helpers in ec2/util.go for example) | 19:05 |
| niemeyer | rogpeppe: The errors are bogus, though | 19:07 |
| rogpeppe | niemeyer: it could be more compact with a little table | 19:07 |
| niemeyer | rogpeppe: That's one important benefit from the schema package | 19:07 |
| rogpeppe | niemeyer: as always, i think it's a trade-off. the higher order stuff is worth it when it really makes things easier to understand and reduces the line count by lots. | 19:08 |
| rogpeppe | niemeyer: in this case, i'm not convinced of its worth. | 19:08 |
| niemeyer | rogpeppe: It think it's worth based on these factors | 19:08 |
| rogpeppe | s/worth/worse/ ? | 19:08 |
| niemeyer | rogpeppe: Yeah | 19:09 |
| niemeyer | rogpeppe: Error checking, for example | 19:09 |
| rogpeppe | niemeyer: all the error checking in schema is implicit - to understand the logic in config.go you have to understand the schema package (which isn't trivial) | 19:10 |
| niemeyer | rogpeppe: https://codereview.appspot.com/6212054 | 19:10 |
| niemeyer | rogpeppe: Implicit? | 19:10 |
| rogpeppe | niemeyer: yes, it's not obvious what inputs will result in what errors. (or even what code is calling what, due to the indirect nature of the schema.Checker) | 19:11 |
| niemeyer | rogpeppe: Sorry, I don't get it.. bad input will result in errors | 19:11 |
| niemeyer | rogpeppe: bad types, missing fields | 19:11 |
| niemeyer | rogpeppe: There's nothing implicit about that | 19:11 |
| rogpeppe | niemeyer: yes, and all that logic is defined within schema. | 19:11 |
| niemeyer | rogpeppe: Yep! | 19:12 |
| rogpeppe | niemeyer: which is quite complex. | 19:12 |
| niemeyer | rogpeppe: and it seems to work | 19:12 |
| niemeyer | rogpeppe: Not really.. | 19:12 |
| rogpeppe | niemeyer: i'm saying we can cut out all that complexity and still reduce line count. | 19:12 |
| niemeyer | rogpeppe: It's used in ec2, in charm config, in charm meta, ... | 19:12 |
| rogpeppe | niemeyer: ec2 is what i was suggesting losing it in... | 19:13 |
| niemeyer | rogpeppe: Yes, and I'm pointing out you're not getting rid of schema | 19:13 |
| niemeyer | Actually, that combined checker is excessively complex for schema.. | 19:14 |
| niemeyer | There's a lot of stuff being done by hand there.. | 19:15 |
| niemeyer | c.authorizedKeys = maybeString(m["authorized-keys"], "") | 19:15 |
| niemeyer | !? | 19:15 |
| rogpeppe | niemeyer: that's necessary, because that key may not exist | 19:16 |
| rogpeppe | well, i suppose i could do: c.authorizedKeys, _ = m["..."] | 19:16 |
| niemeyer | rogpeppe: And how's this any simpler: | 19:17 |
| niemeyer | c.authorizedKeysPath, err = configString(false, m, "authorized-keys-path", "") | 19:17 |
| niemeyer | ? | 19:17 |
| rogpeppe | niemeyer: because all the logic is right there. | 19:17 |
| niemeyer | rogpeppe: Yeah, because you're reimplementing the schema package right there.. sorry, I don't see the point | 19:18 |
| niemeyer | http://paste.ubuntu.com/991192/ | 19:19 |
| rogpeppe | niemeyer: yeah, i agree there are arguments both ways. i think currently that making the Environs interface simpler would be a good thing (and it can still use schema inside if it wants), but i can see arguments for keeping it as it is too. | 19:20 |
| rogpeppe | niemeyer: i really have to go :-) | 19:21 |
| niemeyer | rogpeppe: Once we have some spare time, we can even make schema unmarshal directly onto the struct | 19:21 |
| niemeyer | rogpeppe: That was already my plan, but I never got to it | 19:21 |
| rogpeppe | niemeyer: +1 | 19:21 |
| rogpeppe | niemeyer: now *that* will be properly useful :-) | 19:21 |
| niemeyer | rogpeppe: yeah.. | 19:22 |
| rogpeppe | (for all kinds of things) | 19:22 |
| niemeyer | rogpeppe: It's already on the way to that | 19:22 |
| rogpeppe | anyway, i have an idea. | 19:22 |
| rogpeppe | will propose something tomorrow | 19:23 |
| rogpeppe | if you manage to look at https://codereview.appspot.com/6145043/ that would be scrumptiolicious. :-) | 19:24 |
| rogpeppe | see you tomorrow, have a good rest-of-day. | 19:24 |
| rogpeppe | niemeyer: ^ | 19:24 |
| niemeyer | rogpeppe: Thanks, have a good one too | 19:24 |
| niemeyer | Time for an afternoon food break... biab | 20:28 |
| === flaviamissi_ is now known as flaviamissi | ||
| niemeyer | Phew.. long branch.. | 22:14 |
| * niemeyer steps out.. back later for another round | 22:15 | |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!