| davecheney | niemeyer: howdy | 00:08 |
|---|---|---|
| davecheney | sorry i'm a bit slow off the mark today | 00:08 |
| davecheney | m_3 and I are checking out a coworking space in the city today | 00:08 |
| davecheney | niemeyer: short status report, got ec2 local tests working with a provisoiner working last night | 00:09 |
| davecheney | but there are a few problems that need to be solved before I can propose it | 00:09 |
| niemeyer | davecheney: Heya | 00:09 |
| niemeyer | davecheney: Neat, sounds good | 00:09 |
| davecheney | niemeyer: the main one is we ask ec2test for the DNS name of machine/0 | 00:09 |
| davecheney | whcih returns something liek i-3.example.com | 00:09 |
| davecheney | which breaks the local test because it then uses that hostname in the stateinfo passed to anyone who wants it | 00:10 |
| davecheney | so I need to add some kind of hook to either ec2test to say 'always return localhost' as the dns name, or to be able to override it if the test is running in local mode | 00:11 |
| davecheney | niemeyer: the second problem is ec2 is hard coded to expect the zkport to be 2181 | 00:11 |
| davecheney | but that is simpler to fix | 00:11 |
| davecheney | i'll propose a fix real soon now | 00:11 |
| niemeyer | davecheney: Hmm | 00:12 |
| niemeyer | davecheney: Wait, you mean you're putting a worker within the ec2 tests? | 00:12 |
| davecheney | niemeyer: by worker, do you mean provisioner ? | 00:13 |
| niemeyer | davecheney: a juju-core/worker | 00:13 |
| davecheney | niemeyer: yes, the tests that roger left bootstrap an environment then try to do a state.AddMachine | 00:14 |
| niemeyer | davecheney: Within the *ec2* tests? | 00:15 |
| niemeyer | davecheney: I may be wrong, and will be happy to look at it, but it doesn't sound entirely right | 00:15 |
| davecheney | niemeyer: yes, roger extended jujutest/livetest to start a machine in the provisioned environment | 00:15 |
| davecheney | hang on, will get a link | 00:15 |
| davecheney | niemeyer: sorry, m_3 just turned up | 00:21 |
| davecheney | https://codereview.appspot.com/6347044/ << not a proposal | 00:21 |
| niemeyer | davecheney: No worries, I'll actually have to step out for dinner | 00:21 |
| davecheney | this is a grab bag of rogers stuff and my additions | 00:21 |
| davecheney | niemeyer: the key change is https://codereview.appspot.com/6347044/patch/10001/11012 | 00:22 |
| davecheney | specifically, if hasProvisioner then testprovisioner | 00:22 |
| niemeyer | davecheney: I'm indeed a bit lost.. in a sentence or two, what is this testing? | 00:58 |
| davecheney | niemeyer: the changes to jujutest test that after bootstrapping, a provisioner is running, and it can start an instance with state.AddMachine | 01:05 |
| niemeyer | davecheney: I can't see how we could possibly be testing that without a functional test | 01:06 |
| niemeyer | davecheney: How can we be testing that after bootstrap a provisioner is running? This is entirely dependent on details of ec2 itself | 01:06 |
| davecheney | yes, i haven't got that to work in the ec2 amazon tests yet because of the push secrets problem | 01:08 |
| davecheney | this only works when we make the local provisioning agent in process for local_test.go | 01:08 |
| davecheney | but in theory, once the secrets are pushed (faux juju deploy) the test will also pass on the real ec2 | 01:08 |
| niemeyer | davecheney: The point is that there's nothing being tested there | 01:56 |
| niemeyer | davecheney: The live real tests can do that on real EC2 | 01:57 |
| niemeyer | davecheney: If we fake the precise point we want to test, what's the purpose of the test? | 01:57 |
| davecheney | niemeyer: this test should (ha!) work on the real ec2 as well | 01:58 |
| davecheney | so at the moment all I can prove is the test case works when we fake out all the moving parts behind it | 01:59 |
| davecheney | that is, localtest continues to work with a test plan that expects a provisioner | 02:00 |
| davecheney | once the pushing of secrets is fixed, we can remove the 'hasprovisioner' field as it will always be true | 02:00 |
| niemeyer | davecheney: I understand, but this doesn't seem to be going in a good direction | 02:02 |
| niemeyer | davecheney: All this faking isn't buying us stability or anything | 02:02 |
| niemeyer | davecheney: It's messing up the code instead | 02:02 |
| davecheney | niemeyer: should I put this on hold until we know how to fix the pushing of secrets and can do a real test in a real ec2 ? | 02:03 |
| niemeyer | davecheney: No, I think we should fix the problems we have to fix for getting the real EC2 running | 02:03 |
| davecheney | niemeyer: cool, that means resolving the secrets problem | 02:04 |
| niemeyer | davecheney: The secrets problem is resolved in deploy | 02:04 |
| niemeyer | davecheney: There's no *real* problem.. we just need to do the same thing we used to do | 02:04 |
| niemeyer | davecheney: Sorry, I'm kind of stating that with the belief I know what's going on, and maybe I'm wrong | 02:05 |
| niemeyer | davecheney: So please don't take that at face value.. feel free to bring other points | 02:05 |
| davecheney | niemeyer: thats cool | 02:05 |
| davecheney | the problem i think I'm solving is | 02:05 |
| davecheney | 1. get boostrapping working -- which I believe is done, juju bootstrap works | 02:06 |
| davecheney | 2. prove it with tests | 02:06 |
| davecheney | 2 is tricky, and this CL is part way towads solving it | 02:06 |
| niemeyer | davecheney: There are also several bits in the same CL that should be broken down so we can debate about them (and integrate them!) in isolation | 02:06 |
| niemeyer | davecheney: e.g. there's a fix for cloudinit, etc | 02:07 |
| davecheney | niemeyer: yes, i'll peel off those bits | 02:07 |
| davecheney | just at the moment i'm taking another pass at the s3 location constraint proposal | 02:07 |
| davecheney | because I want to commit that before telling everyone to please go get -u | 02:08 |
| davecheney | then i'll start to pull apart this CL | 02:08 |
| niemeyer | I find this, for instance, unexpected: | 02:08 |
| niemeyer | 87 err = t.seedSecrets(env) | 02:08 |
| niemeyer | 88 c.Assert(err, IsNil) | 02:08 |
| niemeyer | 89 if t.HasProvisioner { | 02:08 |
| niemeyer | 90 t.testProvisioning(c, st) | 02:08 |
| niemeyer | 91 } | 02:08 |
| niemeyer | davecheney: This is in TestBootstrap | 02:10 |
| davecheney | yes, all environments should have a provisioner, but we can't do that yet | 02:10 |
| davecheney | sorry, all providers | 02:10 |
| niemeyer | davecheney: So why would we be seeding secrets if we're testing bootstrap? | 02:11 |
| davecheney | niemeyer: right, i understand | 02:11 |
| davecheney | I will make a new test case | 02:11 |
| niemeyer | davecheney: In fact, not only seeding secrets, but hacking the environment configuration arbitrarily | 02:11 |
| davecheney | niemeyer: agreed, this is to work around the fact that bootstrap doesn't push the complete environment configuration into the state | 02:12 |
| davecheney | if it did, then this wouldn't be a problem | 02:12 |
| niemeyer | davecheney: But this is TestBootstrap! :) | 02:12 |
| niemeyer | davecheney: It does not, and it should not | 02:13 |
| davecheney | understood, i'll rework it | 02:14 |
| niemeyer | davecheney: If the idea is to test a real ec2 deployment on live tests, we should call fwereade_'s conn.Deploy | 02:14 |
| niemeyer | davecheney: and let it do the work | 02:14 |
| davecheney | niemeyer: i'll try that | 02:15 |
| niemeyer | davecheney: There's another issue with the CL approach as well that is good to keep in mind | 02:17 |
| niemeyer | davecheney: One of the goals of jujutest is for it to be reusable across providers | 02:17 |
| davecheney | yes | 02:17 |
| niemeyer | davecheney: So we have to watch out for provider-specific behavior there.. in some cases we may need to hook onto the provider | 02:18 |
| niemeyer | davecheney: Via a callback or whatever | 02:18 |
| niemeyer | davecheney: But ideally we should be able to take these tests, plug on the OpenStack or Compute Engine backend, and have them working | 02:18 |
| davecheney | niemeyer: yes | 02:19 |
| davecheney | just checking what abuse i've done to jujutest | 02:19 |
| niemeyer | davecheney: The configuration is the only thing that stands out to me | 02:20 |
| davecheney | niemeyer: the complication is the ec2 environ has two modes | 02:20 |
| davecheney | local fake ec2 mode and amazon mode | 02:20 |
| davecheney | supporting that is hard | 02:21 |
| davecheney | for example, fake ec2test server gives back DNS names which are impossible | 02:21 |
| davecheney | ie, i-3.example.com | 02:21 |
| davecheney | which can't be used as value StateInfo addresses | 02:21 |
| niemeyer | davecheney: Right, but the question is where do we draw the line | 02:22 |
| niemeyer | davecheney: I don't think it'd pay off if we simulated the *whole* provider | 02:23 |
| niemeyer | davecheney: How do we actually deploy a machine? | 02:23 |
| niemeyer | davecheney: How do we get unit agents running and interacting? | 02:23 |
| niemeyer | davecheney: Faking out all of that stuff would have us with white hair by then | 02:23 |
| davecheney | and as you say, doesn't actually test anything | 02:24 |
| niemeyer | davecheney: It could test, the interaction between the pieces, but indeed not the *deployment* of the pieces, since that's all faked indeed | 02:24 |
| niemeyer | Meaning it could be entirely broken and we'd think it's alright | 02:24 |
| niemeyer | davecheney: So, to begin with, I'd be happy with | 02:25 |
| davecheney | it's really about continuing the illusion of the ec2 provider in local mode | 02:25 |
| niemeyer | if !t.CanOpenState { return } | 02:25 |
| niemeyer | davecheney: At the top of the test that checks the real provisioning agent with conn.Deploy | 02:25 |
| niemeyer | davecheney: run that with real Amazon only | 02:25 |
| niemeyer | davecheney: This means we're testing stuff *for real*, cloud-init, running of commands, upstart script, and Inc. | 02:26 |
| davecheney | niemeyer: ok, i'll try that | 02:26 |
| niemeyer | davecheney: Of course, we won't be able to run that all the time | 02:26 |
| niemeyer | davecheney: But for that we have the unit tests, closer to the implementations themselves | 02:26 |
| niemeyer | davecheney: As soon as we have stuff working like that, I'll be happy to setup an integration test somewhere that does the boring work for us | 02:27 |
| niemeyer | davecheney: I mean, integration test runner | 02:27 |
| niemeyer | davecheney: LiveTest is already an integration test | 02:28 |
| niemeyer | LiveSuite, that is | 02:28 |
| davecheney | niemeyer: thank you for your time this evening | 02:30 |
| niemeyer | davecheney: Sure thing, I'm glad to have that stuff moving forward.. we're about to have something real working, and that's quite exciting | 02:31 |
| davecheney | i'm going to take a break on this CL and work on the s3 location one for a little | 02:31 |
| davecheney | niemeyer: it's been working for over a week | 02:31 |
| davecheney | actually, no | 02:31 |
| davecheney | thta is not true | 02:31 |
| davecheney | once we have deploy it works | 02:31 |
| davecheney | also, ... value *net.OpError = &net.OpError{Op:"remote error", Net:"", Addr:net.Addr(nil), Err:0x28} ("remote error: handshake failure") | 02:32 |
| davecheney | fu amazon | 02:32 |
| niemeyer | Yeah, Amazon has been going through some bad times | 02:33 |
| davecheney | niemeyer: changing gears | 02:34 |
| davecheney | I'd like to change the s3 tests to hit a number of regions | 02:34 |
| davecheney | not just USEast | 02:34 |
| davecheney | is there anyting in juju-core that I can crib for reusing a test ? | 02:34 |
| niemeyer | davecheney: Should be easy to have a setting on the suite type on s3i_test.go, and instantiating a couple of them with different values at the Suite(&S{..}) call time | 02:35 |
| davecheney | I'm thinking jujutest/tests.go | 02:35 |
| davecheney | niemeyer: yes, that is what I want to do | 02:35 |
| niemeyer | davecheney: But if it's s3, s3i_test.go is the thing | 02:35 |
| niemeyer | TheMue, fwereade: Mornings! | 10:04 |
| TheMue | niemeyer: Morning | 10:05 |
| TheMue | niemeyer: You're up early today. | 10:05 |
| niemeyer | TheMue: Yeah, couldn't sleep for whatever reason | 10:05 |
| TheMue | niemeyer: So used the time for a relaxed start with a good breakfast | 10:07 |
| niemeyer | TheMue: Yeah, exactly | 10:08 |
| fwereade | niemeyer, heyhey | 10:24 |
| TheMue | fwereade: Heya | 10:27 |
| niemeyer | fwereade, TheMue: How're things going guys? Good progress there? | 10:40 |
| TheMue | niemeyer: Think so, yes. Setting up my tests. | 10:40 |
| fwereade | niemeyer, not too bad, getting default-series handling right is a touch fiddly | 10:40 |
| fwereade | niemeyer, had been hoping to chat to dave this morning, but missed him | 10:41 |
| niemeyer | fwereade: Aw | 10:42 |
| niemeyer | fwereade: What's the trickiness around it? | 10:42 |
| fwereade | niemeyer, at the heart of it is now we initialize state | 10:43 |
| fwereade | niemeyer, how | 10:43 |
| fwereade | niemeyer, the current Initialize does basically nothing | 10:44 |
| fwereade | niemeyer, what I would like to do is (among the other stuff) to put in a complete environ config, excluding only secrets, at bootstrap time | 10:44 |
| niemeyer | fwereade: That sounds great | 10:45 |
| fwereade | niemeyer, but that has distressingly far-reaching consequences | 10:45 |
| niemeyer | fwereade: Oh? | 10:45 |
| fwereade | niemeyer, I'm hoping that I will be able to see a way to break this down into 2 changes with hindsight | 10:45 |
| fwereade | niemeyer, well, the whole bootstrap pipeline needs to change to accept that complete environ config | 10:46 |
| niemeyer | fwereade: How so? | 10:46 |
| niemeyer | fwereade: Bootstrap() is in the environ, and the environ knows its own config | 10:46 |
| fwereade | niemeyer, cloud-init and initzk | 10:47 |
| niemeyer | fwereade: So on that side at least, we're good | 10:47 |
| niemeyer | fwereade: Right, on the other side we have to load it | 10:47 |
| niemeyer | fwereade: A base64 option to initzk should do the deal | 10:47 |
| fwereade | niemeyer, yeah, that's basically done | 10:48 |
| fwereade | niemeyer, also, we need to be able to figure out which bits are secret and which aren't | 10:48 |
| fwereade | niemeyer, and ATM it feels like we can't sanely get away without some sort of distinction between a loosely-specified user config and a strict real config... but we'll see how we go there | 10:49 |
| niemeyer | fwereade: The only thing that knows about that is the environ | 10:50 |
| fwereade | niemeyer, (I feel it is a profoundly bad thing to have the remote components inferring anything at all about their config, and they should not even accept junk like authorized-keys-path) | 10:50 |
| niemeyer | fwereade: Handly enough, when sending the environ itself can filter out secrets | 10:50 |
| niemeyer | fwereade: given that this is an implementation detail behind Bootstrap | 10:50 |
| fwereade | niemeyer, sure; but I think Conn will also need to get the secrets so it update state on first access | 10:51 |
| fwereade | niemeyer, none of these things are intrinsically painful, but I have yet to see how to turn it into a very clean set of CLs | 10:52 |
| niemeyer | fwereade: I don't think it needs to know those details | 10:52 |
| niemeyer | fwereade: (so far) | 10:52 |
| niemeyer | fwereade: It just has to send the config it has | 10:52 |
| niemeyer | fwereade: So we can make the operation be "send initial config if never done before" rather than "send secrets" | 10:52 |
| fwereade | niemeyer, hmmmmm, so maybe we shouldn't set anything in /environment on bootstrap at all? | 10:54 |
| fwereade | niemeyer, I would be a lot more comfortable if an initialized state implied valid data, even in the /environment node, eschewing only secrets | 10:54 |
| niemeyer | fwereade: Your idea of sending an initial configuration when we have none sounds good, but it doesn't solve the entire problem, so we can postpone it for the moment I guess | 10:55 |
| niemeyer | fwereade: These two options do not conflict with each other, though | 10:55 |
| niemeyer | fwereade: "Bootstrap with config-secrets" & "send initial config if never done before" can be friends :) | 10:56 |
| niemeyer | Sorry | 10:56 |
| niemeyer | fwereade: "Bootstrap with config <minus> secrets" & "send initial config if never done before" can be friends :) | 10:56 |
| fwereade | niemeyer, consider environment-specific settings that should not change, such as the control bucket | 10:56 |
| fwereade | niemeyer, hm, we'll be broken anyway if that changes, won;t be able to find the bootstrap machine anyway | 10:57 |
| * fwereade ponders | 10:57 | |
| fwereade | niemeyer, I guess it just feels kinda icky to ever *overwrite* env settings without checking them | 10:58 |
| fwereade | niemeyer, it feels smarter to send public settings OAOO, and then to update them with secrets alone OAOO | 10:59 |
| niemeyer | fwereade: Okay, I see your point, agreed | 11:00 |
| niemeyer | fwereade: This can be solved in a somewhat clean way, I suppose, if we introduce ConfigSecrets in the Environ interface, returning []string | 11:01 |
| fwereade | niemeyer, anyway, it is progressing nicely enough, and I'll know better how to split it all up cleanly before too long | 11:01 |
| fwereade | niemeyer, yeah, something like that | 11:01 |
| niemeyer | fwereade: I'd be happy with that | 11:02 |
| TheMue | Lunchtime | 11:13 |
| niemeyer | TheMue: Enjoy! | 11:14 |
| Aram | hey all. | 11:24 |
| niemeyer | Aram: Morning | 11:25 |
| fwereade | Aram, heyhey | 11:31 |
| fwereade | niemeyer, I just had a thought... what would happen if we made State.EnvironConfig() return an environs.EnvironConfig? allthe consequences I can currently see are positive ones | 11:33 |
| fwereade | niemeyer, we wouldn't be able to casually update it as we currently do, thanks to it currently being a ConfigNode | 11:34 |
| niemeyer | fwereade: How do we Write it? | 11:34 |
| niemeyer | fwereade: Stat.SetEnvironConfig? | 11:35 |
| niemeyer | State | 11:35 |
| fwereade | niemeyer, I had been thinking of a distinct State.UpdateConfigSettings(map[string]interface{}) | 11:35 |
| fwereade | sorry UpdateEnvironConfig | 11:35 |
| niemeyer | fwereade: Does it replace or does it append? | 11:35 |
| niemeyer | fwereade: I suppose it replaces, so Set would be better.. anyway, +1 | 11:36 |
| fwereade | niemeyer, cool, thanks | 11:36 |
| fwereade | niemeyer, I had originally been thinking of appending, but that's by the by | 11:36 |
| niemeyer | fwereade: That'd mean we'd need an extra interface for removing keys, and it'd be different from the interface we have on Environ for the same purpose | 12:13 |
| fwereade | niemeyer, true | 12:14 |
| fwereade | niemeyer, not sure when we actually want to do that though | 12:14 |
| niemeyer | fwereade: Hmm.. all the time? | 12:15 |
| niemeyer | fwereade: I mean.. what other way would a config be updated? | 12:15 |
| niemeyer | fwereade: Maybe you have something else in mind that I've missed | 12:15 |
| fwereade | niemeyer, I mean I don't know when we delete keys from an environ config | 12:15 |
| fwereade | niemeyer, I guess I'm missing something? | 12:16 |
| niemeyer | fwereade: We surely are able to have missing keys, right? | 12:16 |
| niemeyer | fwereade: Maybe we shouldn't allow that, or should say that missing keys and a key set to "" are the same? | 12:16 |
| niemeyer | fwereade: Either way, the effect is the same.. Set would replace | 12:17 |
| fwereade | niemeyer, IMO we shouldn't really ever have an /environment with missing keys (that aren't secrets) | 12:17 |
| fwereade | niemeyer, we should do all our fuzzy guessing at defaults OAOO, when we load the config | 12:18 |
| niemeyer | fwereade: I don't think that's the case today, and it doesn't feel sane to force the user to have all possible keys, even those not wanted, set | 12:18 |
| fwereade | niemeyer, having missing keys in the provider config has always been an irritation in the python | 12:18 |
| niemeyer | fwereade: In which sense? | 12:19 |
| fwereade | niemeyer, self.config.get("region", DEFAULT_REGION) all over the place, occasional self.config["region"]s that blow up at unexpected times | 12:19 |
| fwereade | niemeyer, it's just ugly | 12:19 |
| fwereade | niemeyer, and I don't see why we shouldn't construct complete configs with real values as soon as they enter the system | 12:20 |
| niemeyer | fwereade: Hmm | 12:22 |
| niemeyer | fwereade: This is bogus indeed | 12:22 |
| fwereade | niemeyer, yeah... I was somewhat upset when I saw we'd copied those flaws over | 12:23 |
| niemeyer | fwereade: The point, though, is that some values may be actually missing. You seem to be implying what I said above regarding having unset keys behaving as "", though | 12:23 |
| niemeyer | fwereade: Not all values have defaults | 12:23 |
| niemeyer | fwereade: In some, the "unset" value is signal | 12:24 |
| fwereade | niemeyer, hmm, yes; I don't see any of those in go atm though | 12:24 |
| niemeyer | fwereade: That's not relevant.. the system is far from complete. We're talking about what we want to happen in those cases. | 12:24 |
| niemeyer | fwereade: We also can't default to "", because our config is typed | 12:25 |
| fwereade | niemeyer, fair point, ok, but consider a setting that *does* have a significant effect, like ec2-uri | 12:26 |
| niemeyer | fwereade: Yep, good example | 12:27 |
| niemeyer | fwereade: What's your feeling about it? | 12:27 |
| fwereade | niemeyer, that an unset ec2-uri is a really bad signal for "not really EC2, modify behaviour appropriately" :) | 12:28 |
| fwereade | niemeyer, it's all very well in a user config | 12:28 |
| fwereade | niemeyer, but internally I would far rather have "ec2-uri" always containing a real value, and another "this-is-not-really-ec2" value that controls the different behaviour | 12:28 |
| niemeyer | fwereade: I'd rather not, to be honest | 12:29 |
| niemeyer | fwereade: Or maybe I do.. hmm | 12:29 |
| niemeyer | fwereade: digesting, just a sec | 12:29 |
| fwereade | niemeyer, it does feel like squishing far too much subtle meaning into a single value | 12:29 |
| niemeyer | fwereade: I don't understand that.. it's the endpoint we're talking to.. how's that subtle? | 12:30 |
| fwereade | niemeyer, that bit is fine... but let me find a code ref | 12:31 |
| fwereade | niemeyer, ok, its impact is limited, because I got annoyed and did this: | 12:32 |
| fwereade | @property | 12:32 |
| fwereade | def using_amazon(self): | 12:32 |
| fwereade | return "ec2-uri" not in self.config | 12:32 |
| fwereade | niemeyer, and even that is not heavily used | 12:32 |
| niemeyer | fwereade: This is wrong, actually | 12:33 |
| niemeyer | fwereade: Because it may be set to an endpoint in Amazon | 12:33 |
| fwereade | niemeyer, it's wrong if someone sets ec2-uri explicitly | 12:33 |
| niemeyer | fwereade: I don't understand this particular issue | 12:33 |
| niemeyer | fwereade: Why!? | 12:33 |
| niemeyer | fwereade: It certainly wasn't the intention when I put it there | 12:33 |
| fwereade | niemeyer, that is what AFAIK it has always been used for | 12:33 |
| fwereade | niemeyer, I always considered it somewhat nasty | 12:34 |
| niemeyer | fwereade: The goal of this setting is to send the endpoint manually, as its name indicates | 12:34 |
| niemeyer | fwereade: I don't get the leap on how it can be something else than that | 12:34 |
| fwereade | niemeyer, so, yes, it would be *even* better if we didn't have this ugly and frequently-incorrect overloading of meaning | 12:34 |
| fwereade | niemeyer, I would guess that someone doing non-ec2 ec2 provider work made an unhelpful assumption | 12:35 |
| fwereade | niemeyer, I stipulate that a this-is-not-amazon setting would be superior in every way | 12:35 |
| niemeyer | fwereade: Yeah, that sounds like the heart of the issue we've been brainstorming on | 12:35 |
| niemeyer | fwereade: If we need such a setting, +1 | 12:36 |
| fwereade | niemeyer, but then, assuming we fix that | 12:36 |
| niemeyer | fwereade: Do we need such a setting? | 12:36 |
| fwereade | niemeyer, possibly, and possibly not, I would have to look through a bit more carefully | 12:36 |
| fwereade | niemeyer, but, ok, consider a correct ec2-uri setting | 12:37 |
| niemeyer | fwereade: Okay | 12:38 |
| fwereade | niemeyer, how do we gain by leaving that as "", when we could just as easily infer the real value *only* when parsing user configs, and ensure that the contents of /environment are always correct and need no further substitutions? | 12:39 |
| niemeyer | fwereade: Because when a user questions for the configuration of their environment, I'd rather tell them what they've set | 12:39 |
| niemeyer | fwereade: We also lose the ability to do changes | 12:40 |
| niemeyer | fwereade: It's fine to adapt values we've set ourselves.. it's not fine to hack user-given values in general | 12:40 |
| fwereade | niemeyer, I feel that argument might not apply all that well to omitted settings... don't they imply "figure it out for me", rather than "I wish to communicate with the fairyland EC2 which has no endpoint?" ;) | 12:42 |
| niemeyer | fwereade: Exactly | 12:42 |
| niemeyer | fwereade: What I mean is that "figure out for me" does not mean "figure out for me just when I bootstrap" | 12:43 |
| fwereade | niemeyer, ok, I've caught up, I think I agree there :) | 12:44 |
| fwereade | niemeyer, so there are indeed some settings that can reasonably be left unset | 12:44 |
| fwereade | niemeyer, and still have a valid config | 12:44 |
| fwereade | niemeyer, how does authorized-keys fit in here? | 12:46 |
| fwereade | niemeyer, I'm pretty certain that should be an environment setting | 12:46 |
| fwereade | niemeyer, and I'm also pretty sure that existence of authorized-keys-*path* in the /environment node indicates that we've screwed up pretty badly | 12:47 |
| niemeyer | fwereade: Agreed on both | 12:47 |
| niemeyer | fwereade: keys-path should be translated to keys as we do today | 12:47 |
| fwereade | niemeyer, ok; would you agree that an authorizedKeysPath field on environConfig is an unwarranted temptation to foolishness? | 12:48 |
| * niemeyer looks | 12:51 | |
| niemeyer | fwereade: Which environConfig? | 12:51 |
| fwereade | niemeyer, ec2 | 12:52 |
| niemeyer | fwereade: You mean providerConfig? | 12:53 |
| fwereade | niemeyer, er, sorry, yes | 12:53 |
| niemeyer | fwereade: np, just to make sure we're on the same page | 12:54 |
| niemeyer | fwereade: Do you have alternative suggestions? | 12:54 |
| fwereade | niemeyer, essentially, that we have 2 paths for config parsing -- fuzzy user configs and strict internal configs | 12:55 |
| fwereade | niemeyer, strict configs can have empty values as discussed above ofc, but we do need to do slightly different work when interpreting user configs and when checking for validity of internal ones | 12:56 |
| niemeyer | fwereade: Feels like a lot of code and work for a one-off field | 12:56 |
| fwereade | niemeyer, similar considerations IMO apply to region | 12:57 |
| niemeyer | fwereade: Hm.. why? | 12:58 |
| niemeyer | fwereade: The path is special because we load from the filesystem on one side, and don't care about the other side | 12:58 |
| fwereade | niemeyer, because region is an env property that should be immutable | 12:58 |
| niemeyer | fwereade: The region doesn't feel similar | 12:58 |
| fwereade | niemeyer, the "what if the default changes" argument applies in the opposite direction | 12:58 |
| niemeyer | fwereade: The path being changed is irrelevant | 12:59 |
| niemeyer | fwereade: A region being changed can be caught up on SetConfig and refused | 12:59 |
| niemeyer | fwereade: Seem simple | 12:59 |
| niemeyer | Seems | 12:59 |
| fwereade | niemeyer, I was referring to the "changing defaults" argument you made wrt ec2-uri | 12:59 |
| niemeyer | fwereade: Erm.. I'm even more confused now | 13:00 |
| fwereade | niemeyer, if we change the ec2-uri default it is a bad thing to have baked it in from the start | 13:00 |
| fwereade | niemeyer, if we change the region default it is a bad thing NOT to have baked it in from the start | 13:00 |
| niemeyer | fwereade: I'm not suggesting we change anything right now.. | 13:00 |
| niemeyer | fwereade: I was pointing out reasoning why having unset fields matters | 13:01 |
| niemeyer | fwereade: The region may be hardcoded yeah, because it can't be changed | 13:01 |
| fwereade | niemeyer, ok, and that then implies again that if we get an /environment without a region, we have screwed up, just as we have if we get an /environment with an authorized-keys-path | 13:02 |
| fwereade | niemeyer, and hence is IMO support for the idea that user configs and internal configs are different enough that we want separate paths for interpreting them | 13:03 |
| niemeyer | fwereade: Sorry, I don't see the relation between these two points. | 13:04 |
| fwereade | niemeyer, both should translate to the same type | 13:04 |
| niemeyer | if region == "" { region = "us-east-1" } | 13:04 |
| niemeyer | fwereade: Why are we discussing this? :) | 13:04 |
| niemeyer | fwereade: The path case is special.. the region case is trivial | 13:05 |
| fwereade | niemeyer, because I think that interpreting an /environment config in exactly the same way as one from environments.yaml is sloppy and risks bugginess | 13:07 |
| niemeyer | fwereade: I've been trying to demonstrate that this is not the case.. preventing a region from changing is if oldRegion != newRegion && oldRegion was set { return error } | 13:08 |
| niemeyer | fwereade: We don't need a new code path for these three lines | 13:08 |
| niemeyer | fwereade: Do I misunderstand? | 13:08 |
| fwereade | niemeyer, since you're talking about changing regions at runtime, I think so | 13:09 |
| niemeyer | fwereade: Okay, so please bear with me while I adapt to reality | 13:09 |
| fwereade | niemeyer, earlier you pointed out that some settings should remain empty, and not be baked in at parse time, on the basis that we may wish to change the defaults in future and having them baked into the env config would therefore be bad | 13:10 |
| fwereade | niemeyer, agreed? | 13:10 |
| niemeyer | fwereade: Can we please focus on the specific issue of region being changed? | 13:10 |
| niemeyer | fwereade: Otherwise we'll derail I think | 13:10 |
| fwereade | niemeyer, this is directly relevant to region changes | 13:11 |
| niemeyer | fwereade: Yeah, so how is region changed difficult to do with current infra? | 13:11 |
| fwereade | niemeyer, do you agree with how I characterised your position above? | 13:11 |
| niemeyer | fwereade: THat's the core point | 13:11 |
| niemeyer | fwereade: Okay, I guess you do want to review the points made? | 13:13 |
| fwereade | niemeyer, I am laser-focussed right at the moment, because I think it will help our communication | 13:13 |
| niemeyer | fwereade: Ok | 13:14 |
| fwereade | niemeyer, do you agree with my statement that " earlier you pointed out that some settings should remain empty, and not be baked in at parse time, on the basis that we may wish to change the defaults in future and having them baked into the env config would therefore be bad" | 13:14 |
| niemeyer | fwereade: More precisely, | 13:14 |
| niemeyer | fwereade: So the point I made is that having unset values allows us to change the values *when that's wanted*, and I also said that it makes sense to allow values to be unset because some settings have unset values naturally, and we can't have a value like "" for unset because we have typed values in the configuration, and also that we want to show users the configuration that they've made | 13:15 |
| fwereade | niemeyer, maybe we should skip sideways a little -- what is the distinction between a "naturally unset value" and an unset value that indicates that some default substitution should be made? | 13:18 |
| fwereade | niemeyer, can you give me an example of the first kind? | 13:18 |
| niemeyer | fwereade: A naturally value is any value that is unset, and should remain as so because we want to know it's unset | 13:19 |
| fwereade | niemeyer, but I can think of two cases where failing to bake them in will kill environments (even if one of them is historical) | 13:19 |
| fwereade | niemeyer, that is, will kill environments if we ever change the default settings | 13:20 |
| niemeyer | fwereade: I don't understand.. suppose we have a new "environment-description" field | 13:20 |
| niemeyer | fwereade: By default we call it "No description for this environment." | 13:21 |
| niemeyer | fwereade: We've made a typo.. now we want to fix it | 13:21 |
| niemeyer | fwereade: Bingo.. we change the code so that the unset value now returns the proper description. | 13:21 |
| fwereade | niemeyer, ok, that is an example where there are distinct benefits to deferring the substitution rather than doing so at parse time | 13:22 |
| niemeyer | fwereade: If the user has put his own description, though, we don't want to touch that. | 13:22 |
| fwereade | niemeyer, I am aware of these examples | 13:22 |
| fwereade | niemeyer, you already explained it perfectly wrt ec2-uri | 13:22 |
| niemeyer | fwereade: Yeah.. so can we go back to region? | 13:22 |
| fwereade | niemeyer, absolutely! what happens when we change the default ec2 region and it has been left unset? | 13:22 |
| niemeyer | fwereade: We don't change it | 13:22 |
| fwereade | niemeyer, never ever? | 13:23 |
| niemeyer | fwereade: How can we possibly change the region of a running environment? | 13:24 |
| fwereade | niemeyer, well, yeah, that would be crackful | 13:24 |
| fwereade | niemeyer, how can we possibly predict thatus-east-1 will be the appropriate default region for ever and ever? | 13:24 |
| niemeyer | fwereade: We can't, we shouldn't, and as far as we know, we don't.. !? | 13:25 |
| fwereade | niemeyer, well, if we allow region to remain unset, how can we prevent a default region change from causing a *runtime* region change when that env is upgraded? | 13:25 |
| niemeyer | fwereade: Why don't we set the region? | 13:26 |
| niemeyer | Jul 05 10:04:14 <niemeyer> if region == "" { region = "us-east-1" } | 13:26 |
| fwereade | niemeyer, well, we do; but that's a setting that the user did not make | 13:27 |
| fwereade | niemeyer, you seemed to be arguing that we should not do that | 13:27 |
| niemeyer | <niemeyer> fwereade: So the point I made is that having unset values allows us to change the values *when that's wanted* | 13:27 |
| fwereade | niemeyer, agreed and understood | 13:28 |
| niemeyer | Jul 05 10:04:14 <niemeyer> if region == "" { region = "us-east-1" } | 13:28 |
| niemeyer | fwereade: I said that :) | 13:28 |
| fwereade | niemeyer, you *also* argued, and reiterated, that we should not show the user settings that they did not set | 13:28 |
| niemeyer | fwereade: True | 13:28 |
| fwereade | niemeyer, these positions are in conflict | 13:29 |
| niemeyer | fwereade: The region case is fine, though | 13:29 |
| niemeyer | fwereade: and I've been saying that for a while | 13:29 |
| niemeyer | Jul 05 10:01:50 <niemeyer> fwereade: The region may be hardcoded yeah, because it can't be changed | 13:29 |
| niemeyer | fwereade: That said, there's a bit missing it seems, which is why I was trying to focus on how we do that in practice and avoid side-tracking | 13:30 |
| fwereade | niemeyer, I have shown you 2 ways in which a user config is a stupid /environment config -- authorized-keys-path and region -- and I can name more (acquired-mgmt-class and available-mgmt-class, off the top of my head) | 13:31 |
| niemeyer | fwereade: It doesn't look like there's a place for us where we get an Environ to intercept a configuration that is about to be put in SetConfig, in a way that it has both the old and the new config | 13:31 |
| niemeyer | fwereade: Sorry, that path has proven unhelpful already.. I've been repeatedly pointing out that region is easy to solve, and you've failed to provide any facts for why it's not | 13:32 |
| niemeyer | fwereade: If there's a reason why region is hard to solve, please tell me as I do want to understand | 13:32 |
| niemeyer | fwereade: But focus on it.. no backtracking | 13:32 |
| fwereade | niemeyer, region is not hard to solve in the small | 13:33 |
| fwereade | niemeyer, the fact is that certain values, one of which is region, can be safely omitted from environments.yaml but should not be omitted in /environment | 13:34 |
| fwereade | niemeyer, and that certain values that can be set in environments.yaml *should* be omitted in /environment | 13:34 |
| fwereade | niemeyer, I am arguing that there is a meaningful distinction between parsing the two environment configuration styles, and that using the same code to do so is going to bite us one day | 13:36 |
| niemeyer | fwereade: I find much more likely that duplicating the same configuration parsing with different semantics will bite us. | 13:37 |
| niemeyer | fwereade: If we have problems, let's address them | 13:37 |
| niemeyer | fwereade: The one thing I see missing is a way to handle a configuration delta for an environment | 13:38 |
| niemeyer | fwereade: At no point the Environ has both old and new config in a way it could refuse it | 13:38 |
| fwereade | niemeyer, agreed | 13:39 |
| niemeyer | fwereade: I suggest introducing ValidateConfigChange(oldConfig, newConfig) | 13:39 |
| niemeyer | fwereade: In the EnvironProvider interface | 13:40 |
| niemeyer | fwereade: And explicitly state that oldConfig will be nil when it's first set | 13:41 |
| niemeyer | fwereade: So that we can introduce logic in there which can act on initial setup | 13:41 |
| niemeyer | fwereade: Such as the handling of -path | 13:41 |
| fwereade | niemeyer, hmmm, ok, I think I see where you're going | 13:45 |
| fwereade | niemeyer, this would indeed address the potential problem of incompatible config replacements | 13:47 |
| niemeyer | fwereade: and give an entry point for handling the harcoding of region, and the weirdness of path | 13:48 |
| fwereade | niemeyer, ie, exploding if we see missing region or existing path? | 13:52 |
| niemeyer | fwereade: Missing region is fine, we hardcode it to us-east-1 on first setup | 13:52 |
| fwereade | niemeyer, sure, ok, thinko | 13:53 |
| fwereade | niemeyer, but seeing an attempt to replace with missing, or otherwise change it | 13:53 |
| niemeyer | fwereade: existing path is an interesting question.. | 13:53 |
| niemeyer | fwereade: I think we should ignore the setting | 13:53 |
| niemeyer | fwereade: and clean it up after we load it onto -keys | 13:54 |
| niemeyer | fwereade: So the env deployment only ever sees -keys | 13:54 |
| niemeyer | fwereade: Or! | 13:54 |
| niemeyer | fwereade: Perhaps even better | 13:54 |
| niemeyer | fwereade: We can load it and compare to the current -keys, and update it if so | 13:55 |
| niemeyer | fwereade: But never let -path itself pass onto the state | 13:55 |
| niemeyer | fwereade: We don't even have to compare I guess | 13:55 |
| fwereade | niemeyer, wait, when are we replacing env configs at runtime on the client? | 13:55 |
| niemeyer | fwereade: juju set-env? | 13:56 |
| niemeyer | fwereade: and on the first deploy | 13:56 |
| niemeyer | fwereade: That stuff is done in the client | 13:56 |
| niemeyer | fwereade: Server side is too late.. | 13:56 |
| fwereade | niemeyer, ahhh, ok, so only on the first SetConfig | 13:57 |
| niemeyer | fwereade: juju set-env should do something like: 1) State.EnvironConfig; 2) update EnvironConfig with keys provided in the cmd line; 3) call ValidateConfigChange(old, new); 4) if all good, call SetEnvironConfig on State | 13:58 |
| fwereade | niemeyer, where's the Environ? | 13:59 |
| niemeyer | fwereade: No where in that picture.. | 14:00 |
| fwereade | niemeyer, ah, I think I misunderstood you when you said "EnvironProvider interface" above | 14:01 |
| niemeyer | fwereade: Yeah, I meant EnvironProvider interface indeed, not Environ | 14:01 |
| fwereade | niemeyer, sorry :) | 14:01 |
| niemeyer | fwereade: By the time it reaches the Environ it should be sweet | 14:01 |
| fwereade | niemeyer, ok, I need to think about this some more -- I do see it will be useful | 14:08 |
| fwereade | niemeyer, a semi-related thought: why do we error out when parsing environments.yaml if any environ has a problem? (unless that problem is "unknown type" in which case we continue) | 14:30 |
| niemeyer | fwereade: Convenience, probably.. I don't see either way as being specially better than the other | 14:31 |
| fwereade | niemeyer, how would you feel about ignoring invalid environments in the same way we do ones with unknown providers? | 14:34 |
| niemeyer | fwereade: Sounds fine | 14:34 |
| niemeyer | fwereade: Assuming it's not the one in use, of course | 14:35 |
| fwereade | niemeyer, indeed so, I need to double-check how that's handled, but it should be fine | 14:35 |
| fwereade | niemeyer, ok; *then*, how would you feel about dropping authorizedKeysPath from ec2.providerConfig, and loading them into .authorizedKeys when we encounter them? | 14:36 |
| fwereade | niemeyer, with what I suggested above, a bad setting won't prevent other envs from loading; but it will mean we don't have this lurking field that is profoundly wrong if used on the server side | 14:37 |
| fwereade | niemeyer, (yeah, attempts to Open that environ will fall over wit the original error) | 14:39 |
| niemeyer | fwereade: ValidateConfigProvider takes two configurations, which were previously parsed, in a single location | 14:50 |
| niemeyer | fwereade: Considering this, it's not clear to me what you're suggesting | 14:50 |
| niemeyer | Sorry, I meant ValidateConfigChange | 14:50 |
| niemeyer | fwereade: Where would the -path be translated into -keys? | 14:53 |
| fwereade | niemeyer, I'm suggesting we do that when we load the environments.yaml, and wondering whether you would consider that to be evil | 14:53 |
| niemeyer | fwereade: Where would that be? | 14:54 |
| fwereade | niemeyer, ec2.NewConfig | 14:54 |
| niemeyer | fwereade: Hmm | 14:55 |
| fwereade | niemeyer, (ok, so technically just a fter we load the environments.yaml ;)) | 14:55 |
| niemeyer | fwereade: Seems alright | 14:55 |
| niemeyer | fwereade: We can easily move if we find a reason why that's bad at some point | 14:56 |
| fwereade | niemeyer, cool, it will make me much more comfortable, at least at the moment :) | 14:56 |
| niemeyer | fwereade: That's great :) | 14:56 |
| fwereade | niemeyer, cheers | 14:56 |
| niemeyer | Lunch time | 15:50 |
| * niemeyer waves | 17:15 | |
| fwereade | niemeyer, ping | 18:17 |
| niemeyer | fwereade: pongus | 18:17 |
| fwereade | niemeyer, I've been wondering about juju.Conn.Environ | 18:18 |
| fwereade | niemeyer, and have been unable to figure out whether it matters that it is potentially out of sync with the state | 18:18 |
| fwereade | niemeyer, I think the answer is probably "not yet, but it will do as soon as we can extract config information from it" | 18:23 |
| niemeyer | fwereade: Do you have a scenario that is concerning you, so that I can better approach your thinking? | 18:23 |
| fwereade | niemeyer, well, a while ago IIRC we tentatively agreed that DefaultSeries would be a sensible thing to add to Environ | 18:24 |
| fwereade | niemeyer, if it is, then as soon as the deploy command tries to get it (as loaded from environments.yaml) rather than from the /environment-backed config | 18:25 |
| fwereade | niemeyer, ...then we have a problem | 18:25 |
| fwereade | niemeyer, I am -1 on that now | 18:25 |
| fwereade | niemeyer, we probably need DefaultSeries, and the other common properties, on EnvironConfig | 18:26 |
| fwereade | niemeyer, and since we can't get an EnvironConfig from an Environ it thus doesn't matter | 18:26 |
| fwereade | niemeyer, since the only thing we will need from Environ is Secrets | 18:27 |
| fwereade | niemeyer, and that will only be relevant when the State environ config has no secrets of its own to overwrite | 18:27 |
| niemeyer | fwereade: Hmm | 18:28 |
| fwereade | niemeyer, so, on reflection, what I'm really asking is "are we OK with adding DefaultSeries, Name, Type, etc, (AuthorizedKeys?) to EnvironConfig?" | 18:28 |
| niemeyer | fwereade: We can't get an EnvironConfig from an Environ? | 18:28 |
| fwereade | niemeyer, don't think so, may be blind | 18:28 |
| fwereade | niemeyer, nope, no access | 18:29 |
| niemeyer | fwereade: A bit surprising, but I guess it's ok.. we need the config to create the environ in the first place | 18:29 |
| niemeyer | fwereade: Which means we must already have it | 18:29 |
| fwereade | niemeyer, I think this is actually a good thing, there's no need to know any of that information (other than the secrets) from the Environ | 18:29 |
| niemeyer | fwereade: So, to your question | 18:30 |
| niemeyer | fwereade: What do you mean by "DefaultSeries", more precisely? | 18:30 |
| niemeyer | fwereade: What is that, a constant, a method, ..? | 18:30 |
| fwereade | niemeyer, I mean the value of the default-series key, returned from a method that is part of the EnvironConfig interface | 18:31 |
| niemeyer | fwereade: No, that's bogus | 18:31 |
| niemeyer | fwereade: Wait, wait | 18:31 |
| niemeyer | fwereade: Not in the way I initially thought.. I see you selected the things that are meaningful cross env | 18:32 |
| niemeyer | fwereade: Hmmm | 18:32 |
| fwereade | niemeyer, yeah, that was the idea | 18:33 |
| niemeyer | fwereade: Not sure.. it feels like we'll be increasing the API surface, and in a way faking it out because we do need to know the string values in more than one place (set-env, parsing, etc) | 18:33 |
| niemeyer | fwereade: What's the goal? | 18:34 |
| fwereade | niemeyer, proximate goal is "implement deploy as required" ;) | 18:34 |
| fwereade | niemeyer, for that I need the default-series one way or another | 18:34 |
| fwereade | niemeyer, and I'm not quite sure that it's better to have State.EnvironDefaultSeries than it is to have State.EnvironConfig return an EnvironConfig that I can then ask | 18:35 |
| niemeyer | fwereade: Sure, but what I'm comparing against is config.Get("default-series") | 18:35 |
| fwereade | niemeyer, ofc I could just use the ConfigNode directly :) | 18:35 |
| niemeyer | fwereade: Which you can then ask anyway | 18:35 |
| niemeyer | fwereade: Didn't we talk about that yesterday, or was it Dave? | 18:36 |
| fwereade | niemeyer, I guess I just hate having the environment config node available for direct manipulation outside the state package | 18:36 |
| niemeyer | fwereade: I see | 18:36 |
| niemeyer | fwereade: So, I like the principle of what you want too | 18:36 |
| fwereade | niemeyer, you seemed cautiously +1 on State.EnvironConfig returning an actual EnvironConfig earlier :) | 18:37 |
| niemeyer | fwereade: Let's try to solve the key issue I mentioned above then | 18:37 |
| niemeyer | fwereade: Yes, I still am | 18:37 |
| niemeyer | fwereade: It sounds reasonable.. we just have to solve that mismatch in a nice way | 18:37 |
| fwereade | niemeyer, jolly good :) | 18:37 |
| niemeyer | fwereade: We have two places (config and cmdline) talking about env-config in terms of string keys | 18:37 |
| niemeyer | fwereade: How do we translate that? | 18:37 |
| fwereade | niemeyer, well, I'm not 100% sure on this, but for context's sake consider get_serialization_data in python | 18:38 |
| fwereade | niemeyer, I don't think we *have* to expose it in the same way, but it might be quite nice | 18:39 |
| fwereade | niemeyer, ha, no, forget it | 18:39 |
| fwereade | niemeyer, the keys in that are not necessarily the valid keys for the env | 18:40 |
| niemeyer | fwereade: Right | 18:40 |
| niemeyer | fwereade: We're translating key <=> method | 18:40 |
| niemeyer | fwereade: and there's the extra complexity that Environ is an interface | 18:41 |
| niemeyer | fwereade: Hmm | 18:42 |
| niemeyer | fwereade: That translation is exactly what NewConfig does | 18:42 |
| fwereade | niemeyer, those schema.FieldMaps are looking like they suddenly want to be reused somehow :) | 18:44 |
| niemeyer | fwereade: They are already being used, precisely within NewConfig | 18:44 |
| fwereade | niemeyer, yeah; and they're doing ~exactly the job we want them to do in env-set | 18:45 |
| fwereade | niemeyer, except some can't be changed | 18:45 |
| niemeyer | fwereade: Well, and that's exactly the method we should use there? | 18:46 |
| fwereade | niemeyer, how do we properly error on unrecognised settings? | 18:46 |
| niemeyer | fwereade: NewConfig does | 18:46 |
| niemeyer | fwereade: (already) | 18:46 |
| fwereade | niemeyer, no it doesn't | 18:46 |
| fwereade | niemeyer, there's a commented-out test saying "should we error?" | 18:47 |
| niemeyer | fwereade: Ah, sorry, ok.. it's the opposite.. it errors on required+not found | 18:47 |
| fwereade | niemeyer, I would be +1 on making that stricter though | 18:47 |
| niemeyer | fwereade: =1 | 18:47 |
| niemeyer | fwereade: +1 | 18:47 |
| niemeyer | fwereade: =1 is great, though :-) | 18:47 |
| fwereade | haha | 18:47 |
| fwereade | niemeyer, ok, with more strictness, it's trivial | 18:48 |
| fwereade | niemeyer, grab serialization data from existing config, poke in new key, reparse, and ValidateConfigChange | 18:48 |
| niemeyer | fwereade: Woohay! | 18:48 |
| fwereade | niemeyer, ok, I'll make the parsing stricter | 18:49 |
| niemeyer | fwereade: Reading it, ValidateConfig should be clear enough, btw | 18:49 |
| niemeyer | fwereade: The parameters will make the semantics clear | 18:49 |
| fwereade | niemeyer, with (old, new EnvironConfig), yeah | 18:49 |
| niemeyer | fwereade: Right | 18:49 |
| fwereade | niemeyer, bah, keyword | 18:50 |
| fwereade | niemeyer, we'll think of something :) | 18:50 |
| niemeyer | fwereade: Interestingly, it's not | 18:50 |
| fwereade | niemeyer, oh! sweet! | 18:51 |
| niemeyer | fwereade: new, make, delete, etc | 18:51 |
| niemeyer | close | 18:51 |
| fwereade | niemeyer, yeah, all functions :) | 18:51 |
| fwereade | niemeyer, hadn't appreciated that properly until just now :) | 18:51 |
| niemeyer | fwereade: Yeah, handy | 18:52 |
| fwereade | niemeyer, anyway, I'd better be off for a bit | 18:52 |
| fwereade | niemeyer, ttyl | 18:52 |
| niemeyer | fwereade: Have a great one | 19:00 |
| fwereade | davecheney, ping | 23:26 |
| davecheney | fwereade: ack | 23:28 |
| fwereade | davecheney, I'm planning to change State.EnvironConfig and associated watcher to return actual environs.EnvironConfig~s; any thoughts/objections? | 23:30 |
| davecheney | fwereade: is that type already in the tree ? | 23:30 |
| fwereade | davecheney, yeah, it's the interface implemented by all env config types | 23:30 |
| fwereade | davecheney, so hopefully the provisioner will just be able to SetConfig(newConf) and not worry about errors | 23:31 |
| davecheney | fwereade: sounds excellent | 23:31 |
| davecheney | my concern would be in the type of the thing that is passed to the SetConfig (ish) method in the actual provider | 23:32 |
| fwereade | davecheney, assuming we manage to avoid monumental scrwups like somehow saving environments for the wrong type, anyway ;) | 23:32 |
| davecheney | +1 | 23:32 |
| fwereade | davecheney, expand on concern? | 23:33 |
| davecheney | fwereade: i'm guessing that inside, say, the ec2 environ, it still gets the config object of it's coerced type ? | 23:36 |
| davecheney | if that is true, then I have no objection | 23:36 |
| fwereade | davecheney, the Environ.SetConfig methods all cast to the appropriate type, and as I said if we don't manage to screw that up we're golden | 23:37 |
| fwereade | davecheney, the plan is not to even bother sending configs that aren't valid | 23:38 |
| davecheney | i originall had a type assertion at that point, but gustavo felt it was appropriate to just let the runtime panic | 23:38 |
| davecheney | which is pretty reasonable given how hard you would have to fuck that up | 23:38 |
| fwereade | davecheney, yeah, I think that's way over on the "programmer error" end of the spectrum | 23:38 |
| fwereade | davecheney, f7u12: state.AssignmentPolify, state.Info; both used by environs; import loop | 23:46 |
| fwereade | davecheney, not so bad, I just need to move them somewhere, but eww | 23:47 |
| fwereade | davecheney, I'll decide where tomorrow, sleeping now ;) | 23:47 |
| davecheney | urgh | 23:48 |
| davecheney | fwereade: want to leave it with me ? | 23:48 |
| fwereade | davecheney, no worries, I'll deal with it :) | 23:48 |
| davecheney | kk | 23:48 |
| fwereade | davecheney, (both of them seem to me to be reasonable members of environs instead; sanity check?) | 23:49 |
| davecheney | niemeyer: thanks for the review | 23:49 |
| davecheney | i'll do a followup fix for the lower case naming thing (fscking us-east-1) | 23:50 |
| davecheney | fwereade: is there a commit syntax for lp that closes issues ? | 23:53 |
| davecheney | Fixes issue 1234. ? | 23:53 |
| fwereade | davecheney, err, I forget, something like that; but I think lbox takes -bug 123456 | 23:54 |
| davecheney | ahh, cool | 23:54 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!