| TheMue | morning | 06:44 |
|---|---|---|
| fwereade_ | TheMue, heyhey | 06:44 |
| TheMue | fwereade_: heya | 06:45 |
| davecheney | howdy | 06:48 |
| rog | fwereade_, TheMue: hiya | 07:20 |
| fwereade_ | rog, heyhey | 07:20 |
| TheMue | rog: hi | 07:21 |
| TheMue | rog: impl of environ config is - almost - done, i'm currently change the testing. here Config behaves different than ConfigNode | 07:21 |
| rog | TheMue: great! thanks for doing it. | 07:22 |
| TheMue | rog: one question here | 07:23 |
| TheMue | rog: is SetEnvironConfig intended to update or to replace the config? | 07:24 |
| rog | TheMue: yes, i believe so | 07:24 |
| rog | TheMue: for State anyway | 07:24 |
| rog | TheMue: it's not intended to replace ConfigNode in general, i think | 07:24 |
| TheMue | rog: no, only this one method. so the passed config replaces an existing one, ok. | 07:26 |
| rog | TheMue: oops, sorry i answered the wrong question | 07:26 |
| rog | TheMue: i *think* it should replace the config | 07:27 |
| rog | TheMue: as it's usually set all at once. but i'm not so sure | 07:27 |
| rog | TheMue: interesting question. | 07:27 |
| TheMue | rog: i think so too, otherwise by updating you only can add more values but never remove them | 07:27 |
| rog | TheMue: the issue i'm thinking is that if one client updates the environment settings and the other does an upgrade (setting the proposed version), then one or other of the changes may be lost. | 07:29 |
| TheMue | rog: cfg(a, b, c).update(cfg(b, c, d)) => cfg(a, b', c', d) | 07:29 |
| TheMue | rog: oh, yes, would be an issue too | 07:29 |
| TheMue | rog: but the same would be in case of an update by two clients | 07:30 |
| rog | TheMue: i'm not sure | 07:30 |
| rog | TheMue: what i'm worried about is: c := cfg(a=1); {c.set(a=2)} in parallel with {c.set(b=1)} | 07:31 |
| TheMue | rog: client a thinks b has to be b' and updates, a millisecond later client b thinks b (the old value he remembers) has to be b'' and updates too. the first update is lost. | 07:31 |
| rog | TheMue: that's true, but in this case we're dealing with disjoint sets of attributes. | 07:32 |
| TheMue_ | shit, network agaiin | 07:33 |
| TheMue_ | rog: ok, as long as keys are different an update mixing the value sets is ok. but updating the same key is still a problem. | 07:33 |
| rog | TheMue: the environ config that we set when changing provider attributes doesn't hold the proposed version attribute. | 07:33 |
| Aram | hello. | 07:34 |
| rog | TheMue_: of course, but that's inevitable | 07:34 |
| rog | Aram: hiya | 07:34 |
| TheMue_ | Aram: moin ;) | 07:34 |
| rog | TheMue_: it's reasonable that someone might want to do an upgrade at the same time as updating the provider attrinbutes | 07:34 |
| rog | TheMue_: i'm becoming less convinced that it's appropriate to lump the proposed version in with the provider attributes | 07:35 |
| TheMue_ | rog: a kind of txn.exec(func(cfg)) would be nice, so the whole change based on the current config executes in a txn | 07:36 |
| === TheMue_ is now known as TheMue | ||
| rog | TheMue_: actually, we do have that kind of thing. i think we might be ok. let me check. | 07:36 |
| TheMue | rog: i have to leave for some time in a few minutes, so you've time to rethink it | 07:37 |
| rog | TheMue: ok | 07:37 |
| rog | TheMue: i think it depends on how the API to changing environment settings works | 07:39 |
| rog | fwereade: do you know what mechanism the user will use to change provider settings in the state? (i'm presuming that the client is the only thing that will change them - is that right?) | 07:42 |
| TheMue | rog: let's talk about it later | 07:42 |
| * TheMue is now afk, bbl | 07:42 | |
| rog | TheMue: k | 07:42 |
| fwereade | rog, um, I think there's an env-set command planned | 08:00 |
| fwereade | rog, constraints will interact somehow; python already has set-constraints which allows for env-level settings | 08:01 |
| rog | fwereade: that's good. i don't think the user would expect attributes they don't provide on the command line to be deleted. | 08:01 |
| fwereade | rog, damn straight :) | 08:01 |
| rog | fwereade: but the SetEnvironConfig method kind of implies that | 08:02 |
| fwereade | rog, what's the justification for that method? | 08:02 |
| fwereade | rog, surely all we ever want is some form of update? | 08:02 |
| rog | fwereade: i *think* we already have something similar. but maybe not. | 08:03 |
| rog | fwereade: i agree. | 08:03 |
| fwereade | rog, what we originally had was just a ConfigNode, but I haven't been following the updates very closely | 08:03 |
| rog | fwereade: yeah. | 08:04 |
| rog | fwereade: we're going to replace that with a config.Config | 08:04 |
| fwereade | rog, ok, indeed, that's a good thing to get | 08:04 |
| rog | fwereade: but there's no way of setting attributes on a config.Config, so the interface will have to be SetEnvironConfig(*config.Config) | 08:04 |
| fwereade | rog, my heart still says we want an UpdateEnvironConfig method | 08:05 |
| rog | fwereade: and it seems perhaps a little surprising that if you do that, then no attributes will ever be deleted. | 08:05 |
| fwereade | rog, what attributes might we want to delete? | 08:05 |
| rog | fwereade: signature? | 08:05 |
| rog | fwereade: i don't think we do | 08:05 |
| fwereade | rog, hold on, a bell is ringing | 08:06 |
| fwereade | rog, maybe SetEnvironConfig *is* ok, if we can trust ourselves to build a provider-valid config before we call it | 08:06 |
| rog | fwereade: it's not ok if it does a replace not an update | 08:07 |
| fwereade | rog, ha, concurrent updates | 08:07 |
| rog | fwereade: yup | 08:07 |
| fwereade | rog, hm, it could still use a confignode internally and work right | 08:08 |
| rog | fwereade: not really. because we don't know what attributes to delete. | 08:08 |
| fwereade | rog, what's the use case for deleting attributes? | 08:08 |
| rog | fwereade: i think we should have UpdateEnvironConfig(*config.Config) and document that attributes are never deleted. | 08:09 |
| rog | fwereade: 'cos yeah i don't see a use case for deleting attributes. | 08:09 |
| fwereade | rog, sounds sane to me | 08:09 |
| rog | fwereade: are you planning to add more methods to UniterSuite? | 11:21 |
| rog | fwereade: more Test methods, that is | 11:21 |
| fwereade | rog, no, I think I'll just be adding table entries | 11:21 |
| rog | fwereade: currently we've got TestUniter only, but SetUpSuite writes stuff to VarDir. unfortunately JujuConnSuite sets up environs.VarDir on a per-test basis, not a per-suite basis | 11:22 |
| rog | fwereade: actually, perhaps i'll just do the tools building in SetUpTest instead of SetUpSuite. | 11:22 |
| rog | fwereade: it makes me wish for more flexible fixtures... | 11:23 |
| fwereade | rog, yeah, I know the feeling | 11:23 |
| fwereade | rog, I have a vague recollection of being aware of that and deciding it didn't matter -- I guess it does for what you're doing? | 11:23 |
| rog | fwereade: i'm wondering if fixtures should all conform to interface{SetUp(c *gocheck.C); TearDown(c *gocheck.C); Reset(c *gocheck.C)} | 11:24 |
| TheMue | re | 11:24 |
| rog | fwereade: JujuConnSuite did not set environs.VarDir before | 11:24 |
| TheMue | rog: i've just read that UpdateEnvironConfig() makes more sense? would be fine for me. | 11:24 |
| fwereade | rog, ah, ok | 11:24 |
| rog | TheMue: cool | 11:24 |
| fwereade | rog, I do rather like that potential fixture style, yes | 11:25 |
| rog | fwereade: i might run the idea past niemeyer some time | 11:25 |
| rog | fwereade: actually, it seems that the above interface would not be sufficient for the use case in UniterSuite. the JujuConnSuite is reset for every test in the table, but that makes a new VarDir; whereas we want to keep the same jujuc executable around for all the tests. | 11:44 |
| rog | fwereade: so actually i think that suite really does want to set VarDir itself. | 11:44 |
| rog | fwereade: (took me a while to realise what was going on!) | 11:45 |
| rog | note for the future: this is not a good idea: c.Logf("testlog: %q", c.GetTestLog()) | 11:57 |
| rog | i wondered why i was seeing sequences of 255 backslashes | 11:58 |
| rog | 256 presumably | 11:58 |
| fwereade | rog, huh, did I do that? | 12:01 |
| rog | fwereade: no, i did, trying to debug a failing UniterSuite test | 12:01 |
| fwereade | rog, ah, good, because I'm pretty sure I felt the temptation to the other day, was wondering if I'd blacked out and checked it in or something ;p | 12:02 |
| rog | fwereade: i'm sure the cause is something simple, but currently i can't see the wood for trees | 12:02 |
| rog | fwereade: do you have an instant gut feeling for what kind of thing might be causing this failure? http://paste.ubuntu.com/1173708/ | 12:11 |
| Aram | lsr | grep test | grep -v internal | grep -v confignode | grep -v life | xargs grep '(Update(Id)?)|(Find(Id)?)|(Insert(Id)?)|(Upsert(Id)?)' | 12:11 |
| Aram | oops | 12:11 |
| Aram | wrong focus | 12:11 |
| rog | fwereade: it's almost certainly to do with something in the environment that hasn't been set up correctly. | 12:12 |
| TheMue | rog: so, implementation and test in state are done, now check for not yet covered dependencies. had to fight with the schema for test ;) | 12:14 |
| rog | TheMue: cool. i'm struggling trying to merge trunk into a LGTM'd branch :-( | 12:26 |
| TheMue | rog: *lol* yeah, sometimes hard | 12:26 |
| rog | TheMue: the uniter tests are broken somehow, and it's too subtle for me to see yet | 12:27 |
| TheMue | rog: strange. no useful hints? | 12:27 |
| Aram | TheMue: thanks for the work you did on mstate, I merged all txn stuff and pushed it. | 12:28 |
| rog | TheMue: no clue yet. am delving to see what's actually going on in the tests. | 12:28 |
| TheMue | Aram: yeah, took already a look. it's doing great steps into the right direction. | 12:28 |
| fwereade | rog, sorry, I was having lunch | 12:53 |
| niemeyer | Gooood morning | 12:53 |
| rog | fwereade: np | 12:53 |
| rog | niemeyer: hiya | 12:53 |
| fwereade | rog, and, hum, I'd usually expect to see rather more in the logs than just that, looks almost like there's no uniter at all | 12:53 |
| fwereade | niemeyer, heyhey | 12:54 |
| fwereade | man, I think there's a thunderstorm on its way, I'm feeling ridiculously flat | 12:54 |
| rog | fwereade: it's failing in startupError, doing waitUnit for hook failed just after createUniter{} | 12:54 |
| fwereade | rog, yeah, I saw that much... and I didn't see any of the uniter logging at all in that paste | 12:55 |
| rog | fwereade: i changed the tests so that everything gets run with this function; that way i can see substeps: | 12:55 |
| rog | func step(c *C, ctx *context, s stepper) { | 12:55 |
| rog | c.Logf("%#v", s) | 12:55 |
| rog | s.step(c, ctx) | 12:55 |
| rog | } | 12:55 |
| fwereade | rog, ISTM like the uniter is failing before it even hits a mode | 12:56 |
| rog | fwereade: hmm, maybe the LoggingSuite isn't being hooked in right | 12:56 |
| fwereade | rog, otherwise I'd expect an "examining charm state..." | 12:56 |
| fwereade | rog, possibly something is up with the tools? | 12:57 |
| rog | fwereade: entirely possible | 12:57 |
| fwereade | rog, NewUniter calls esnureFs which calls EnsureTools | 12:57 |
| fwereade | rog, except hmm *that* surely ought to be caught :/ | 12:57 |
| fwereade | rog, yeah, startUniter checks that :/ | 12:58 |
| rog | fwereade: the logger seems to be hooked in ok | 12:58 |
| rog | fwereade: here's the full test output BTW: http://paste.ubuntu.com/1173763/ | 12:59 |
| fwereade | rog, all I can say is that something is very up, because test 3 at *least* ought to be giving you log output | 13:00 |
| rog | fwereade: what log messages should i be seeing? | 13:00 |
| * rog has an idea | 13:01 | |
| rog | ha! | 13:02 |
| TheMue | rog: is there a way to push an invalid value (not matching the schema) into config.Config? | 13:02 |
| rog | TheMue: which schema? | 13:02 |
| TheMue | rog: i have to invalidate it for a test | 13:02 |
| fwereade | rog, something more like http://paste.ubuntu.com/1173766/ | 13:03 |
| TheMue | rog: environ config | 13:03 |
| rog | fwereade: the log hooking is being disabled after Reset! | 13:03 |
| fwereade | rog, ahhhhh | 13:03 |
| TheMue | rog: i want to set name = 1, the provider test has done it before to see how the provider reacts | 13:03 |
| fwereade | rog, then that would definitely explain it :) | 13:03 |
| TheMue | rog: and the old usage of the config node allowed it to be so mean ;) | 13:04 |
| rog | TheMue: you'll have to go behind the scenes, i'm afraid | 13:04 |
| TheMue | rog: *sigh* | 13:04 |
| rog | TheMue: kinda the point of config.Config is that it always matches the schema | 13:04 |
| TheMue | rog: expected this answer, but hoped for an easier one :| | 13:05 |
| rog | fwereade: all tests pass. phew. thanks for your help. | 13:05 |
| TheMue | rog: need a kind of config.Evil(attrs) *Config, err | 13:06 |
| fwereade | rog, phew indeed :) | 13:06 |
| rog | fwereade: i'd let an extra line creep in when merging. | 13:06 |
| rog | fwereade: which happened to be LoggingSuite.TearDownTest | 13:06 |
| niemeyer | TheMue: Yo | 13:06 |
| fwereade | rog, ouch :) | 13:06 |
| niemeyer | TheMue: How's the EnvironConfig stuff? | 13:06 |
| TheMue | niemeyer: implemented in state and tested there, now i'm changing the dependencies | 13:06 |
| TheMue | niemeyer: hi btw | 13:07 |
| TheMue | niemeyer: and for the provider test i have to write an invalid config | 13:07 |
| niemeyer | TheMue: Superb | 13:08 |
| TheMue | niemeyer: i think i have to go directly to ZK here *sigh* | 13:09 |
| Aram | niemeyer: TheMue: I want to get rid completely of mgo in mstate_test. Tests should only rely on the public API and not implementation details. for test that HAVE to peek in the DB, I'd rather move them to a internal_test.go file or something. | 13:11 |
| niemeyer | Aram: +1 | 13:11 |
| TheMue | Aram: +1 | 13:11 |
| niemeyer | TheMue: FWIW, it's a good thing that you're finding it difficult to write down a broken config ;) | 13:12 |
| TheMue | niemeyer: yeah, indeed *lol* | 13:12 |
| TheMue | niemeyer: the usage of config.Config with coercing a schema makes it hard, and that's a good feature | 13:14 |
| rog | fwereade: could you have a glance at the uniter test changes in this CL, please, before i submit: https://codereview.appspot.com/6484051 | 13:16 |
| fwereade | rog, they look fine to me :) | 13:26 |
| rog | fwereade: thanks | 13:26 |
| fwereade | rog, I'm feeling rather uncomfortable about writing tests which check git output directly, but I can't think of another way to get, say, the status/log of a git repo | 13:28 |
| fwereade | rog, am I missing something obvious? :/ | 13:28 |
| rog | fwereade: i presume that's the only way, but i'm not familiar with git | 13:29 |
| rog | fwereade: there may be some API | 13:29 |
| fwereade | rog, hmm, seems that such things exist, but they look like a hassle | 13:31 |
| fwereade | ;) | 13:32 |
| niemeyer | Aram: How's the watcher going? | 13:33 |
| Aram | niemeyer: working on it, have a good model drawn on paper, expecting very good results. | 13:33 |
| niemeyer | Aram: Was there any issue with the proposal? | 13:34 |
| Aram | no. | 13:34 |
| Aram | at least I haven't found any. | 13:35 |
| niemeyer | Aram: Cool, was just wondering if the model on paper was different | 13:35 |
| rog | lunch | 13:52 |
| rog | niemeyer: can you think of any reason to retain the AgentToolsWatcher? | 14:11 |
| niemeyer | rog: No, I think EnvironConfig watcher should cover it | 14:12 |
| rog | niemeyer: yeah, i think so. i was wondering if we might have reason to watch the current tools of an agent, but i don't think so. just checking. | 14:12 |
| niemeyer | rog: I don't think so as well.. it's mostly informational | 14:13 |
| fwereade_ | niemeyer, I'm wiped out and need a break; if you have a moment, can I get a really casual pre-review on https://codereview.appspot.com/6488051 please? there's something not quite right about it: | 14:45 |
| fwereade_ | niemeyer, I'm starting to think I should drop charm.Manager entirely and move its functionality into Uniter itself | 14:46 |
| niemeyer | fwereade_: Interesting, what's the motivation that makes you feel that way? | 14:46 |
| fwereade_ | niemeyer, (also the tests are screwed up, but I'm hoping fresh eyes after a lie down will fix that, so you shouldn't have to worry too much there... | 14:46 |
| fwereade_ | niemeyer, I can't quite put my finger on it... I'm sort of hoping you'll take one look and say "why are you doing X" and I'll say "er, I don't know", and we'll all live happily ever after | 14:48 |
| niemeyer | fwereade_: LOL | 14:48 |
| fwereade_ | niemeyer, don't spend too much time on it, though | 14:49 |
| niemeyer | fwereade_: Cool, will check it out | 14:50 |
| fwereade_ | niemeyer, cheers | 14:50 |
| TheMue | niemeyer: most stuff for environ config is done, but provisioner (and here especially the tests) are a problem. dave also tested invalid environments. his impl today is relative friendly and retries based on the changes delivered by the watcher. but the new watcher stops on invalid environments, because config.New() thankfully doesn't except them. | 15:21 |
| niemeyer | TheMue: There is an important distinction between an invalid config.Config and a config.Config considered invalid by a provider | 15:22 |
| niemeyer | TheMue: The logic in the provisioner, and in fact in other workers too, should still consider a broken env and retry | 15:23 |
| TheMue | niemeyer: config.New() checks the schema and returns an error if it is invalid | 15:23 |
| niemeyer | TheMue: The firewaller should do that too, for example | 15:23 |
| niemeyer | <niemeyer> TheMue: There is an important distinction between an invalid config.Config and a config.Config considered invalid by a provider | 15:23 |
| TheMue | niemeyer: what should the watcher return in this case? a struct containing the config and an error, so that the receiver can check? would be easy, but leads to more changes in other places. | 15:25 |
| niemeyer | TheMue: If the watcher receives an invalid *config.Config*, it can drop it, and wait for the next change | 15:25 |
| niemeyer | TheMue: That said, the firewaller and the provisioner must still check for a config.Config considered invalid by the provider | 15:26 |
| TheMue | niemeyer: ah, good idea | 15:26 |
| niemeyer | TheMue: Because a valid config.Config is not necessarily a valid configuration for a specific provider | 15:26 |
| TheMue | niemeyer: yeah, i don't change that check | 15:26 |
| TheMue | so, it seems there's only one failing assert left. have to check why | 15:36 |
| TheMue | ah, landed, only one well known fail due to my locale (compared bzr message of store is wrong) | 15:48 |
| TheMue | dinnertime | 15:50 |
| niemeyer | fwereade_: I think I spotted some of your dislike there | 15:57 |
| niemeyer | TheMue: Enjoy! | 15:57 |
| fwereade_ | niemeyer, oh yes? | 16:03 |
| niemeyer | fwereade_: Yeah, I've sent a review | 16:05 |
| fwereade_ | niemeyer, sweet, tyvm | 16:05 |
| niemeyer | fwereade_: np! | 16:06 |
| * niemeyer steps out for lunch | 16:06 | |
| fwereade_ | niemeyer, yeah, good food for thought -- thanks :) | 16:07 |
| rog | niemeyer: ping | 17:37 |
| rog | fwereade_: ping | 17:37 |
| niemeyer | rog: Pongus | 17:50 |
| rog | niemeyer: just wanted to have a word about the strictness of provider config attributes | 17:50 |
| niemeyer | rog: Ok | 17:51 |
| rog | niemeyer: you said yesterday that they shouldn't be strict, but we had tests specifically checking for strictness. just wanted to check that it's ok to relax the checks. | 17:51 |
| rog | niemeyer: i've proposed the CL anyway. | 17:51 |
| niemeyer | rog: I'm not sure about what this is about | 17:51 |
| rog | niemeyer: EnvironConfig | 17:51 |
| niemeyer | rog: I don't recall saying anything about strictness | 17:51 |
| rog | [16:05:25] <niemeyer> rog: No provider should break if config.COnfig has an attribute that it doesn't know about | 17:52 |
| niemeyer | rog: Yeah, that's still the case | 17:53 |
| rog | niemeyer: but at least one provider *was* breaking when it saw an unknown attribute | 17:53 |
| rog | niemeyer: (and we had a test to check it) | 17:53 |
| niemeyer | rog: I can't imagine how that would have happened.. they don't validate the config.Config internals | 17:53 |
| rog | niemeyer: environs.Provider.SetConfig | 17:54 |
| niemeyer | rog: ? | 17:54 |
| rog | niemeyer: the provider sees all the attributes in the config | 17:54 |
| rog | niemeyer: including (now) agent-version | 17:54 |
| niemeyer | rog: I said something else | 17:55 |
| niemeyer | rog: They don't validate the config.Config internals | 17:55 |
| niemeyer | rog: What is breaking, more precisely/ | 17:55 |
| niemeyer | ? | 17:55 |
| rog | niemeyer: your comment didn't mention internals. | 17:56 |
| niemeyer | rog: config.New validates itself | 17:56 |
| niemeyer | <niemeyer> rog: I can't imagine how that would have happened.. they don't validate the config.Config internals | 17:56 |
| rog | niemeyer: they validate the attributes they get from the config | 17:56 |
| rog | niemeyer: SetConfig was failing | 17:56 |
| niemeyer | rog: What is breaking, more precisely? | 17:56 |
| niemeyer | rog: file and line, please | 17:56 |
| rog | niemeyer: well, the symptom i saw was that the provisioner failed to make a valid environment | 17:57 |
| rog | niemeyer: the cause was: | 17:57 |
| rog | niemeyer: environs/dummy/environs.go, checker.Coerce | 17:58 |
| rog | niemeyer: (in Validate) | 17:59 |
| niemeyer | rog: There are no general attributes in there | 17:59 |
| niemeyer | rog: The file and line where an environment validates the general config.Config attributes, please? | 17:59 |
| rog | niemeyer: ah, should config.Config know about agent-version? | 17:59 |
| niemeyer | rog: Yes | 17:59 |
| rog | niemeyer: ok | 17:59 |
| rog | niemeyer: that makes sense. | 18:00 |
| niemeyer | rog: Sweet | 18:00 |
| rog | niemeyer: ok, thanks. i'll mark that branch as WIP | 18:01 |
| niemeyer | rog: np | 18:01 |
| rog | niemeyer: gotta go now. see you tomorrow. | 18:02 |
| niemeyer | rog: have a good time | 18:02 |
| rog | niemeyer: have fun | 18:02 |
| niemeyer | rog: Thanks | 18:02 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!