rogfwereade: mornin'!07:02
rogfwereade: ping07:13
fwereaderog, heyhey!07:15
fwereaderog, nice holiday?07:15
rogfwereade: lovely, thanks, although the weather... left something to be desired07:15
rogfwereade: i'd forgotten just how lovely and wild cornwall actually is07:16
fwereaderog, ha, yes, I have been hearing such reports07:16
fwereaderog, lovely07:16
rogfwereade: we had no problem finding places to park up "al fresco"07:16
rogfwereade: i'm a bit sad though 'cos i managed to splash sea water on my phone on the last day and it's died07:17
rogfwereade: losing some nice photos with it07:17
fwereaderog, aw, bad luck :(07:17
rogfwereade: and probably other stuff too that i haven't thought about. not to mention 300 quid's worth of phone :-(07:17
rogfwereade: annoying thing is that all the stuff's probably still in there ok, i just can't get at it!07:18
fwereaderog, quite so :(07:18
rogfwereade: anyway, i was looking at environs/config07:18
fwereaderog, well, I guess once it's dead you can take it to bits ;p07:18
fwereaderog, ah, yes07:19
rogfwereade: true, but i'll probably just find a flash chip - not easy to hook it up to anything07:19
rogfwereade: there was a reason why parsing was separate from opening07:19
rogfwereade: (gustavo seems keen to remove the distinction)07:20
fwereaderog, cool, go on?07:21
fwereaderog, wait I don't think that's what he wants07:21
rogfwereade: no?07:21
fwereaderog, getting a Config is still separate from getting an Environ and I expect it to remain so07:21
rogfwereade: it looks to me like he wanted validaton to take place at Open time07:21
rogThe translation onto a struct type, though, and the further validation07:22
rogof the configuration itself should still be done in the same style07:22
rogdone today, except inside EnvironProvider.Open.07:23
rogfwereade: but perhaps i'm misunderstanding07:23
fwereaderog, heh, perhaps I was too07:23
fwereaderog, because I'm not sure what he means by "further" there07:24
rogfwereade: i think he's suggesting that *all* the current validation take place in EnvironProvider.Open07:24
rogfwereade: which means that if you have an environs config with several sections, each section will only be validated if you open it.07:25
rogfwereade: i dunno, perhaps that's actually ok.07:25
fwereaderog, I think that aspect particularly is in fact a win07:26
rogfwereade: oh?07:26
fwereaderog, I am still not 100% sold on the rest of it, but I am soldiering through and hoping all will become clear07:26
fwereaderog, just because I have ballsed up the configuration of environment X I should not lose access to environment Y07:27
rogfwereade: hmm, if that's what we want, then gustavo's comments make sense. although i'm not sure there's any need for a config type at all in that case.07:28
rogfwereade: we can just use a map07:29
rogfwereade: and i'm not sure when we'd ever use ValidateConfig07:29
fwereaderog, ConfigSet.Get, env-set, secrets upload07:30
fwereaderog, but, yes, I understand your reaction, the provider-internal struct types now seem pretty pointless07:32
rogfwereade: could you expand on the above ( ConfigSet.Get, env-set, secrets upload)07:32
fwereaderog, (and ValidateConfig does kinda assume that you can get a Config from an Environ in the first place)07:32
fwereaderog, those are the times we want to change environment config07:33
fwereaderog, but, well, I am not really in a position to give a robust defence of these choices07:34
rogfwereade: we can easily change a config by setting elements in the map, i think.07:34
fwereaderog, yeah, but those changes need to be validated somehow07:35
rogfwereade: that's what EnvironProvider.SetConfig would do, no?07:35
rogfwereade: ah, before we put the new config into the state?07:36
fwereaderog, yeah, the idea is that the SetConfig is too late07:36
fwereaderog, I dunno, this API does feel like it will be more awkward in some situations and less so in others07:37
fwereaderog, I thought it was great having the EnvironConfig interface07:38
rogfwereade: has any of the code for env-set or secrets upload already been written?07:38
fwereaderog, not really07:38
fwereaderog, I had the bulk of it written (and, I thought, quite nice), but...07:39
rogfwereade: is the idea behind env-set that you can change any of the environ config settings live07:39
fwereaderog, some of them07:39
fwereaderog, name, type, ec2 region etc must not change07:39
rogfwereade: oh, and what's ConfigSet.Get ?07:40
fwereaderog, ConfigSet is EnvironsConfig or whatever the environments.yaml-wrapper was called07:40
fwereaderog, ie the point at which configs enter the system07:41
rogfwereade: i see. i'm not sure how that relates to ValidateConfig though.07:42
fwereaderog, ValidateConfig will be called as part of it (with nil for a first argument)07:42
rogfwereade: hmm, i realise i don't understand how ValidateConfig is supposed to work07:44
fwereaderog, I dunno, it really doesn't feel to me like it's been thought through properly, but I'm experiencing notable internal conflict between just-implement-what-was-asked-for and but-I-think-it-hasn't-been-thought-through-fully07:44
rogfwereade: what's the "new" argument?07:44
fwereaderog, it always validates new, and returns something based on it, with possible tweaks (ensure region is set; fix the authorized-keys-path malarkey)07:45
fwereaderog, if old is not nil, it also checks need-to-be-immutable fields against old07:45
fwereaderog, it still doesn't feel quite like the right fit to me07:47
* rog looks through code07:48
TheMueMorning btw07:48
fwereaderog, but I do not in general have niemeyer's foresight, so I am trying to reserve judgment and follow the spirit of what he seeks07:48
rogTheMue: yo!07:49
fwereaderog, if you wish to take it up with him I will observe thediscussion with great interest, though07:49
fwereadeTheMue, heyhey07:49
TheMuerog: The two weeks are already over?07:49
rogfwereade: AFAICS your environs/config proposal didn't have equivalent functionality to ValidateConfig, or am i missing something?07:49
* TheMue thought it only has been one so far.07:49
rogTheMue: yes, in the blink of an eye!07:50
fwereaderog, no it didn't -- I was going to add a ValidateReplacement method to the EnvironConfig interface07:50
fwereaderog, I felt that having providers only ever emit validated EnvironConfigs was a great feature07:51
fwereaderog, now any bozo can create a *Config from whatever junk they have lying around in their scope07:51
Arammorning rog.07:52
Aramalready back? :).07:52
rogAram: hiya07:52
fwereaderog, hopefully it will be apparent enough that this is not actually a very helpful thing to do07:52
rogfwereade: with niemeyer's proposal, i really don't see what advantage the config type has over just using a map.07:52
fwereaderog, but it seems reasonably sane to have SetConfig call ValidateConfig under the covers anyway so we shouldn't e able to screw it up too badly07:53
fwereaderog, I am not the right person to give helpful insight into the advantages of niemeyer's proposal07:54
fwereaderog, perhaps it will suddenly click with me07:54
rogfwereade: hmm, it does seem a bit like churn for the sake of it07:54
fwereaderog, but it doesn't feel to me like the simplest or most expedient way to go about it07:54
fwereaderog, well, what I *originally* wanted to was to have State.EnvironConfig and associated methods return and use actual EnvironConfigs07:55
fwereaderog, and it's perfectly easy to do this, and to have real instances which I know for sure are valid, in every possible circumstance, bcause they're backed by a real struct type that can only be created by code inside that specific environ07:57
rogfwereade: currently, there's no way of turning an EnvironConfig back into a map, i think.07:58
rogfwereade: wouldn't that be a disadvantage of the above?07:58
fwereaderog, yeah, but we need that one way or another and it's trivial07:58
fwereaderog, the EnvironConfig itself can be responsible for producing a map that will unpack back to an identical instance07:59
rogfwereade: it's just one more thing to get consistent (every time you add an environ field, you have to add code to go in the opposite direction)07:59
rogfwereade: in gustavo's proposal, the translation is always one way, and i think that could work ok.08:00
rogfwereade: network probs?08:20
fwereaderog, apparently so08:20
fwereaderog, did I miss anything?08:20
rogfwereade: last two things i said:08:20
rog[08:59:29] <rog> fwereade: it's just one more thing to get consistent (every time you add an environ field, you have to add code to go in the opposite direction)08:20
rog[09:00:09] <rog> fwereade: in gustavo's proposal, the translation is always one way, and i think that could work ok.08:20
rogfwereade: last thing i saw from you: [08:59:23] <fwereade> rog, the EnvironConfig itself can be responsible for producing a map that will unpack back to an identical instance08:21
fwereaderog, ha, our timelines are somewhat divergent08:21
fwereade<fwereade> rog, yeah, but we need that one way or another and it's trivial08:23
fwereade rog, the EnvironConfig itself can be responsible for producing a map that will unpack back to an identical instance08:23
fwereade<fwereade> rog, don't we have to do that regardless?08:23
fwereade rog, they're stored in /environment as maps, so we have to have some way to turn a config into a map, like it or now08:23
fwereaderog, I think it would be a really good thing if each env's idiosyncratic config-handling code were isolated behind a Config interface08:24
fwereaderog, it seems to me that different static types for configs with very different behaviour is a good thing because we don't have to smoosh the config-handling code across various bits of the relevant Environ and EnvironProvider types08:24
fwereaderog, as it is the only opportunity we have to turn it into a struct type is at SetConfig time, at which point its usefulness as a struct seems to me to be somewhat limited08:24
* fwereade at this point looked at the code again and remembered something else, and went off along a new train of thought08:24
fwereaderog, and now I'm not sure what to do about environments without secrets08:25
fwereaderog, offhand it seems to me that an environ without secrets is not intrinsically an invalid thing08:25
fwereade rog, ie you don't need to have secrets locally if the env already has them08:25
fwereaderog, but maybe that's a case we don't care about08:25
fwereaderog, hmm, do we have any rational basis for expecting the control-bucket to be public?08:25
rogfwereade: do we expect it to be public?08:25
fwereaderog, I didn't think so08:26
fwereaderog, so that then means we *do* need auth to even get a StateInfo out of an environ08:26
fwereaderog, and an Environ is not actually valid or useful in any way without its secrets08:27
fwereaderog, this is inconvenient wrt possible future needs (let someone manipulate your env without giving them your AWS keys? can't be done)08:28
rogfwereade: agreed.08:28
fwereaderog, but maybe I should just not worry about that at the moment08:28
fwereaderog, however this breaks my mental model a little08:28
rogfwereade: there's no particular reason that it must be the *same* secret, i suppose08:29
fwereaderog, I had been thinking the right thing to do was to allow an environ to exist without secrets, because I don't like the idea of an environ config delivered by the system that is not actually valid08:29
fwereaderog, true, but that's still a bunch of stuff to manage08:29
rogfwereade: when are we creating a partially valid environ?08:30
fwereaderog, state.Initialize seems sensible08:30
fwereaderog, IMO we should be pushing everything in the config that isn't a secret at bootstrap time08:30
rogfwereade: why's that?08:30
fwereaderog, consider default-series08:31
rogfwereade: i have a suspicion that that might make several things more complex without much benefit, but please persuade me otherwise!08:31
rogfwereade: go on08:31
fwereaderog, if we start an env with a given default-series, and then change our environments config, it is IMO unhelpful to suddenly swap out the environment's default-series justbecause we're doing our first explicit deploy08:32
rogfwereade: that's true of the environ secrets too.08:33
rogfwereade: i think it might be more reasonable just to document that the environ config gets frozen at first deploy08:33
fwereaderog, but  there is a purpose to that and it doesn't lead to apparent random inconsistencies assuming the keys are validin the same situations08:34
fwereaderog, so, env-set shouldn;t work until we've deployed?08:34
rogfwereade: then we don't need to have anything that distinguishes secret from non-secret08:34
rogfwereade: hmm.08:34
rogfwereade: actually, i think env-set should work, but it should freeze the environment first08:35
rogfwereade: so we could say that the first command *after* bootstrap freezes the env08:35
fwereaderog, ok, actually, that works nicely08:35
fwereaderog, kinda :/08:35
* fwereade arranges thoughts08:35
fwereaderog, default-series in particular seems to me to act oddly there08:36
fwereaderog, as an example of an env setting that has a tanglible effect on the bootstrap machine08:36
rogfwereade: because it affects which bootstrap machine is chosen?08:37
fwereaderog, and that it seems somewhat surprising to overwrite that arbitrarily, givn that the expectation want to set up, AIUI, is that environments.yaml settings will not be pushed by random operations08:38
fwereaderog, and that changes to environments.yaml will not in general have any effect on a running environment08:38
rogfwereade: i think that it's ok to say that bootstrap is special, and that environments.yaml is frozen on the first command after bootstrsap08:38
rogfwereade: and that's a reasonable and comprehensible model08:39
fwereaderog, I would prefer not to encourage the idea that it's sane or meaningful to alter environments.yaml after bootstrap08:40
rogfwereade: otherwise people might well say "why is default-series different from aws secret keys?"08:40
rogfwereade: i agree, but we do have the reality that we *have* to push some of the environ config after bootstrap08:41
fwereaderog, because the secrets are unique in that we need them to do *anything* with the env, even get the bootstrap machine's id from the storage08:41
fwereaderog, valid secrets are a prerequisite ofr doing *anything*; and so, transparently pushing known-valid secrets when none already exist in state prevents users from having to actually think about it at all08:42
rogfwereade: am i right in thinking there's only one secret key that we need to push?08:42
rogfwereade: (for ec2, at any rate)08:43
fwereaderog, I guess the access key id is not technically a secret08:43
rogfwereade: yeah08:43
rogfwereade: here's a possible (maybe crackful) thought08:43
rogfwereade: outside of environs, we just use map[string]interface{}08:44
rogfwereade: then08:44
rogfwereade: we provide EnvironsProvider.ValidateConfig(old, new map[string]interface{}) error08:45
rogfwereade: and we say that any key containing the string "secret" is secret08:45
rogfwereade: so it's trivial for provider-agnostic logic to filter out secret keys08:45
rogfwereade: of course, this assumes that other provider types use the string "secret" in their secret keys...08:46
fwereaderog, I have had thoughts roughly along those lines08:46
fwereaderog, not specifically including the secret stuff08:47
* fwereade reserves judgment08:47
rogfwereade: if we go this way, then the environs/config package isn't required at all, i *think*08:47
rogfwereade: oh no, it is08:47
rogfwereade: to avoid the loop08:47
rogfwereade: except...08:47
rogfwereade: there's no need for state to call ValidateConfig08:48
rogfwereade: so there's no need for a cyclic dependency.08:48
rogfwereade: so no need for environs/config, i think.08:48
fwereaderog, I think I may just be overly paranoid about people dumping random crap into /environments08:49
fwereaderog, this is at the heart of my desire to have the state methods that manipulate environ configs work with actual environ configs08:49
rogfwereade: we can still check that!08:50
fwereaderog, but not within state08:50
rogfwereade: state is internal08:50
rogfwereade: we still have control over where the environ config gets into the system08:50
rogfwereade: env-set08:50
rogfwereade: environs.Open08:51
rogfwereade: and those places can do the checking08:51
fwereaderog, so we have a bunch of state methods that validate what they're given, and one that pushes all responsibility for validation onto the various clients08:51
rogfwereade: state methods validate what they can.08:51
rogfwereade: we can state explicitly that the environment pushed into the state is not validated by the state08:52
rogfwereade: that may even be a good thing actually08:52
fwereaderog, ofc we can, doesn't stop it feeling sloppy to me08:53
fwereaderog, we may as well say that state is internal and so there's no need to validate anything we put into it08:53
fwereaderog, we can always trust the client code we write to do the right thing, after all08:53
rogfwereade: it's a trade-off - i *think* that it's not worth creating a new package for08:53
* fwereade maintains a studiously straight face08:54
rogfwereade: there are lots of ways we can create invalid content inside state, i thinl08:54
fwereaderog, like what?08:55
rogfwereade: like we could do OpenPort("tpc", 45)08:56
fwereaderog, instructive example08:57
* fwereade ponders08:57
fwereaderog, I think the distinction there is that there is no global source of truth we can use to validate against the set of protocols we know about elsewhere in the system08:59
fwereaderog, in the environment case we can do so, and it doesn't feel like much work to me08:59
fwereaderog, but look, this is not an especially productive conversation because I keep falling back to describing my own perspective on the problem09:00
fwereaderog, which is to all intents and purposes irrelevant because you the perspective that is most relevant is niemeyer's, and my internal niemeyer sim is not an outstanding predictor of the real niemeyer's thought09:01
rogfwereade: it seems to me that there are several levels of verification we have on the state09:01
fwereaderog, I think I do have to step back and sketch everything out again :)09:02
rogfwereade: i'm looking at niemeyer's paste, and it seems to me that his environs/config is trivial09:02
rogfwereade: and not really worth it09:02
rogfwereade: each of the different methods on config.Config just return a particular field of the map09:03
fwereaderog, look, yes, I am somewhat sympathetic to that point of view; if the prevailing opinion is that the content of /environment is a free-for-all and we can handle that then map[string]interface{} is perfectly sufficient09:04
fwereaderog, however at the moment I am somewhat without compass and it has become apparent that I need to chart the differences between my perspective and niemeyer's in some detail so I can actually proceed usefully09:05
rogfwereade: i'm sympathetic to this point of view of niemeyer's: "09:05
rogThere are very09:05
rogfew places through which configuration get into the system, and the09:05
rogconfiguration should be validated right there.09:05
rogfwereade: so the content of /environment is only a free-for-all if we treat it as such09:05
fwereaderog, my concern is that if we can then at some stage we will09:05
rogfwereade: and placing the validation outside of state means that everything else becomes simpler.09:06
rogfwereade: (the fewer places with registered global state the better, i think)09:06
rogfwereade: the idea behind State isn't, i think, that we design it such that it can express *exactly* the things we can do in juju, but that it's a nice convenient abstraction layer that expresses *at least* the things we can do in juju.09:08
rogfwereade: when we start making things more complex so that we can get more verification in State, i think we're going a little too far.09:09
rogfwereade: it's a convenience for our own use, and i don't think we need to be bound by it.09:09
fwereaderog, I just don't see how requiring explicit validation outside of Config objects *actually* makes things simpler09:09
fwereaderog, it just means we need to do lets of external messing about with maps and typecasting09:09
rogfwereade: it means we don't need a new package with separate global registration09:09
fwereaderog, if that's the only objection then that can be worked around09:10
rogfwereade: really? i thought that was inherent to the cyclic-dependency-breaking nature of environs/config.09:11
fwereaderog, oh wait, maybe not09:11
fwereaderog, no, it means we need a provider to get a config and there's no way around it09:11
rogfwereade: yup09:12
rogfwereade: i'm not sure where you think all the messing around with maps and typecasting will go09:12
fwereaderog, my perspective just clicked around a notch09:14
fwereaderog, I can restrict the messing around to only the bits that are directly concerned with the struct type09:15
rogfwereade: not sure i understand09:15
fwereaderog, ok, this perspectives STM to assume that ValidateConfig will directly construct struct types from its *Config (or map) params09:16
fwereaderog, will manipulate those as required09:16
fwereaderog, and then return a fully validated one09:16
fwereaderog, as a new *Config (or map)09:17
rogfwereade: i don't think there's any need to return a validated config.09:17
rogfwereade: is there?09:17
rogfwereade: isn't the "new" argument supposed to be valid?09:17
fwereaderog, how are we meant to know that?09:17
rogfwereade: isn't that what ValidateConfig checks?09:18
fwereaderog, yeah, a valid user-specified config may still require manipulation09:18
fwereaderog, (1) region must never change at runtime, so we should bake in the value in case an update changes the default region09:18
rogfwereade: that's why we pass in the old config, no?09:19
fwereaderog, (2) authorized-keys-path should not exist on the server -- if it does it's evidence of total crazkfulness on our part09:19
fwereaderog, and we should therefore remove it and sub in a correct authorized-keys setting09:19
rogfwereade: how does the provider know that it's on a server or not?09:20
fwereaderog, right! it shouldn't have to -- it shouldn't even see anything that could indicate a difference09:20
fwereaderog, the actual configuration setting is authorized-keys -- authorized-keys-path is a convenient alternate way of setting it09:21
fwereaderog, the environment should not have to care even slightly about what filesystem it happens to be running on, or even I guess whether it is ;)09:22
rogfwereade: i see. what we get out of environments.yaml isn't what we put into the state.09:23
fwereaderog, yeah, I have this total conviction that they are two distinct things and we could make our lives easier by distinguishing them in code09:24
fwereaderog, but that has not received a lot of traction09:24
fwereaderog, to tweak what you said: what we get out of environments.yaml isn't what fwereade personally thinks we put into the state.09:25
fwereaderog, I guess we could just have the field and not bother to look at it when it might not be valid, but that seems pretty crackful itself09:25
fwereadeto me09:25
rogfwereade: mutating authorized-keys-path makes sense.09:26
rogfwereade: this *was* my idea for how ValidateConfig would work: http://paste.ubuntu.com/1082463/09:27
fwereaderog, yeah, I think it needs a little more09:27
rogfwereade: the problem with making client-side and server-side config two different things is that they are really very similar!09:28
fwereaderog, sure09:28
rogfwereade: hmm, against my previous inclination, perhaps we should just have Environ.Config to return a configuration map from an opened environ09:29
fwereaderog, but if we had struct types we could just serialize/deserialize the struct types with yaml for storage, and *only* ever let valid struct types into the system, and only bother with the fancy stuff when loading a user config09:30
fwereaderog, yeah, that feels to me like a sensible thing09:30
rogfwereade: that means we'd need two versions of the deserializing code, which seems somewhat like overkill to me.09:32
rogfwereade: also, we can't deserialize directly from a config code, i think.09:33
fwereaderog, one which just blats the yaml direct into a struct, and one that contains all the pre-validation and tweaking etc?09:33
fwereaderog, oh? why not?09:33
fwereaderog, I would imagine we could just blat it straight into a *whateverConfig09:34
rogfwereade: because yaml.Unmarshal takes a []byte09:34
rogfwereade: not a map[string]interface{}09:34
fwereaderog, yes, this also depends on the separate global config registry so that we can have the configs directly responsible for what is stored09:35
fwereaderog, which I know you don;t like09:35
rogfwereade: the *only* reason that i made config parsing separate from config opening was so that a config file would get validated completely.09:36
rogfwereade: and actually, that's still possible with ValidateConfig09:36
fwereaderog, I don't actually see the benefit of refusing to work with env X because env Y is broken09:36
rogfwereade: you may well be right. in which case i'm very happy to live without passing around configurations as structs.09:39
rogfwereade: i'm happy to have state never call into environs09:39
fwereaderog, yeah, I know :)09:39
rogfwereade: it seems like a reasonable separation of concerns to me09:40
rogfwereade: BTW does state actually need to know anything that goes on inside the environ config at all?09:41
rogfwereade: for instance, does the state need to know that environ name, default series or type?09:42
fwereaderog, only if we care about its validity -- and I still think we do/should but I think I have been pretty comprehensivelyoverruled on that one09:42
fwereaderog, certainly default-series09:42
rogfwereade: of course we care about its validity...09:42
fwereaderog, yes, but it has been argued that we can trust the clients09:42
rogfwereade: they're our clients, so can we trust ourselves?09:42
fwereaderog, and therefore at that specific point we merely assume validity, rather than worrying about it09:43
rogfwereade: yea09:43
rogfwereade: what does state use default-series for?09:43
fwereaderog, IMO no, of course we can't trust ourselves ;p09:43
fwereaderog, deploy command09:43
fwereaderog, and the only sane place to get it is from state09:43
rogfwereade: the deploy command isn't in state, is it?09:43
fwereaderog, how is it going to get the default series *except* from state?09:44
rogfwereade: i'm not suggesting that the default series isn't store inside state,09:44
rogfwereade: but that state has no reason to get default-series (or any environ config attribute) itself09:45
rogfwereade: i.e. for the purposes of the state package, the environ config is opaque09:45
fwereaderog, well there was talk of a state.DefaultSeries a while back but I think that would indeed want reevaluation in the light of the current direction09:45
fwereaderog, it all comes down to how much we trust ourselves, and I have found that *not* trusting myself is frequently a winning strategy09:47
fwereaderog, I am happy to follow the collective wisdom on this point but I have not successfully internalised the attitude that the decision follows from09:48
rogfwereade: it's a trade-off as always IMHO. i've also found that keeping things as simple as possible is good.09:48
rogfwereade: it's also a matter of how easy it is to verify the trust09:49
rogfwereade: in this case, i think there are only a very few places that need verification.09:49
fwereaderog, yeah, I know -- I'm just saying it's a tradeoff I would make differently, and that makes it harder because my complexity-predicting heuristics lead me in a different direction to that I am consciously aiming for09:51
rogfwereade: here's a representation of my current thinking: http://paste.ubuntu.com/1082500/10:04
rogfwereade: oops, EnvironConfig should have gone there10:05
fwereaderog, was wondering about that10:05
fwereaderog, not implausible, I'm just trying to think through the consequences of a slightly different shape10:06
rogdavecheney: hiya!10:44
Aramwhat's the difference between Test and TestPackage in Gocheck?10:53
davecheneyrog: howdy mate11:11
davecheneyhad a good holiday ?11:11
rogdavecheney: that i have11:11
rogdavecheney: just trying to get up to speed with all the stuff that's happened while i've been gone...11:11
rogdavecheney: how're the agents these days?11:12
rogdavecheney: did you manage to get that branch working?11:12
rogAram: i don't understand your question11:16
Aramrog: the function used to integrated gocheck tests into gotest sometimes is named Test, sometimes TestPackage.11:17
Aramany particular meaning to that?11:17
rogAram: it looks like Test is standard11:18
rogAram: but it doesn't matter too much11:19
davecheneyrog: yup, it's waiting on final approval now11:20
rogdavecheney: yay!11:20
rogdavecheney: so the machine agent is actually starting new machines now?11:21
rogdavecheney: i mean the PA of course11:21
davecheneyrog: no11:21
davecheneysadly not11:21
davecheneywe're still stuck at the 'who pushes the secrets' problem11:22
rogdavecheney: ah, of course.11:22
niemeyerGooood Mondays!13:07
Aramhey hey13:09
rogniemeyer: yo!13:14
niemeyerrog: Welcome back!13:18
rogniemeyer: i was discussing the newly proposed environs/config package with fwereade earlier13:19
rogniemeyer: i think i'm broadly in agreement with your take on it13:20
niemeyerrog: That's great.. I think we're all in agreement then, phew13:20
rogniemeyer: i'm not sure that the actual package pulls its weight though13:20
niemeyerrog: The package is needed for the state package to not have a dependency on environs13:21
rogniemeyer: i was wondering about something like this: http://paste.ubuntu.com/1082729/13:21
rogniemeyer: i'm not sure13:21
rogniemeyer: i think the main reason for the dependency was so that state could call back into environs to verify the config13:22
rogniemeyer: but your proposal does away with that13:22
rogniemeyer: and hence, i *think*, the need for a new package13:22
niemeyerrog: Yes, and state still wants to return a Config13:22
niemeyerrog: Despite the fact it won't be validating13:22
rogniemeyer: it could just return a map[string]interface{}13:22
niemeyerrog: Yes, it could be less nice, but it can be nicer too13:23
rogniemeyer: which can then be validated outside state13:23
niemeyerrog: There's no need to validate what's coming out of state13:23
niemeyerrog: It was validated when going in13:23
rogniemeyer: oh yes, that's true13:23
rogniemeyer: if we *do* have a config package, i also wondered if it was really worth hiding the map.13:25
rogtype Config map[string]interface{}13:25
rogmight be more transparent13:25
niemeyerrog: I think it is, because we don't want code to be fiddling with the map in principle13:25
rogniemeyer: ok, fair enough13:26
niemeyerrog: Nor using stock methods (DefaultSeries) from the map13:26
rogniemeyer: i don't mind m["default-series"].(string) too much13:26
rogniemeyer: but i see your point too13:27
niemeyerrog: I do mind, because it's just as easy to write m["default-seres"].(string)13:28
rogniemeyer: presumably we'd also need config.New(attrs map[string]interface{}) (*Config, error) too?13:29
rogniemeyer: with a config package, i might suggest something like this: http://paste.ubuntu.com/1082741/13:30
niemeyerrog: Yeah, that sounds good13:30
niemeyerrog: Yeah, that looks mostly ok.. the Validate method needs to return a new config, though, because the proposal is to have config being immutable for now13:31
rogniemeyer: why does validate need to return a new config?13:32
rogniemeyer: isn't the new config already valid?13:32
niemeyerrog: The transition from -keys-path to -keys will be done there13:32
rogniemeyer: i thought that could be done by Environ.Config13:32
niemeyerrog: Doing SetConfig/Config so we can get a proper configuration is awkward.. validation sounds like the proper place to do it13:33
rogniemeyer: it seems like Validate is doing more than just validation here.13:34
niemeyer  Validate \Val"i*date\, v. t. [See {Valid}.]13:35
niemeyer     To confirm; to render valid; to give legal force to.13:35
rogniemeyer: and i'm not sure that we would ever need to do SetConfig then Config to get a valid configuration - wouldn't we always Open the environ first.13:35
niemeyerThat's validation.13:35
niemeyerrog: Not necessarily.. we can change the configuration without going through a re-open cycle13:36
rogniemeyer: the "?" was to be added to the end of my sentence...13:36
niemeyerrog: Sorry, ok, that's the answer htne :)13:36
rogniemeyer: so Validate is the same as Open(old) followed by SetConfig(new) followed by Config?13:38
niemeyerrog: No.. validate is definitely not the same as Open13:39
rogniemeyer: i'm trying to get a handle on the different checks performed by Validate vs Open13:39
Aramman, how annoying the misinformation on #go-nuts can be. "regexp needs an overhaul", "yes, it's very bad on the shootout", etc.13:40
rogniemeyer: because, presumably Validate does some checks that are currently performed by Open, such as opening the ssh provider key13:40
niemeyerrog: What does opening a key mean?13:41
niemeyerAram: Ouch13:41
rogniemeyer: i mean opening the authorized-keys-path and reading it, so that it can transmute it into authorized-keys13:42
niemeyerrog: Well, yes, I just mentioned that above13:42
niemeyerrog: Another one is the validation of whether a region can change, for example, and the hardcoding of the region name13:53
niemeyerrog: region: "" => region: "us-east-1"13:53
rogniemeyer: something like this, perhaps? http://paste.ubuntu.com/1082778/13:54
niemeyerrog: Yeah, except it should be called Validate as we've been agreeing on13:54
rogoops, yeah13:54
rogniemeyer: as for secrets, how about when the environment is initially uploaded, all keys containing the string "secret" are excised.13:55
rogniemeyer: alternatively...13:55
niemeyerrog: That's a great idea, but it's unfortunately too late13:56
rogniemeyer: we don't upload the environment at all until the first command after bootstrap.13:56
niemeyerrog: Too late as in we have a configuration format right now that people are used to13:56
rogniemeyer: are there secret keys in other providers that are called different things?13:56
rogniemeyer: i guess so.13:56
rogniemeyer: i should look at orchestra etc13:57
rogniemeyer: another alternative: http://paste.ubuntu.com/1082789/13:58
niemeyerrog: That doesn't seem like a good place to do this13:59
rogniemeyer: where are you thinking is a good place?13:59
niemeyerrog: We should look at the problems we have relative to secrets, and try to address those, but for now it sounds like we're good to go with the first change13:59
rogniemeyer: sounds good14:02
niemeyerrog: When you have a moment would you mind to file your flight details in the sprint page?15:44
niemeyerrog: Otherwise you may miss a room :-)15:45
rogniemeyer: ok, will do15:45
rogniemeyer: hmm, i'm not allowed to view the page, it seems15:46
niemeyerrog: You have to login15:46
rogniemeyer: i was logged in. perhaps i was logged in as the wrong user... will try again. sadly my phone is dead, with all my login details with it :-(15:47
niemeyerrog: Everyone in Canonical must necessarily have access to this wiki15:47
niemeyerrog: I don't think you can have two users there even15:48
rogniemeyer: ah, yes, i've now logged in. apparently i've got two valid logins.15:48
niemeyerrog: I doubt it15:48
niemeyerrog: This is associated to your Launchpad account, and it's tightly controlled15:48
rogniemeyer: there was a mix up. i thought they'd sorted it out, but it seems not.15:49
rogniemeyer: i've got two valid launchpad ids15:49
rogniemeyer: but only one is associated with the right canonical perms15:50
niemeyerrog: Well, apparently one of them doesn't work? :-)15:50
rogniemeyer: it works for some things...15:50
niemeyerrog: Sure, it can work for anything that is not the internal wiki :)15:50
rogniemeyer: this was the post i got from someone in admin: https://pastebin.canonical.com/69677/15:52
rogniemeyer: it probably not surprising i was confused15:53
niemeyerrog: Indeed15:53
niemeyerSounds messed up15:53
rogniemeyer: yeah. i'm not sure i really understand the underlying auth model.15:54
rogniemeyer: done.15:59
niemeyerrog: Cheers15:59
rogTheMue: hey, seems like we're sharing flights!15:59
TheMuerog: Nice, should try to get seats together.16:02
TheMueniemeyer: How free are you for reviews today? With https://codereview.appspot.com/6345072/ in I would propose my firewall tomorrow for a first look. Right now I'm doing the tests, all service related ones are green, now I start with the machin related ones.16:03
niemeyerTheMue: I was just about to ping you to talk about your eod email the other day and see how things are going16:04
niemeyerTheMue: But I decided not to because lunch is waiting16:04
niemeyerTheMue: I'm definitely available for reviews + talk on it16:04
niemeyerTheMue: In ~1h16:05
TheMueniemeyer: Fine, the watcher in the review above should be simple.16:05
niemeyerTheMue: Super, let's catch up in a moment then16:06
rogniemeyer: what was the name of that logging web service you mentioned a while back?16:53
niemeyerrog: https://papertrailapp.com/17:01
rogniemeyer: thanks. i'd been grepping the logs and googling "logging as a service" but failed to dig it up17:02
niemeyerrog: No worries17:02
niemeyerTheMue: Yo17:27
niemeyerTheMue: ping?17:36
* rog is off for the day. i hope to get something more done than just catching up tomorrow!17:36
niemeyerrog: Have a good one17:39
TheMueniemeyer: pong17:42
niemeyerTheMue: Hey17:42
niemeyerTheMue: Going over your CL now.. do you want to catch up something around it?17:42
TheMueniemeyer: No, it's only needed for the one containing the firewall. That one is almost done (some tests missing) and I would like to propose is tomorrow morning.17:44
niemeyerTheMue: Super17:45
TheMueniemeyer: I've needed some time to get behind it, but it's nice to see in the tests how it fits together.17:46
niemeyerTheMue: Neat!17:48
niemeyerTheMue: That's always a good sign17:48
niemeyerTheMue: I thought the watcher tests had been moved to a nicer location (out of state_test.go) with fwereade_'s modification?18:27
TheMueniemeyer: It has been watcher_test.go before. William moved them to the types where you retrieve them from. And This one is retrieved directly from State,18:30
niemeyerTheMue: I see.. there's WatchMachines there indeed18:32
niemeyerTheMue: Might be good to at least put them next to each other18:32
niemeyerTheMue: Perhaps pulling WatchMachines down to the end18:32
TheMueniemeyer: Geed idea.18:33
niemeyerTheMue: The use of idx in the test is a bit awkward18:34
niemeyerTheMue: Well.. I guess it's fine18:34
niemeyerTheMue: Bothers me a little that it only works for a single change, but I guess it's ok18:34
niemeyerTheMue: LGTM with doc suggestions only18:35
TheMueniemeyer: Cheers18:36
TheMueniemeyer: Will do it tomorrow, wife want's me now. ;)18:37
niemeyerTheMue: Enjoy man18:37
TheMueniemeyer: Thx, cu.18:37
niemeyerTheMue: Cheers18:37
niemeyerWill have a walk outside.. back later20:24
niemeyerdavecheney: Morning!22:13
davecheneyniemeyer: howdy22:41
davecheneyjust on the phone22:41
niemeyerdavecheney: About // TODO(rogerpeppe) change upstart.Conf.Cmd to []string so that22:50
niemeyerdavecheney: This is going in with this CL, apparently?22:50
niemeyerdavecheney: Ah, you've based off of his branch22:50
davecheneyniemeyer: yes22:50
davecheneythat is somethign rog identified, i guess he is going to look at it22:51
davecheneybut I can do so if you like22:51
niemeyerdavecheney: Please just drop it.. that code is fine. THere's no quoting necessary for it to work correctly22:51
niemeyerdavecheney: Actually, hold on22:52
niemeyerdavecheney: The %q there is bogus.. that's probably what it is referring to22:52
niemeyerdavecheney: I suggest using something simpler, such as "'%s'"22:53
niemeyerdavecheney: No address should contain single quotes in them, so this should be fine22:53
niemeyerdavecheney: It's also not something we're getting through user input, which might turn it into an issue22:54
niemeyerdavecheney: Sent a review on https://codereview.appspot.com/634704422:59
davecheneythanks, still on the phone22:59
davecheneywill check in a sec22:59
niemeyerdavecheney: Cheers.. there just a couple of minors from the previous where we seem to misunderstand each other. Otherwise we can move on23:00
davecheneyawesome, i'll review those now23:00
niemeyerand a plain LGTM on the other one.. good stuff.23:03
davecheneyniemeyer: in related news, are you able to add mramm to the golang gophers23:04
davecheneyand also juju-dev on lp23:04
davecheneyi noticed he was missing from those groups ?23:04
niemeyerdavecheney: You mean ~juju?23:05
niemeyer(there's no juju-dev, I think)23:05
davecheneyniemeyer: yes, i think I do, he should probably be in the same groups I am in so he can receive moar email23:06
niemeyerDone adding to ~juju23:06
niemeyerdavecheney: Haha :)23:06
davecheneyniemeyer: just submitting my fix branch that fixes a bug on lp[23:06
davecheneythis will be interesting to see how the integration works23:06
niemeyerdavecheney: hm? bug in lp?23:06
niemeyerWhich lp?23:07
davecheneyno no, a bug that was registered to lp:goamz project23:07
davecheneyactually no, the bug is on juju-core23:07
davecheneyi have another branch for the goamz work23:07
davecheneywhat i meant is this is the first time submitting a -bug via lbox23:08
davecheneyniemeyer: in related news, i had a quick go at using gocov on juju-core23:08
davecheneyit works, but is quite buggy23:08
davecheneyniemeyer: this was the goamz bug, https://bugs.launchpad.net/goamz/+bug/102151523:10
niemeyerdavecheney: Ah, I see.. your first message got here corrupted (it said "lp[")23:10
niemeyerdavecheney: As one of the tasks for the sprint, I'd like to try to get the lifecycle logic correct in mstate, state, and the provisioning agent (which is our most complete agent to date)23:24
niemeyerdavecheney: making it consider alive/dying/dead semantics23:24
niemeyerdavecheney: Just something to boil up in your mind meanwhile23:25
davecheneyniemeyer: yes, i have been pondering on that since last week23:36
niemeyerPhew.. review queue empty again23:37
davecheneynice work23:39

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!