[14:14] <fwereade> niemeyer, response to https://codereview.appspot.com/6355077/ fwiw
[14:15] <niemeyer> fwereade: I'm almost done with the reply
[14:16] <fwereade> niemeyer, cool :)
[14:16] <niemeyer> fwereade: Done
[14:21] <niemeyer> fwereade: Actually, we don't need Get and Set..
[14:21] <niemeyer> fwereade: If Validate wants to mutate the configuration, grab the map, do whatever, and return a new config
[14:22] <niemeyer> fwereade: This way a Config is immutable
[14:22] <niemeyer> fwereade: Which is a nice property and goes towards your desires
[14:22] <fwereade> niemeyer, ok, that would probably work for me
[14:23] <niemeyer> fwereade: So ValidateConfig(old, new) => (valid *Config, err error)
[14:29] <fwereade> niemeyer, yes indeed (sorry, laura is being obnoxious ;))
[14:30] <niemeyer> fwereade: It's Saturday, she's right! :p
[14:31] <fwereade> niemeyer, nah, hitting me in the balls when I censured her for interrupting a conversation ;p
[14:31] <niemeyer> fwereade: ROTFL
[14:31] <niemeyer> fwereade: That's a bit extreme indeed ;-)
[14:31] <fwereade> niemeyer, (I don't think it was deliberately targeted but that did not make it a notably more enjoyable experience ;))
[14:32] <niemeyer> fwereade: I know.. it's just the easiest target for her size
[14:32] <fwereade> niemeyer, yeah :)
[14:32] <fwereade> niemeyer, my only reservation remains that I don't really see on what basis you make the distinction between environ config and all the other things that we handled as dicts in python
[14:33] <fwereade> niemeyer, I've been loving knowing exactly what's in the types and don;t really see any drawbacks :)
[14:33] <niemeyer> fwereade: I think you're too focused on how Python used a dict for the configuration
[14:33] <niemeyer> fwereade: The configuration *is* a dict
[14:33] <niemeyer> fwereade: That's not the issue per se
[14:34] <niemeyer> fwereade: and as I explained in the reply, you'll continue having a type in the exact places we have them today
[14:34] <niemeyer> fwereade: Nothing is changing
[14:35] <niemeyer> fwereade: We're just gaining in: a) Being able to carry that type around in a better form, in and out of state; b) Having a proper interface that reveals the common fields; c) Having a properly typed EnvironConfig in *more* places
[14:35] <niemeyer> fwereade: Right now we do provider.NewConfig(attrs).Open()
[14:36] <niemeyer> fwereade: We'll now do provider.Open(config)
[14:36] <niemeyer> fwereade: The typing happens inside the same path
[14:36] <fwereade> niemeyer, that is indeed surely nicer
[14:37] <fwereade> niemeyer, I think the specific thing I am paranoid about is that State.SetEnvironConfig will be unable to do appropriate replacement validation, and I prefer to mistrust my clients (even when I myself am writing the client ;))
[14:38] <niemeyer> fwereade: I have the same concern, and I see the solution I'm suggesting as doing a better job in exact that regard
[14:39] <niemeyer> fwereade: SetEnvironConfig is too late for doing validation.. we shouldn't allow an invalid config to ever reach it
[14:39] <niemeyer> fwereade: The proper place to do configuration validation is "at the door"
[14:44] <fwereade> niemeyer, I totally agree there -- it's just that, well, bad things happen and stupid code slips in, and I'm twitchy about being unable to do sanity-checking in the state package
[14:46] <fwereade> niemeyer, offhand we want to set env state (1) at Initialize time, (2) at env-set time, (3) (probably) when setting constraints when we get to them and (4) on first secure connection, for the secrets
[14:46] <fwereade> niemeyer, the burden of putting the sanity-checking in each of those places is I agree not overwhelming
[14:47] <fwereade> niemeyer, but my heart says that I should be able to fling anything I want at State.SetEnvironConfig and have it dourly block crazy requests
[14:49] <niemeyer> fwereade: Man.. do you want me to implement that?
[14:49] <niemeyer> fwereade: This is trivial really
[14:49] <niemeyer> fwereade: We only need a single function that does validation of configuration at entrance
[14:50] <niemeyer> fwereade: This is getting totally out of proportion to the actual problem
[14:50] <fwereade> niemeyer, sorry, I'm not even really arguing against your approach any more -- I'm mostly just trying to figure out where I'm being crazy
[14:51] <niemeyer> fwereade: The root of the problem is that you're obsessed about doing validation at SetEnvironConfig, and can't see that it's trivial to do validation in *one single place* elsewhere
[14:55] <niemeyer> fwereade: That should be the place where we get EnvironConfig from..
[14:55] <niemeyer> fwereade: If the way we use to load EnvironConfig validates it, how can we possibly get an EnvironConfig into State.SetEnvironConfig that is invalid?
[14:57] <fwereade> niemeyer, ok, I appear to have missed something... the Validate method you propose takes a *Config
[14:58] <fwereade> niemeyer, well, two, but I thought you were saying we should be calling ValidateConfig(nil, someConfig)
[14:58] <niemeyer> fwereade: Yes, that's the interface through which the *provider* validates the config
[14:58] <niemeyer> fwereade: Who calls it?
[14:58] <niemeyer> fwereade: How do we get from yaml onto a *Config?
[14:58] <fwereade> niemeyer, ok; but that is not validating at the gate
[14:59] <niemeyer> fwereade: No, it's not, it's infrastructure to do what must be done
[14:59] <niemeyer> fwereade: We can have something within environs itself that validates it at the door
[14:59] <niemeyer> fwereade: Leave Read there, for example
[15:00] <niemeyer> fwereade: and run ValidateConfig on all configuration we get from environments.yaml
[15:00] <fwereade> niemeyer, ok, *that* SGTM
[15:01] <fwereade> niemeyer, it seemed to me that I had propsed something that validated at the gate and you were saying we didn't need that
 fwereade: We only need a single function that does validation of configuration at entrance
[15:02] <fwereade> niemeyer, (I'm not *specifically* obsessed with validating in state -- more with being sure that if I see a *Config I can know is actually valid)
 fwereade: The proper place to do configuration validation is "at the door"
[15:02] <niemeyer> fwereade: It seems like I've been repeating that since yesterday
[15:03] <fwereade> niemeyer, yeah, and your proposed solution did not allow for that -- but I think moving Read/ReadBytes does
[15:03] <niemeyer> fwereade: How did it not allow hat?
[15:03] <niemeyer> that
[15:04] <fwereade> niemeyer, I guess I'm missing something still... how did it allow for it?
[15:05] <niemeyer> fwereade: Can you please tell me what do you see as a blocker?
[15:06] <niemeyer> fwereade: I've been saying the same thing, so maybe if you tell me what's the limitation I can fix my explanation, or figure what I'm thinking wrong
[15:06] <fwereade> niemeyer, it seemed to be explicitly allowing for unvalidated *Configs to be floating around...
[15:06] <fwereade> niemeyer, I think I am happy with your latest suggestion
[15:06] <niemeyer> fwereade: "Floating around" is somewhat unprecise
[15:07] <niemeyer> fwereade: ValidateConfig does validation, it takes two *Config values
[15:07] <niemeyer> fwereade: So, necessarily, it is possible for an unvalidated *Config to exist before it is validated
[15:07] <niemeyer> fwereade: We do validation as soon as possible
[15:07] <fwereade> niemeyer, but what you suggested yesterday AFAICT indicated that you expected people to get *Configs from the config package, and to make sure they used ValidateConfig from the environs package, before they did anything with that *Config
[15:08] <niemeyer> fwereade: That's been the plan I'm trying to suggest since I first suggested a proper EnvironConfig type rather than interface
[15:08] <niemeyer> fwereade: Yes, that's what I'm still suggesting
[15:08] <niemeyer> fwereade: I also said we should verify as soon as possible
[15:08] <niemeyer> fwereade: Using that same mechanism
[15:09] <fwereade> niemeyer, huh, OK, I thought that by moving Read into environs we could guarantee that nobody saw a *Config that hadn't passed through Validate
[15:09] <niemeyer> fwereade: YES!
[15:09] <niemeyer> fwereade: Those two things do not conflict!
[15:09] <niemeyer> :-)
[15:09] <niemeyer> fwereade: "As soon as possible" may involve Read, and whatever
[15:10] <fwereade> niemeyer, ...but *without* Read in environs, we surely make it a lot easier to forget to validate?
[15:10] <fwereade> niemeyer, hence my discomfort
[15:11] <fwereade> niemeyer, anyway, thank you for bearing with me
[15:11] <niemeyer> fwereade: Sure, it was my mistake to suggest that in environs/config in the paste.. I clearly didn't think through
[15:11] <niemeyer> fwereade: This has always been my feeling, though, and I think you'll agree with me if you read back
[15:12] <fwereade> niemeyer, yeah, it was the code I was disagreeing with
[15:13] <fwereade> niemeyer, I had a strong feeling of us somehow being in violent agreement
[15:13] <niemeyer> fwereade: Phew, glad to hear it :)
[15:14] <fwereade> niemeyer, I think I conceded the validate-at-the-gate point yesterday, and it seemed to me that reading via environs was the only way to do it
[15:18] <niemeyer> fwereade: Well, I hope we do understand each other
[15:18] <niemeyer> fwereade: Because I'm still suggesting an EnvironConfig in the form that is in the paste
[15:18] <niemeyer> fwereade: and the use of environs/config for having it, so that we can use in State et al
[15:19] <niemeyer> fwereade: Reading it, and validating it, can be done within environs, though
[15:21] <niemeyer> fwereade: Is that what you understood from the agreemnet as well?
[15:21] <fwereade> niemeyer, the only tweak I would favour is to put the common fields in as fields and keep the provider-specific types in a map, but even that is not worth the time we'd spend debating (unless you immediately think it's a good idea ;))
[15:21] <fwereade> gahh not provider-specific *types*
[15:22] <fwereade> provider-specific attributes/settings/whatever-we-call-them
[15:22] <niemeyer> fwereade: I don't think it's a good idea only because state needs the full map
[15:22] <fwereade> niemeyer, we're copying the map anyway
[15:22] <fwereade> niemeyer, but I concede :)
[15:24] <niemeyer> fwereade: Well, I don't understand the suggestion then.. you suggested having only provider-specific types in the map, I said state needs the full map, you say ... ?
[15:25] <niemeyer> fwereade: I'd be happy with having an additional method, like CustomMap(), that excludes the well known fields
[15:25] <niemeyer> fwereade: I don't know if that's what you're suggesting?
[15:25] <fwereade> niemeyer, I say "we'll be implementing Map such that it copies, anyway, and the 5 lines of code to explicitly poke in the common field values is not going to be a ig deal"
[15:26] <niemeyer> fwereade: It sounds like too much trouble for little reason..
[15:26] <fwereade> niemeyer, like I said I don;t think the benefits outweigh the costs of talking about it
[15:26] <fwereade> ;)
[15:27] <niemeyer> fwereade: c.m.Get("default-series").(string) vs. 1) Do the same once; 2) Augment the struct; 3) Reinject the fields
[15:27] <niemeyer> fwereade: Sure, sorry..
[15:27] <niemeyer> fwereade: Something like Extras() seems like a good idea, though
[15:27] <niemeyer> fwereade: Returning a map of unknown fields
[15:27] <fwereade> niemeyer, oh, wait, one more thought... Secrets are an env-specific subset of Extras
[15:27] <niemeyer> fwereade: Yeah
[15:27] <fwereade> niemeyer, meh, we'll figure something out
[15:28] <fwereade> niemeyer, it's certainly not enough to invalidate your suggestion
[15:28] <niemeyer> fwereade: I think provider.ConfigSecrets(config) => <somethign> could do it
[15:29] <fwereade> niemeyer, we also want to be able to strip them out, but nowhere we want to do that won;t have a real Environ anyway
[15:29] <fwereade> niemeyer, there are certainly no architectural issues with it
[15:32]  * fwereade is happy
[15:33] <fwereade> niemeyer, thanks again for spending your weekend on this ;)
[15:33] <niemeyer> fwereade: No problem, and sorry for being unable to articulate the idea in a clearer way before
[15:34] <fwereade> niemeyer, it takes 2 to have a communication problem ;)
[15:34] <niemeyer> fwereade: Thanks :-)
[15:56] <niemeyer> fwereade: Heading out for lunch.. have a nice weekend in case we don't speak today again