/srv/irclogs.ubuntu.com/2012/10/10/#juju-dev.txt

davecheneyWHAT00:36
davecheney% juju status00:36
davecheneyerror: Get https://s3.amazonaws.com/juju-4218c46225cb7856d9d5ca3c1a685cd87b647996/provider-state: lookup s3.amazonaws.com: no such host00:36
davecheneyinternets is whack!00:36
davecheneyamazon is sooooo slow today00:43
davecheneyniemeyer: % error: cannot assign unit "tomcat7/0" to machine: cannot assign unit "tomcat7/0" to unused machine: duplicate key insert for unique index of capped collection01:12
davecheneyhappens a lot01:12
davecheneyis this related to the version of mongo i'm running01:12
SpamapSdavecheney: shouldn't you be hitting ap-southeast-1 ?01:35
davecheneySpamapS: doesn't make any difference01:36
davecheneys3 is still slow as shit01:36
niemeyerdavecheney: Yeah, new mgo solves that02:40
davecheneyniemeyer: hmm, maybe i didn't go get02:54
davecheneywill double chekc02:54
davecheneyno, i must have had too, otherwise it wouldn't compile02:55
niemeyerdavecheney: It wouldn't?02:56
davecheneydidn't you introduce a new symbol in the latest mgo ?02:57
davecheneyand we use that in juju02:57
niemeyerdavecheney: I introduced a symbol before the fix to the problem above was released02:58
davecheneyok02:59
davecheneyniemeyer: i've recompiled02:59
davecheneythanks02:59
niemeyerdavecheney: and mgo will still compile file even if juju lacks the fix to the problem above02:59
davecheneyniemeyer: welcome to the unspoken dependency problem in go02:59
niemeyerdavecheney: Yeah, it's kind of a general problem02:59
niemeyerdavecheney: Also one of the reasons why I don't do interim announcements/releases03:00
davecheneyniemeyer: yeah, there is no good way for people to _know_ they have the right release03:00
davecheneyi've worked around it in juju by adding tests which break when we've added something to a dep, like the --format= stuff in gnuargs03:01
davecheneybut that isn't really a solution03:01
niemeyerdavecheney: Yeah, not a general one at least.. we can easily add a test against a specific version, though03:03
niemeyerAssert(mgo.Version, Equals, Version)03:03
niemeyerdavecheney: Assuming mgo.Version exists, which it doesn't at the moment03:04
niemeyerAnyway, I'm in serious need of some sleep03:04
davecheneyniemeyer: later03:04
niemeyerIt was a long day, both with great stuff and with unusually awkward stuff03:05
niemeyerdavecheney: Have a good day there03:05
davecheneyniemeyer: will have the charm audit done by this arvo03:06
davecheneyam down to s03:06
davecheneywith a larger sample size, the numbers are looking better03:06
davecheneyfewer charms broken as a % of the total03:06
davecheneyso, some intersting problems with subordinate charms05:15
davecheneythey tend to clobber each other's apt-get requests05:16
davecheneyand fail05:16
davecheneyand this is what happens when you start too many instances05:22
davecheney  20:05:22
davecheney    instance-id: pending05:22
davecheney  21:05:22
davecheney    instance-id: pending05:22
davecheney  22:05:22
davecheney    instance-id: pending05:22
davecheneylucky(~) % grep -c -- '- failed' ~/charm-audit.txt06:15
davecheney2406:15
davecheneylucky(~) % grep -c -- '- started' ~/charm-audit.txt06:15
davecheney5406:15
davecheneylucky(~) % grep -c -- '- unknown' ~/charm-audit.txt06:15
davecheney506:15
davecheneywill post full report tonight.06:15
SpamapSdavecheney: I would not expect subordinates to install in parallel08:12
fwereadedavecheney, how are you installing subordinates in the first place? :)08:16
davecheneyfwereade: juju deply08:20
davecheneythere are a few charms that don't do anyting08:20
davecheneythe juju charm for example08:21
davecheneybut others happily cohabitate08:21
fwereadedavecheney, still don't see how that'd work -- what's deploying them?08:22
davecheneyfwereade: try it yourself08:23
davecheneyhappy uniter is happy08:23
fwereadedavecheney, that is pleasing, but... the uniter can't deploy new units yet, afaik, and the MA shoudln't be deploying subordinate units... should it?08:24
davecheneyfwereade: maybe it can't tell08:25
fwereadedavecheney, but juju deploy exits early, before adding units, with subordinate charms08:26
davecheneyfwereade: ahh08:27
davecheneyi was mistaken08:28
davecheneyjuju is a subordinate08:28
fwereadedavecheney, and Service.AddUnit refuses to add them anyway :)08:28
davecheneybut charms like hadoop-master and hadoop-slave are cohabitatin08:28
wrtpdavecheney, fwereade: morning08:28
fwereadedavecheney, ah, cool... so you have hacked placements or something?08:29
fwereadewrtp, heyhey08:29
davecheneyfwereade: nope08:29
davecheneythat is just what happens08:29
fwereadedavecheney, I has a suspicious08:29
fwereadedavecheney, but well, hey, at least we're running them, even if it's not clear tome how ;p08:29
SpamapSsounds like the placement policy of "unused" machines isn't being respected08:30
fwereadeSpamapS, yeah...08:30
SpamapSwhich would actually make quite a few people very happy ;)08:30
fwereadeSpamapS, but then lots of things *do* end up on their own machines (right, davecheney?)08:30
SpamapS(even if it is dangerous and error prone)08:30
fwereadeSpamapS, haha, yeah08:30
davecheneySpamapS: fwereade it could be related to the charm names08:30
davecheneyit only happens to charms that have the same prefix08:30
fwereadedavecheney, whoa, wtf?08:31
davecheneyfwereade: try it08:31
fwereadedavecheney, so cs:precise/hadoop-master and cs:precise/hadoop-slave end up cohabiting?08:31
davecheneyyup08:32
davecheneytwo unit agents08:32
davecheneyrunning at the same time08:32
davecheneyfighting over the apt lock08:32
SpamapShttp://blog.ancientlasers.com/wp-content/uploads/2012/03/Mind-Blown.jpg08:32
fwereadedavecheney, hmm, this feels rather like a bug with a capital B, and probably capital U and G as well08:33
SpamapSdavecheney: the apt thing is a problem when python unit agents co-habitate as well.. charms tend to be written assuming they're "masters of their own domain"08:33
davecheneySpamapS: intersting08:34
davecheneyisn't their a flag to apt to say 'please wait, forever if that is what it takes'08:34
SpamapSnot that I know of08:36
SpamapSbut I believe you can use aptd for that08:36
davecheneymaybe i'm thinking of yum08:36
* davecheney slaps himself08:36
SpamapSdavecheney: either way, its not just apt-get ... there may be other ordering issues08:38
SpamapSdavecheney: if two things want to rearrange the services running right now.. the stops/starts may happen in a very wrong order.. for instance08:38
SpamapSif you're going to have more than 1 unit per "container", you have to serialize all hook execution08:39
* SpamapS really should go try to sleep again08:40
davecheneySpamapS: more investigation is called for08:41
SpamapSdavecheney: I'm still quite surprised that charms with similar names are being chosen to live on one machine08:42
davecheneySpamapS: it is possibly user error08:43
fwereadewrtp, I think I have a fix for lp:1065576 (the git problem you found yesterday)... can you remind me what the standard I-fixed-a-bug summary line should look like?09:01
wrtpfwereade: pkgname: fix git so it doesn't do X09:02
wrtpfwereade: Fixes bug NNN09:02
fwereadewrtp, ah, I thought there was meant to be a bug ref in the summary09:02
fwereadewrtp, cheers09:02
wrtpfwereade: i don't think so09:03
wrtpfwereade: but you can use --bug with lbox09:03
wrtpfwereade: (and you probably should)09:03
fwereadewrtp, last time I tried that it overwrote the description so I steered well clear09:03
wrtpfwereade: fairy nuff09:03
fwereadewrtp, https://codereview.appspot.com/6636056 should be a trivial, I think09:05
fwereadebbs09:06
wrtpfwereade: it looks plausible, but i don't really know what's going on here. what does "a hook has run and is being committed by git" mean? how are hooks committed by git?09:08
Arammoin.09:22
fwereadewrtp, b09:24
fwereadeAram, morning09:24
fwereadewrtp, we snapshot dir state at the end of each hook09:24
wrtpfwereade: which dir?09:25
fwereadewrtp, so git can be already running something in the uniter when we try to do status09:25
fwereadewrtp, the charm dir09:25
wrtpfwereade: wow. isn't that a bit... heavyweight?09:25
fwereadewrtp, on discussion a good while ago, it seemed to generally be considered to be a good thing09:26
wrtpfwereade: doesn't that mean that if you've got a large charm, it's read in its entirety on each hook?09:27
wrtpfwereade: i'm sure i'm missing something. what's this in aid of?09:27
fwereadewrtp, git has always seemed to me to be pretty efficient09:27
fwereadewrtp, what is it you're worried about?09:27
wrtpfwereade: i suppose if we trust mtimes, it won't be too bad.09:28
wrtpfwereade: i thought the charm directory was usually immutable actually09:28
fwereadewrtp, huh? the charm dir is anything but09:29
fwereadewrtp, IMO this is a serious drawback09:29
wrtpfwereade: you mean the directory containing the charm hooks &c ?09:29
fwereadewrtp, yeah, the deployemtn dir09:29
wrtpfwereade: interesting. so what does this enable?09:31
wrtpfwereade: i hope we don't have charms that put large database files in the charm directory.09:32
wrtpfwereade: because that could make things seriously slow (and we could use lots of disk space too)09:33
fwereadewrtp, mainly it's intended to help us figure out wtf has been going on when things go wrong09:34
fwereadewrtp, AIUI it's charm state that tends to get put in there09:34
fwereadewrtp, I don't think people are going to be installing postgres inside the charm dir, for example09:35
wrtpfwereade: if they do, we'll be in for a nasty surprise :-)09:35
wrtps/we/they/09:35
fwereadewrtp, if they do they're fucked whatever we do09:35
fwereadewrtp, because charm upgrades will do the snapshots anyway09:36
fwereadewrtp, this way, if people are messing with the charm dir other than in hooks, at least we find out fast09:36
wrtpfwereade: there's a difference between snapshotting once on upgrade, and snapshotting on every hook09:36
wrtpfwereade: what's wrong with messing with the charm dir other than in hooks?09:37
fwereadewrtp, how do you tell a hok isn't messing with the charm dir itself?09:37
wrtpfwereade: when i wrote a charm before, the charm dir seemed to be the only piece of space that was the charm's own. so it seemed logical to put mutable data files there.09:37
wrtpfwereade: i think you're saying that's frowned on09:38
fwereadewrtp, depends how they're mutated09:38
wrtpfwereade: it didn't depend on anything before AFAIK09:38
fwereadewrtp, if you've got, say, a cron job messing with the charm dir then, yeah, that's not sensible09:38
wrtpfwereade: why not?09:38
wrtpfwereade: do we document that?09:38
fwereadewrtp, I have no idea09:39
fwereadewrtp, (why we don't document it)09:39
wrtpfwereade: it makes me a little anxious09:39
fwereadewrtp, (*whether* we document it)09:39
wrtpfwereade: because we're saying "of course you shouldn't do *that*", but there's nothing that ever told anyone that it might be a bad idea (and it certainly seemed like a reasonable idea to me back when)09:40
fwereadewrtp, I don't see how it could ever be considered sane to mutate the charm dir from outside a hook at the same time a hook is running09:40
fwereadewrtp, not knowing when a hook is running, which you don't, would STM to preclude the sanity of touching the charm dir other than through juju09:41
wrtpfwereade: why not? say you're implementing a subordinate. you run a database of some kind; where do you put the data files for it? putting them in the charm dir seems kinda reasonable to me.09:41
fwereadewrtp, woudl you consider it sensible to change DB files youself, from outside the DB, while the DB was running?09:41
wrtpfwereade: no, but the DB will change its files while the hooks are running and while they're not, right?09:41
fwereadewrtp, how is that situation any different?09:41
wrtpfwereade: there are files that matter to us in the charm dir, and files that don't09:42
fwereadewrtp, I would personally put the data files somewhere in var09:42
fwereadewrtp, oh really? how do we tell what isn't important?09:42
fwereadewrtp, you seem to be assuming perfect knowledge of all current and future possible charm upgrades...09:43
fwereadewrtp, if you want to keep some charm-related state in the charm dir, and manipulate it in the hooks, great09:44
fwereadewrtp, if you want to install everything from the service in a non-standard place, on your own head be it ;)09:45
wrtpfwereade: if the user creates a "data" dir in their charm dir and uses it for some running database (that's similar to what i did), i don't see that should have untoward effects09:45
wrtpfwereade: but we're going to break that09:45
fwereadewrtp, I don't see how that's possible09:45
fwereadewrtp, you can't do that at all ever anyway, can you?09:46
wrtpfwereade: huh?09:46
fwereadewrtp, (without it being risky/broken, anyway)09:46
wrtpfwereade: why's that?09:46
fwereadewrtp, how do you handle concurrent writes to the charm dir?09:46
fwereadewrtp, or writes/reads, whatever09:46
wrtpfwereade: how does who handle it?09:47
wrtpfwereade: the running database?09:47
wrtpfwereade: or juju?09:47
fwereadewrtp, how do you propose to implement this scheme whereby juju, and something-else, are both writing concurrently to the same dir without errors?09:48
wrtpfwereade: two things can happily write to two independent files without errors. i'm not sure i see the problem.09:49
fwereadewrtp, and how do you propose to ensure this?09:49
wrtpfwereade: i must be missing something here. we all share the same filesystem.09:49
wrtpfwereade: are you thinking about upgrades in particular here?09:50
fwereadewrtp, I'm thinking about charm upgrades, and every hook09:50
fwereadewrtp, all these things can write arbitrary things into the charm dir09:50
fwereadewrtp, you're apparently saying that we should be able to guarantee that *anything* else can *also* write arbitrarily to the charm dir, at any time, without anything ever goind wrong09:51
wrtpfwereade: the things written by hooks are under the control of the charm author. as are things written by programs running in the bg, started by the hooks.09:51
wrtpfwereade: not really. i'm saying that the charm dir is just a dir. we can have two independent things writing to files in it with no problem, just the same way that many programs can write to independent files under the global root.09:52
wrtpfwereade: the difficulty here is that we're assuming that juju has entire control of the charm dir, but i'm not sure that's necessarily the case.09:53
fwereadewrtp, I think you're saying that you can guarantee that the written files will be independent09:53
fwereadewrtp, which STM to be an unsupportable assertion09:53
wrtpfwereade: if i start a database and give it a file to write to, or a directory that i've created for it to put its data files in, i know that it's not going to write outside that file or dir.09:54
wrtpfwereade: how is the charm dir different from, say, /tmp ?09:54
fwereadewrtp, the charm dir is owned by juju, and juju expects to have control over it?09:55
wrtpfwereade: (FWIW, i'm not saying that what we're doing now with git snapshots is *wrong*, but i think it's worth exploring the implications)09:55
fwereadewrtp, would you install mysql inside postgres' data dir?09:55
wrtpfwereade: the problem is that the charm author might feel *they* have control over the charm dir09:56
fwereadewrtp, they have control over it, strictly mediated by juju09:56
wrtpfwereade: after all, they've created the files and populated the directories09:56
wrtpfwereade: it's not strictly mediated...09:56
wrtpfwereade: and i wouldn't be surprised if lots of charms used it for general scratch space09:57
fwereadewrtp, this STM like a documentation issue, fundamentally09:59
wrtpfwereade: and if they do, i we may have a problem09:59
wrtpfwereade: sure09:59
wrtpfwereade: it's also a potential backward-compatibility issue09:59
fwereadewrtp, IMO it's a latent-charm-bug issue10:00
wrtpfwereade: out of interest, is a charm allowed to change its hooks (e.g. to add a new relation-joined hook) on the fly?10:00
fwereadewrtp, I don;t think we ever claimed remotely that it was safe to modify juju's dirs while juju is running10:00
wrtpfwereade: it's only a bug because we've made it a bug10:01
fwereadewrtp, nothing stopping it from doing so10:01
wrtpfwereade: so it doesn't cache the current hooks then?10:01
fwereadewrtp, assuming that it's fine to mess with something's runtime state, just because nobody told you not to, doesn't seem very sensible to me10:01
fwereadewrtp, no, it just executes whatever's there10:02
wrtpfwereade:  we never claimed remotely that it *wasn't* safe to modify juju's dirs while juju is running AFAIK :-)10:02
fwereadewrtp, what makes you think that is a reasonable assumption?10:02
wrtpfwereade: it's not *something's* runtime state. it's a *charm's* runtime state. so it seems not unreasonable for a charm to assume that it can change it whenever it likes.10:03
wrtpfwereade: why not?10:03
fwereadewrtp, 5 seconds of thinking would surely reveal that juju has the power to make arbitrary changes to that dir, and lead one to look for mechanisms by which that operation can be made safe, and to conclude from the lack of such mechanisms that it is in fact not safe10:04
fwereadewrtp, details like "no, you cannot run juju tools outside hooks" seem to support this10:04
wrtpfwereade: is there not a hook that's run before an upgrade?10:05
fwereadewrtp, nope10:06
fwereadewrtp, that might also be considered a hint10:06
wrtpfwereade: that seems wrong actually10:06
fwereadewrtp, hmm, how would it help?10:07
wrtpfwereade: ISTM that a charm ought to be able to have the opportunity to shut things down cleanly before an upgrade if it wants to10:07
fwereadewrtp, whoa, why would upgrading the charm remotely justify screwing up the service?10:07
wrtpfwereade: depends on the service.10:08
fwereadewrtp, if, say, a config change causes us to want to change *service* version, then we can deal with that10:08
fwereadewrtp, conflating service version and charm version seems to be a fundamental error10:09
wrtpfwereade: there is a *post*-upgrade hook, right?10:10
fwereadewrtp, yes10:11
fwereadewrtp, AIUI this is essentially to give you the opportunity to rearrange the files inside the charm dir if required10:12
wrtpfwereade: well, you might also need to restart services, etc10:12
fwereadewrtp, what does a charm change matter to the service?10:13
wrtpfwereade: i'm talking about daemons, not juju services, sorry10:13
fwereadewrtp, yeah: why would a charm change justify such an action?10:14
fwereadewrtp, a config change might10:14
wrtpfwereade: what if the charm change is to make a daemon start with different command-line arguments?10:14
wrtpfwereade: or to use an alternative daemon for something10:15
wrtpfwereade: there are many possible reasons a charm might be upgraded10:15
fwereadewrtp, ok, fair enough -- then you can do that just fine in the post-upgrade hook with a minimum of downtime10:15
fwereadewrtp, what good would a pre-upgrade hook do?10:15
=== TheMue_ is now known as TheMue
fwereadewrtp, although, more likely, you'll just do that in the subsequent config-changed anyway10:16
wrtpfwereade: ISTM that people may have daemons that look at files in the charm dir. if the charm dir can change without warning, that's not a good idea.10:16
fwereadewrtp, I agree that that is not a good idea10:16
wrtpfwereade: it's probably not a good idea anyway, but i bet lots of people do it10:17
fwereadewrtp, that is why people should not do it10:17
fwereadewrtp, the charm dir is for the charm10:17
fwereadewrtp, the service already has a standard place for everything, surely?10:17
wrtpfwereade: i see that. but what *you* think of as the charm is not what others might think of as the charm10:18
wrtpfwereade: what place is that?10:18
wrtpone mo, gotta do the bins10:18
fwereadewrtp, service-dependent, surely?10:18
fwereadewrtp, but the general philosophy is that we should be able to completely nuke juju, and leave the service running unhindered10:19
fwereadewrtp, if the service knows about juju, the charm is being bad10:19
fwereadewrtp, brb ciggie10:21
wrtpfwereade: is there a way for a charm to find out its service name?10:27
wrtpfwereade: because you might easily have two of the same charm deployed in the same container. then each one needs some space for itself. the charm dir might seem like a good place for that, but i see why you think it's not.10:30
wrtpfwereade: i wonder if we should provide each charm with its own temporary directory that it can do whatever it likes to.10:30
wrtpfwereade: then deprecate the whole idea of writing to the charm dir, whether inside or outside a hook.10:31
fwereadewrtp, +1 on that, but I think it's independent of the don't-let-the-service-know-about-the-charm issue10:32
wrtpfwereade: i was responding to your "the service already has a standard place for everything, surely?" - that's not necessarily true10:33
wrtpfwereade: but i agree with your sentiment in general10:34
fwereadewrtp, by convention, if nothing else10:34
fwereadewrtp, if you're installing X, you tend not to put X's data files inside Y's data dir10:35
wrtpfwereade: what if you're installing two Xs?10:35
wrtpfwereade: we really really really need to write some decent docs10:35
fwereadewrtp, agreed there :)10:36
wrtpfwereade: that was a genuine question above BTW; is there a way for a charm to find out its service name?10:36
fwereadewrtp, not directly, but it knows its unit name10:37
wrtpfwereade: ah, that's good enough then.10:37
wrtpfwereade: how does it find its own unit name?10:37
fwereadewrtp, JUJU_UNIT_NAME I think10:37
fwereadewrtp, yep10:38
fwereadewrtp, what would you expect to need it for?10:38
fwereadewrtp, (the service name, I mean)10:38
wrtpfwereade: it's a useful disambiguator. if you're a subordinate service, you want to choose a place to store your charm-external state that doesn't clash with other subordinates that are potentially using the same charm.10:39
fwereadewrtp, I'd expect the relation id to be the disambiguator, but maybe that's not right10:40
wrtpfwereade: there may be no relation10:41
fwereadewrtp, how do you deploy a subordinate without a relation?10:41
wrtpfwereade: i suppose for subordinates, there's always a relation10:41
wrtpfwereade: but do you know that relation id when you're executing the install hook?10:42
fwereadewrtp, shouldn't the install hook just, er, install?10:42
wrtpfwereade: it might need to install to some charm-external directory. same applies to the start hook.10:43
fwereadewrtp, I'm not sure that we're trying to solve the run-multiple-versions-of-the-same-software problem here are we?10:43
wrtpfwereade: it's not hard to run multiple versions of the same software; all we need is a directory that each instance can call its own.10:44
wrtpfwereade: we allow several subordinate services in a given container that might use the same charm. i can imagine cases where that might be very useful.10:44
wrtpfwereade: (i'm imagining a subordinate service that provides some fairly general functionality, determined by the config settings)10:45
fwereadewrtp, then, in that case, surely the unit name is a reasonable disambiguator there10:45
wrtpfwereade: indeed10:46
fwereadewrtp, and the relation id is good for handling multiple relations run by the same charm10:46
wrtpfwereade: yeah10:46
* fwereade once again fails to successfully use wrtp's fly-killing techniques11:50
fwereadebut it was *close* that time11:50
wrtpfwereade: you'll get there :-)11:50
wrtpfwereade: did it escape through the top or the bottom?11:50
fwereadewrtp, bottom11:50
wrtpfwereade: keep trying...11:51
fwereadewrtp, really impressively sharp turn away actually, it probably deserved to live11:51
wrtpfwereade: you're selecting for canny flies11:51
wrtpfwereade: a few more generations and you'll never get 'em11:51
fwereadewrtp, I'm not saying I won't try again next time it comes in range ;)11:51
wrtpcrack crack crackity crack12:11
* wrtp has just found out why his tests were passing when they couldn't possibly be passing12:12
wrtpniemeyer: morning12:42
wrtp!12:42
niemeyerGood morning!12:42
wrtpniemeyer: my use-passwords branch last night was a bit crackful i'm afraid. i forgot to pass the --auth flag to the mongod started by environs/cloudinit.12:44
fwereadeniemeyer, heyhey12:45
wrtpniemeyer: i thought of something this morning and could not for the life of me think how the tests were passing12:45
wrtpniemeyer: and that's the reason...12:45
niemeyerwrtp: Hah12:46
niemeyerfwereade: yo12:46
wrtpniemeyer: i thought it all went a bit too smoothly!12:46
niemeyerAram: ping12:54
Aramyo12:54
Aramhello12:54
Aramniemeyer: ^ ^12:55
niemeyerAram: Heya!12:55
niemeyerAram: Good point regarding the check12:55
niemeyerAram: I suspect we'll need to verify if it's already in the pending list or not in those removal detection branches then12:56
Aramniemeyer: perhaps it's still a good idea to check if pending is required at the begining of the logic.12:56
Aramso we do it only once12:56
Aramand then depending on that boolean and the rest of the logic determine what to actual returnm12:57
Aramdoes that make sense?12:57
Aramalteratively, we could do it once at the end.12:57
Arambut I think logic is simpler if we do it at the begining.12:57
niemeyerAram: I think having this logic in a small method would be as nice, and perhaps more clear12:58
Aramgood idea.12:58
niemeyerAram: hasString and hasInt can probably be shared by all watchers13:00
Aramyes13:00
fwereadehmm, maybe I should eat something, bbiab13:07
niemeyerfwereade: Sounds wise13:07
wrtpniemeyer: fairly trivial: https://codereview.appspot.com/664105313:15
wrtpoops, the branch is misnamed13:16
wrtpniemeyer: renamed and reproposed13:17
wrtphttps://codereview.appspot.com/663506213:17
niemeyerwrtp: On a call with flacoste.. back in a bit13:23
wrtpniemeyer: np13:23
=== andrewsmedina is now known as fssparademimipor
niemeyerwrtp: Very ncie14:01
niemeyernice14:01
wrtpniemeyer: thanks14:06
wrtpniemeyer: another fairly small one: https://codereview.appspot.com/660406114:12
niemeyerwrtp: LGTM14:14
wrtpniemeyer: lovely, thanks14:14
=== fssparademimipor is now known as andrewsmedina
niemeyerConnection Timeout: disconnecting client after 300.0 seconds14:49
niemeyerbzr: broken pipe14:49
niemeyerAm I the only one facing those issues?14:49
fwereadeniemeyer, I think so -- I did notice slightly greater than usual flakiness earlier this week but that's it14:50
niemeyerfwereade: Weird.. I can't really guess what could be causing it14:52
niemeyerEverything else seems to work normally14:52
wrtpAram: i just saw this test failure. looks like something's not sorting the unit names as expected: http://paste.ubuntu.com/1271309/15:07
Aramwrtp: sorry, my fault.15:08
Aramwill fix right away15:08
wrtpAram: thanks!15:08
* wrtp only discovered this morning that you can do tail -f * and it works how you'd want.15:13
wrtpniemeyer: next step: https://codereview.appspot.com/661507115:15
Aramwrtp: quick trivial so I fix the build: https://codereview.appspot.com/6625075/15:17
wrtpAram: LGTM15:17
Aramwrtp: sorry for the breakage, in my defense I always run the state tests 20 times before submitting, and it worked here.\15:18
fwereadeniemeyer, can you think of any reason for me not to move uniter.HookContext to hook.Context? I've come to the conclusion that RelationContext is actually fine as it is -- it's Relationer that really needs the big changes, and it shouldn't be affected by this15:19
niemeyerwrtp: done15:21
niemeyerfwereade: RelationContext or ContextRelation?15:21
fwereadeniemeyer, ha, ContextRelation, I'm pretty sure I've changed the name throughout, but my brain hasn't synced up yet15:22
fwereadeniemeyer, (ContextRelation would also move)15:22
niemeyerfwereade: I don't have the context to tell whether a package makes sense, but I can try to help: why do you think a package makes sense? You've been going back and forth in other cases; what's the criteria you've used on those?15:22
fwereadeniemeyer, I'm planning to move it into the existing hook package15:23
niemeyerfwereade: Is that an answer to the above question?15:25
fwereadeniemeyer, not really, still composing15:25
fwereadeniemeyer, I moved it to uniter originally because I was sure that was "closer" to the "right place" for it, because there was clearly some overlap between Context and Relationer15:26
fwereadeniemeyer, it's probably not justified yet though15:27
niemeyerfwereade: My feeling is that Context will be strongly bound to the uniter data, but I don't really know yet15:28
fwereadeniemeyer, I *think* that with something like the following, uniter doesn't need to know about Context at all -- does this just look awful to you? http://paste.ubuntu.com/1271366/15:39
fwereadeniemeyer, the trouble is that putting that in uniter seems wrong, but I can't really put it in hook if Context (et al) aren't there15:42
wrtpniemeyer, fwereade: what do you think about log.Printf messages, and whether/how they should be prefixed?15:44
wrtpthe first log messages i did (in environs/ec2, for example) are prefixed with the package name.15:45
wrtpTheMue, Aram: any thoughts?15:45
fwereadewrtp, last time I prefixed normal log messages with the package, niemeyer suggested I remove them; but I'm not sure we have much in the way of conventions15:46
TheMuewrtp: my personal logging package has the package es prefix in braces, and that's helpful15:46
wrtpi think that some prefix is nice to have, as it makes it obvious where the messages are coming from15:46
TheMuewrtp: so i don't have to think about, it's done automatically15:46
wrtpfwereade: my thought has been: error messages have no prefix; printed messages get a prefix.15:46
wrtpTheMue: that's a reasonable approach actually15:47
wrtpTheMue: assuming we remove the launchpad.net/juju-core/ part of the package path15:48
fwereadewrtp, doesn't sound unreasonable, but don't really have strong feelings15:48
fwereadebrb15:48
TheMuewrtp: my package currently shows the full path, but yes, that should be configurable (as well as the logging level)15:48
TheMuewrtp: see https://code.google.com/p/tcgl/source/browse/applog/applog.go15:49
TheMuewrtp: Debugf() also shows filename, function name and line number15:51
wrtpTheMue: tbh i think that's a bit heavyweight for what we want.15:52
TheMuewrtp: sure, that's my personal approach. but the automatic prefix could help15:52
wrtpTheMue: i'm happy with seeing the prefix in the message itself - then it's very obvious how the message is being made15:52
wrtpTheMue: i dunno, i could go either way15:53
TheMuewrtp: yes, doing it by convention may be enough15:54
TheMuewrtp: sometime i fall back in my mainframe history *lol*15:54
* TheMue has to step out, wife needs me for shopping. bbl.15:56
niemeyerfwereade, wrtp: We had jointly decided to drop these prefixes.. my reviews were merely attempting to put in place what we agreed16:01
wrtpniemeyer: ah, i don't think i saw that discussion16:01
fwereadeniemeyer, likewise -- but not bothered by it :)16:02
niemeyerwrtp: You were part of it for sure.. it's just been a while16:02
wrtpniemeyer: personally, i think the prefixes are useful, particular when the messages from different workers are mixed up in the same file16:02
niemeyerfwereade, wrtp: We can change the convention.. I just don't want to be going back and forth pretending we have a convention when we don't16:02
wrtpniemeyer: ok, yeah, i definitely thought we had a convention to have prefixed for log messages but not for error messages.16:02
wrtps/prefixed/prefixes/16:03
niemeyerwrtp: I hadn't seen that distinction yet, but happy to have it16:03
wrtpniemeyer: i'm slightly wondering if you're thinking about the error-message prefix conventions16:03
niemeyerwrtp: Yes, I'm thinking of those too16:04
wrtpniemeyer: ah, i see16:04
wrtpniemeyer: i think there's a definite difference.16:04
wrtpniemeyer: error messages are usually printed as a suffix to something else16:04
wrtpniemeyer: log messages are printed by themselves.16:04
niemeyerwrtp: Not really16:05
niemeyerwrtp: We don't have that convention today in any meaningful way.. logged messages are all over the place16:05
wrtpniemeyer: all the log messages i've written have a prefix, i think16:05
niemeyerwrtp: "That's the way I do" != "That's the convention we use"16:05
wrtpniemeyer: indeed. i tried to make a convention by writing log messages that way, but we never discussed it.16:06
niemeyerwrtp: Can you please just send a message to juju-dev so that it's actually an agreed convention?16:06
wrtpniemeyer: ok, will do16:07
niemeyerwrtp: Thanks!16:10
wrtpniemeyer: done16:21
niemeyerfss: ping16:36
niemeyerAram: ping16:37
Aramniemeyer: pong16:37
niemeyerAram: Re. "Addressed review points. Not submitting because I'm waiting for changes16:37
niemeyerin firewaller and machiner, so I don't break the build."16:37
niemeyerAram: What changes?16:37
Aramniemeyer: the firewaller and the machiner use the old, API incompatible version of the watcher16:37
Aram(the one that returns entities, not ids).16:38
niemeyerAram: Would you mind to cover that as well so that we can integrate that change?16:38
AramI'll take a look.16:39
niemeyerAram: I think you can have an idea for the change by diffing.. hmmm16:39
niemeyerAram: Within the firewaller directory: bzr diff -r 600..601 . | vi -16:40
niemeyerAram: It should be fairly simple.. my suggestion is to, right below those case unit, ok := <-ud.watcher.Changes():" lines, actually load the unit16:41
fssniemeyer: pong16:41
niemeyerAram: The rest will remain mostly untouched16:41
niemeyerfss: Hey16:41
niemeyerfss: I'm trying to merge your changes16:41
niemeyerfss: Both look good16:41
niemeyerfss: iam-deleteuser is conflicting with the previous branch, though16:42
fssniemeyer: =/16:42
niemeyerfss: Can you please merge lp:goamz into iam-deleteuser, fix conflicts, run tests, and re-propose?16:42
niemeyerfss: I'll then submit16:42
fssniemeyer: sure16:42
niemeyerfss: Cheers!16:43
fssniemeyer: I'll do it in a second16:43
fssniemeyer: if I get in any trouble using bzr, I may ask for your help :-)16:44
niemeyerfss: Sure thing16:44
fwereadeout for a while, have good evenings all16:49
wrtpfwereade: have fun16:53
wrtpniemeyer: does it matter that anything logged into the database can change the password for any other entity?16:53
wrtpniemeyer: i'm kinda presuming not, but it surprised me slightly.16:54
niemeyerwrtp: I mentioned it a few times when were brainstorming on the concept16:54
wrtpniemeyer: that must've gone over my head, sorry16:54
niemeyerwrtp: We won't be playing games with database permissions at the moment16:54
niemeyerwrtp: np16:54
niemeyerwrtp: Just saying it's expected16:54
wrtpniemeyer: cool16:55
niemeyerwrtp: The reason for the apparently silly care we're taking now is because this is the same logic we'll need once we have the API16:55
niemeyerwrtp: Once we get the API, we can easily cut down that kind of cross-talking16:55
wrtpniemeyer: i just wrote another test in state to verify that it works, because i thought it didn't :-)16:55
niemeyerwrtp: It surely does.. the authentication is against the database.. the user data is inside the database16:56
wrtpniemeyer: yeah - i guess i thought there was a difference between admin users and other users16:56
niemeyerwrtp: There is in fact16:56
wrtpniemeyer: ah yes, i remember16:57
niemeyerwrtp: We add the admin to the admin database, which means it can manipulate everything16:57
wrtpniemeyer: but it doesn't make any difference to us, because we only have two dbs16:57
wrtpniemeyer: i was thinking that only the admin could add users16:57
niemeyerwrtp: Certain commands can only be run against the admin database16:57
niemeyerwrtp: So there's actually some practical implication in addition to hat16:57
niemeyerthat16:57
niemeyerwrtp: Only admins can add users to all databases16:58
wrtpniemeyer: uh huh16:58
niemeyerwrtp: We shouldn't worry too much about that, though.. our primary goals on that round is getting basic authentication and transport security in place16:59
niemeyerwrtp: We can split down capabilities on the next round, once we get the API stuff going16:59
wrtpniemeyer: take it or leave it, i don't mind. https://codereview.appspot.com/663605717:01
niemeyerwrtp: Hmm, that's pretty interesting17:07
niemeyerwrtp: I suspect we'll want to enable the machine to set a unit's password, *if* the unit is assigned to the machine17:08
wrtpniemeyer: yeah, that's a good point.17:08
niemeyerwrtp: So that we can deploy the unit within the machine with a password that is locally defined and thus known by the machine so that it can provide --initial-password17:09
niemeyerwrtp: It shouldn't be able to set the password for an unrelated unit, though17:09
wrtpniemeyer: agreed17:09
wrtpniemeyer: the test should probably assign the units to the given machine17:09
wrtpniemeyer: but maybe it's nice to have a test that will fail in an interesting way when we add appropriate capability checking17:10
niemeyerwrtp: Not sure.. I'd prefer to not waste time writing tests we know are wrong17:12
wrtpniemeyer: ok. i can easily do that for this test if you like; or i can just drop the CL for now.17:13
niemeyerwrtp: I sometimes do that when I'm not in charge of implementing the feature, just so I can tell when upstream has changed, but that's not the case.. we know exactly when that will be done, and know it won't be a trivial change17:13
niemeyerwrtp: I'm happy to have the CL with the Assign change.. it's already written, and this would verify intended behavior17:14
wrtpniemeyer: ok, will do17:14
* niemeyer sips some nice coffee17:18
niemeyerwrtp: Thanks man17:19
wrtpniemeyer: unit passwords: https://codereview.appspot.com/663206117:28
niemeyerwrtp: Checking17:31
niemeyerwrtp: done17:34
wrtpniemeyer: thanks17:34
niemeyerwrtp: np17:35
wrtpniemeyer: i've fixed the password test: https://codereview.appspot.com/663605717:36
niemeyerwrtp: LGTM.. would be nice to clarify the commit message17:37
wrtpniemeyer: will do. in fact, i'll submit tomorrow, as i hear a voice from below...17:38
wrtpniemeyer: have a good rest-of-day17:39
wrtpnight all17:39
niemeyerwrtp: Thanks, and have a good one too!17:39
fsslet the merge begin17:46
niemeyerfss: Good to go?17:54
niemeyerHmm.. not yet17:55
fsslbox proposing :-)17:55
fssniemeyer: ptal17:56
niemeyerfss: Looking17:56
niemeyerfss: On the way17:59
fssniemeyer: \o/18:04
fssniemeyer: later today i'll start sending iamtest package18:04
niemeyerfss: Sweet!18:06
fssniemeyer: https://codereview.appspot.com/663106318:55
niemeyerfss: Looking18:56
fssniemeyer: thanks :-)18:56
fssniemeyer: notice that I didn't use iamtest_test in tests, so it was easier to test the calls, accessing private members of Server directly. What do you think?18:57
niemeyerfss: Being able to ask the test server for which users exist sounds useful18:58
niemeyerfss: I'd prefer to have that as part of the public interface, and have the test not poking at internals18:58
fssniemeyer: I see18:59
niemeyerfss: Also, we really need the integration test in the same style we have within ec218:59
niemeyerfss: So that someone can run tests for goamz/iam and have an idea that Amazon is actually happy with the implementation19:00
fssniemeyer: hmm19:00
niemeyerfss: The existing stuff within ec2i_test.go should serve as inspiration19:00
fssniemeyer: I will add these integration tests19:00
fssniemeyer: then provide GetUser support on iam, and use it to test iamtest's CreateUser and DeleteUser19:01
niemeyerfss: Right, that's a great way to wire things together19:01
fssniemeyer: actually, I think I'll need GetUser for CreateUser and DeleteUser integration tests too19:02
niemeyerfss: Then you know that: a) goamz/iam works agains the real interface; b) goamz/iam works against the fake interface; Thus c) the real and the fake interface look alike19:02
niemeyerfss: Absolutely19:02
niemeyerfss: And you can do that with a single suite19:03
fssniemeyer: a single suite?19:03
niemeyerfss: Have a look at ec2i_test.go, and the relationship between ClientTests and AmazonClientSuite19:04
fssniemeyer: ok, thanks :-)19:04
niemeyerfss: Then, check out ec2t_test.go, and check how LocalServerSuite is defined19:04
fssniemeyer: cool, thanks for the help. Time for some fun19:08
niemeyerfss: np19:14
fssniemeyer: here is GetUser, I will use it in integration tests: https://codereview.appspot.com/663106419:17
fssniemeyer: now I will start to work on the integration suite19:17
niemeyerfss: Looking19:17
niemeyerfss: Looks good.. just a trivial on the doc19:20
fssniemeyer: opsss19:20
fssniemeyer: shame on me19:20
fssniemeyer: ptal19:22
niemeyerfss: No worries, and thanks19:22
niemeyerfss: That's in19:28
fssniemeyer: thanks, I'm looking ec2i_test and ec2t_test19:33
fssniemeyer: the integration suite already paid itself, the error treatment is broken. Should I send the fix with the integration tests cl, or in a separate CL?20:09
niemeyerfss: It's fine to be in the same CL in that case.. it's really the test for the fix20:10
fssniemeyer: ok20:14
fssniemeyer: do you have any trouble with lbox + vim?20:17
fssniemeyer: whenever I use vim, I get "error: Change summary is empty.". It's like vim is not actually saving the temporary file in the disc20:18
niemeyerfss: Hmm.. nope.. works fine here20:19
niemeyerfss: How does $EDITOR look like?20:19
fssEDITOR=/usr/local/vim20:19
fssI set it to nano, so I can send the cl20:19
fss/usr/local/bin/vim, sorry20:20
fssniemeyer: here we go: https://codereview.appspot.com/662507720:23
niemeyerfss: Looking20:24
niemeyerfss: Are you sure you're saving the file? I really can't imagine what might be going on20:24
fssec2t_test.go is the test suite for the ec2test package?20:25
fssniemeyer: yes, I can `cat` it20:25
niemeyerfss: Very strange.. if you find what's up, please let me know20:25
niemeyerfss: On the previous question, yes20:26
fssniemeyer: I've compiled /usr/local/bin/vim by hand, I'll try /usr/bin/vim later and see if the problem persists20:26
niemeyerfss: Suite is great, thanks. Submitting20:26
fssniemeyer: okay, I'll add iamt_test in the previous iamtest CL20:26
niemeyerfss: Ah, please add to a new one20:26
niemeyerfss: I'm moving this one forward20:26
fwereadeniemeyer, heyhey20:27
fssniemeyer: this one? https://codereview.appspot.com/6631063/20:27
niemeyerfss: Ah, sorry, I misunderstood, that's great20:30
niemeyerfwereade: Heya20:30
fwereadeniemeyer, re HookContext.service: perhaps I've been making bad assumptions re service config -- I had assumed that we'd be able to get the config for the unit's current charm without directly involving the service at all... but now I seem to recall a discussion in which you were planning to keep all a service's configs in a single document20:31
fwereadeniemeyer, well, not necessarily planning20:31
fwereadeniemeyer, I can't remember whether you were for or against it20:32
niemeyerfwereade: That's probably because I wasn't either for or against it :-)20:32
fwereadeniemeyer, that would explain my inability to recall one or the other :)20:33
niemeyerfwereade: Hehe :)20:33
niemeyerfwereade: I'm still wondering, actually.. I leaning slightly towards different documents20:33
fwereadeniemeyer, so, with that half-baked assumption in mind, it seemed clear that the service was enitrely transitory data20:33
fwereadeniemeyer, I think my brain had drifted into a similar configuration20:33
fwereadeniemeyer, it had crossed my mind that it would be convenient to key them on service name + charm url, which can be constructed trivially from a unit20:34
fwereadeniemeyer, and so service seemed like a sort of unwanted appendix that didn't really deserve a field20:34
niemeyerfwereade: service.Config(charmURL) is perhaps the best candidate at the moment20:34
niemeyerfwereade: With optional charmURL, so service.Config(nil) returns the tip20:35
fwereadeniemeyer, my heart says unit.ServiceConfig(), but I expect you have good reasons for your choice... yeah, it's more orthogonal20:35
niemeyerfwereade: That sounds slightly awkward.. unit.ServiceCharm()? unit.ServiceCharmURL()? :-)20:36
fwereadeniemeyer, just so long as the unit is up to date, anyway -- but, yeah, at least the service wouldn't need a refresh20:36
fwereadeniemeyer, it's not my heart: it's my lazy fingers that want it, and my brain has now overruled them20:37
niemeyerfwereade: LOL20:37
fwereadeniemeyer, => service is an expected long-term feature of HookContext => it stays20:37
fwereadeniemeyer, cheers20:38
niemeyerfwereade: Cool, I see your original reasoning as well, thanks20:38
fwereadeniemeyer, btw, possible uniter weirdness20:39
fwereadeniemeyer, given that .unit (and, I guess, .service ;)) are long-lived, I'm conflicted about when they should be being refreshed20:40
niemeyerfwereade: If we don't know it probably means we don't have to refresh (yet)20:41
fwereadeniemeyer, ha, I have just looked at the state code... I'm sure I remember a time when we weren't always updating the local doc with the change we made20:42
niemeyerfwereade: Ah, we should, I think20:43
fwereadeniemeyer, (was there some idea at one stage that a local state doc should always reflect the actual state of the remote document at some point in time, rather than being a palimspest?)20:43
niemeyerfwereade: I think we were unsure until the first sprint in Lisbon20:44
fwereadeniemeyer, must have just missed more recent conversations20:44
niemeyerfwereade: That's when we explored the options in details, and agreed on doing what we have now20:44
niemeyerfwereade: I don't think we've changed the approach since then20:44
fwereadeniemeyer, great20:44
niemeyerfwereade: That doesn't mean it's perfect, but at least so far things to be holding together20:45
niemeyererm20:45
niemeyerthings seem to be20:45
fwereadeniemeyer, yeah, so far, digging deeper has always reassured me20:45
fwereadeniemeyer, oh, btw20:46
fwereadeniemeyer, filter has a config watch, which is only used in ModeAbide (but will probably also be used in ModeAbideDying, which wil emerge at some stage)20:47
fwereadeniemeyer, however, I am in the position of adding a relations watch which is only going ot be useful to a single mode20:47
niemeyerfwereade: Cool20:47
fwereadeniemeyer, and it's feeling a little bit off-kilter20:47
niemeyerfwereade: Because we're watching things we can't always act upon?20:48
fwereadeniemeyer, partly... but more because ModeAbide, as a consumer, will be acting on uniter-wide state in response to these changes20:48
niemeyerfwereade: Hmm.. I don't yet see the light20:49
fwereadeniemeyer, and it is, I think, flat-out *wrong* for any other mode to use those events and mess with state in response to them20:49
fwereadeniemeyer, unless they do exactly what ModeAbide does20:49
niemeyerfwereade: Sure, but that sounds a bit of a general rule20:50
niemeyerfwereade: It's flat out wrong for any mode to watch state and do changes they shouldn't20:50
fwereadeniemeyer, I think what I'm trying to say is that it's legitimate for different modes to respond to (say) resolved events in different ways20:51
niemeyerfwereade: I think I understand what you're saying, but IMO what you describe is one of the benefits we intended with the filter method20:52
niemeyerfwereade: To me it's a huge simplification the fact we can simply allow modes to care about stuff they should, and ignore the stuff (including events) they shouldn't act upon20:52
fwereadeniemeyer, ok: (drags self laboriously towards point) I am conflicted over whether or not I should implement a WantRelationsEvent on filter20:53
niemeyerfwereade: I suggest not implementing it until there's a need for the logic20:54
niemeyerfwereade: It won't save us from someone using the method when they shouldn't, and it'll mean we'll have to write and read the logic for the whole thing20:55
fwereadeniemeyer, well -- I'm not sure why it is a good idea to add complexity to filter -- which is pretty clean, but not unworthy of careful treatment20:56
fwereadeniemeyer, when I can be pretty sure that only ModeAbide should ever be messing with those events20:57
fwereadeniemeyer, especially considering that ModeInit and ModeAbideDying will themselves be expected to independently manipulate runtime relation state20:57
niemeyerfwereade: I don't understand how the two things link together20:57
niemeyerfwereade: Yes.. only ModeAbide should be receiving those events, so...?20:58
fwereadeniemeyer, sorry, I'm rather thinking out loud20:58
niemeyerfwereade: Seems to be tightly related to everything I said above20:58
fwereadeniemeyer, so it is bad and scary and wrong to let anyone else see them20:58
niemeyer<niemeyer> fwereade: I think I understand what you're saying, but IMO what you describe is one of the benefits we intended with the filter method20:58
niemeyer<niemeyer> fwereade: To me it's a huge simplification the fact we can simply allow modes to care about stuff they should, and ignore the stuff (including events) they shouldn't act upon20:58
niemeyer<niemeyer> fwereade: It won't save us from someone using the method when they shouldn't, and it'll mean we'll have to write and read the logic for the whole thing20:58
fwereadeniemeyer, primarily because they are not recoverable in the way that, say, a resolved event is20:58
fwereadeniemeyer, if there is only a single correct way to consume these events, and we cannot recover history, it seems unwise to leave them lying around on filter where not-William (;)) can use them21:00
niemeyerfwereade: Understood.. recoverable as in we can't request the same event to be sent again21:00
niemeyerfwereade: Right?21:00
fwereadeniemeyer, yes21:01
niemeyerfwereade: Cool, so WantConfig would be fine it seems21:01
niemeyerfwereade: Except, do we want to run that at all times?21:01
fwereadeniemeyer, WantConfig is ok because (1) it's recoverable and (2) it's used by 2 modes, even though one of them doesn't exist21:01
fwereadeyet21:02
niemeyerfwereade: I don't think I get it.. WantConfig would mean we'd *always* get such an event21:02
fwereadeniemeyer, yeah, that's exactly the idea21:02
niemeyerfwereade: and my understanding is that we don't want to run a config-changed hook unless the config changed, in normal situations21:02
niemeyerfwereade: which is what ModeAbide is about, in my understanding21:03
fwereadeniemeyer, except when we enter ModeAbide -- we agreed this was a clean way to guarantee that start and upgrade-charm were always succeeded by config-changed21:03
* fwereade twitches as an idea clicks21:04
fwereadeniemeyer, huh, I don't need it in ModeAbideDying21:04
fwereadeniemeyer, so now we have relations more like config than anything else, and neither of them really deserves to dirty up the filter21:04
niemeyerfwereade: Won't we get into ModeAbide when we get out of an error state? Or out of a relatoin changed state? Etc?21:05
fwereadeniemeyer, we discussed this, and yu were keen on it; the primary justification, which I was very happy with, was that if *any* hook *had* to be idempotent anyway, it was config-changed21:05
niemeyerfwereade: I totally agree with that21:06
niemeyerfwereade: I don't think I agreed we should run config-changed whenever we sneeze, though :-)21:06
fwereadeniemeyer, the only extra time it's run is after ModeHookError AFAIR21:07
niemeyerfwereade: ModeAbide is the steady state.. we'll get into it everytime we get out of any other mode21:07
fwereadeniemeyer, easy to change, happy to do it (queued hooks are already a thing), will be nicer :)21:08
fwereadeniemeyer, at this stage the only reason to do it is historical21:08
niemeyerfwereade: Cool21:08
fwereadeniemeyer, the uniter's shed a lot of complexity since then :)21:08
niemeyerfwereade: Indeed21:09
fwereadeniemeyer, hum, ok21:11
fwereadeniemeyer, wait21:11
fwereadeniemeyer, we *also* run config-changed whenever we start up and first come into modeabide21:11
fwereadeniemeyer, and that is necessary21:11
niemeyerfwereade: Right, after start21:12
fwereadeniemeyer, because we don't have any permanent record of service config version21:12
fwereadeniemeyer, no, any time we're bounced21:12
niemeyerfwereade: bounced?21:12
fwereadeniemeyer, process death21:12
fwereadeniemeyer, or even just the UA restarting the uniter21:12
fwereadeniemeyer, I did think this was crazy21:13
niemeyerfwereade: Hmm.. why is it crazy?21:14
fwereadeniemeyer, because it forces us to run one whenever we enter ModeaBIDE :)21:14
niemeyerfwereade: The two things sound pretty unrelated21:14
fwereadeniemeyer, we're entering ModeAbide, and we might need to run a config-changed hook21:15
fwereadeniemeyer, how do we tell?21:15
niemeyerfwereade: One is a high-level requirement, the other is an implementation detail21:15
* niemeyer looks at fwereade21:15
niemeyerfwereade: Okay, it's pretty late there.. I'll help.. how can we tell when is the first thing we do something inside a process or type?21:16
niemeyers/first thing/first time21:16
fwereadeniemeyer, ha :/21:16
fwereadeniemeyer, seems somehow wrong to have to store that state on the uniter itself21:16
niemeyerfwereade: There are many ways to do it, but it's far from a OMG case :)21:17
niemeyerfwereade: Sorry, I have to run a quick errand.. I'll be back in ~15mins21:19
fwereadeniemeyer, quite so... it feels weird to make the uniter stateful like that21:19
fwereadeniemeyer, np, will probably be around ;)21:19
niemeyerfwereade: LOL.. just ponder for a moment about that last statement regarding how it's weird to make the uniter stateful :-)21:19
fssniemeyer: here is it: https://codereview.appspot.com/6631063/21:21
fssniemeyer: I've implemented iamt_test.go running full ClientTests suite, since they're born together21:21
niemeyerfwereade: Around again21:53
niemeyerfwereade: So,22:16
niemeyerfwereade: back to your point, the uniter is already pretty stateful.. we have a full state machine on it22:16
fwereadeniemeyer, yeah, indeed22:16
niemeyerfwereade: (a pretty nice one, actually!) :-)22:16
fwereadeniemeyer, I'm implementing it now, it's interesting22:16
niemeyerfwereade: I'm not sure if I misunderstood the heart of your point, though.. maybe it's becoming stateful in a way it wasn't?22:17
fwereadeniemeyer, well, the state machine is really in the modes, which aren't in uniter fields, but I take your point22:17
fwereadeniemeyer, I think the code will illustrate my perspective better than anything I can say, and I*think* it's nearly there22:18
niemeyerfwereade: There are actually other stateful bits too.. filter being a notable example22:18
fwereadeniemeyer, yeah, indeed, I actually don;t think it's a bad thing -- I've just got very used to Modes not directly using uniter for state, except as a proxy for disk state22:20
fwereadeniemeyer, I have a nervous feeling about this code, because I'm not sure whether or not you'll like it -- it's relatively neat, but I felt the need to comment to explain what it's doing in one place, which is always a bit of a worry22:36
fwereadeniemeyer, ready to propose in a sec22:36
fwereadeniemeyer, then I should go, need to be up tomorrow (bah)22:36
niemeyerfwereade: Thanks a lot, looking forward to checking it out22:37
fwereadeniemeyer, ok, looking at it in codereview, I think it's pretty good :) https://codereview.appspot.com/663206222:43
* fwereade suspects these words will come back to haunt him22:44
niemeyerfwereade: Hehe :-)22:52

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