fwereademgz, fwiw, I have 4 CLs with diffs of <100 lines that have 1 LGTM already -- if you're free at any point this afternoon I would be most grateful if you would look at them13:44
fwereademgz, they are: https://codereview.appspot.com/7001050/ https://codereview.appspot.com/7001053/ https://codereview.appspot.com/7005054/ https://codereview.appspot.com/7013048/13:44
fwereademgz, (and I have others too, if you happen to be bored... ;p)13:46
mgzwill do.13:47
fwereademgz, cheers13:47
mgzhave still not submitted to love of rietveld...13:48
mgzthe forced file-by-file review workflow just doesn't match what I want to do13:50
fwereademgz, I find it works pretty well for me, but mileage clearly varies -- I find that once I've done an initial skim for overall structure I'm fine13:52
fwereademgz, but then I'm probably misunderstanding how you like to review code13:53
fwereadearosales, ping16:10
arosalesfwereade: hello16:10
fwereadearosales, heyhey, hope you had a good xmas16:10
arosalesI did, spent some good time with the family.16:11
fwereadearosales, I have a change I think we need to make to a few charms16:11
arosalesI hope yours was well too16:11
fwereadearosales, cool, likewise :)16:11
arosalesgood to hear16:11
fwereadearosales, it's simple but may be annoying -- it's that several of them use relations with names starting with "juju-" and those are disallowed in juju-core16:11
fwereadearosales, I think/hope you're the right person to talk to about how to move forward on changing them all16:14
arosalesI am16:14
fwereadearosales, cool16:15
arosalesjimbaker: has also been working on reconciling differences in charms going to Go16:15
arosalesthat sounds a bit weird16:15
arosalesbut stated differently jimbaker has been working on the format2 work which is aimed at reconciling the differences in charms between py and Go16:16
fwereadearosales, the issue is very simply that every charm implicitly provides juju-info (over the juju-info interface) -- and a charm that provides and requires relations with the same name are obviously insane16:16
fwereadearosales, rather than just disallowing juju-info (which I believe is utterly necessary) it seemed smarter to reserve the juju-* namespace16:16
arosalesmakes sense16:17
jimbakercatching up16:17
arosalesfwereade: how do you feel about filing a bug to identify these charm porting issues16:17
fwereadearosales, np at all -- the question is how you'd best like it16:18
arosalesI can then make a task against appropriate charms that need to be changed to support Go16:18
jimbakerfwereade, i believe it was our intent to reserve the juju- namespace16:18
fwereadearosales, ie what the bug should be filed against16:18
jimbakerback when juju-info was first proposed after orlando 201116:18
fwereadejimbaker, excellent, I *thought* something like that had been agreed among, er, at least some of us all16:19
arosalesjimbaker: it sounds like we still have charm that are using a juju-* relation, correct fwereade?16:19
jimbakerfwereade, i will try to dig up, although my thunderbird client has been extremely flaky recently16:19
fwereadejimbaker, correct16:19
fwereadejimbaker, IIRC the precise restrictions in go are as follows:16:20
fwereade1) no "juju" or "juju-*" relations are allowed16:20
fwereade2) no "juju" or "juju-*" filenames are allowed in the hooks dir16:20
jimbakerwell it's in our docs, https://juju.ubuntu.com/docs/implicit-relations.html16:21
jimbaker"reserved namespace"16:21
fwereade3) no relations can provide the "juju" or "juju-*" interface16:21
fwereadejimbaker, ok, that's fantastic -- at least it means that the charms are flat-out wrong and there's little justification for complaints about the change :)16:21
fwereadejimbaker, (note that ofc it's fine to *require* over a reserved interface)16:22
jimbakerfwereade, it's also restricted by the python code16:22
jimbakerin that require is feasible (as it must!)16:22
fwereadejimbaker, awesome -- we are all on the same page then, looks like it's just the charms :)16:22
jimbakeri think the only additional restriction here is the juju interface name16:23
fwereadearosales, against what would you like the bug to be filed?16:23
jimbakervs juju-*16:23
fwereadejimbaker, I forget the reasoning behind that -- probably just that it's more confusing to allow it than to disallow it16:23
jimbakerfwereade, we do have one example of the juju charm16:24
fwereadejimbaker, (btw: merry xmas and happy new year :))16:24
jimbakerwhere the name is used, although not for relation name/interface name16:24
fwereadejimbaker, ah, yes, interesting -- but it doesn't quite collide -- yeah :)16:24
jimbakerfwereade, thanks! and i hope it was a wonderful holiday and start of the new year too!16:24
fwereadejimbaker, cheers, it was :)16:25
jimbakerfwereade, in the case of the juju charm, it is a nice metahack, and makes a lot of sense. so at the least, grandfather that in16:25
fwereadejimbaker, yeah, I'm fine with that16:25
fwereadejimbaker, it's not disallowed by anything as it stands16:26
jimbakerfwereade, cool16:26
jimbakerfwereade, one thing that would be useful to do is to update charm proof for this sort of stuff16:27
jimbakersuch as requiring format: 2 to be declared16:27
jimbakerit be warn for now, but err if a special flag is used16:27
* fwereade looks a bit guilty re format 2 -- he seems to recall someone else taking responsibility for that a few months ago but can't remember who, and sees that there are still open bugs related to it16:28
fwereade(in juju-core, that is)16:28
fwereadejimbaker, actually, *is* anything actually using format 2 yet?16:28
jimbakerfwereade, i'm afraid widespread adoption is lacking16:28
jimbakerthe reality is that ascii usually works16:29
fwereadejimbaker, I'm not certain that's actually such a bad thing, because I *think* most of the charms are working fine under juju-core anyway16:29
jimbakerfwereade, where format: 2 presumably is implied16:29
fwereadejimbaker, theoretically, yes -- it just seems like almost nothing actually depends on repr-style output16:30
jimbakerfwereade, the reality is, you know it if you need it, or just working around via b6516:30
fwereadejimbaker, ha, yes, I'd forgotten that side of it tbh -- the primary source of my own panic was entirely repr-style output16:31
jimbakerfwereade, no doubt because it's difficult to parse in a shell script16:31
fwereadejimbaker, yeah, lucky for us :)16:31
jimbakerfwereade, indeed16:32
fwereadejimbaker, just a thought - do the subordinates which use juju-* relations actually work at all under pyjuju?16:32
fwereadejimbaker, I can't quite believe either that they do or that they don't ;p16:33
jimbakerfwereade, i think the one thing we did get out of the exercise is making it quite well defined. there was no nearly testing of formatting. now there is, for both legacy and format 216:33
fwereadejimbaker, yeah, that's a definite win16:33
jimbakerfwereade, they work. the real problem is lack of serialization between primary and subs16:33
fwereadejimbaker, ha, that16:33
jimbakerfwereade, now perhaps that's reasonable. but there's the special case of apt-get to consider16:34
fwereadejimbaker, well, my view is that it's not reasonable at all, but I'm up against niemeyer and rogpeppe on that, so I don't hold out much hope16:35
jimbakerso we have been going back & forth between recommending flock to guard, or use aptdcon (and make that a dependency of the juju package)16:35
jimbakerfwereade, so the current status in goju is not to serialize then?16:35
fwereadejimbaker, (I feel that it makes sense to see juju as ceding administratory-style control over the machine when running hooks, and that having multiple admins logged into the same machine is not a sane thing in general)16:36
fwereadejimbaker, the current status in goju is to ignore the problem I'm afraid -- has pyju addressed it already?16:36
jimbakerno, we ignore too16:36
jimbakerhence the problem16:36
fwereadejimbaker, the ?consensus? is that we should provide special apt tools16:37
fwereadejimbaker, I think that's crack, so I haven't done it16:37
jimbakerfwereade, you mean like a wrapper of apt-get?16:37
fwereadejimbaker, (not that anyone has asked me to, but still)16:37
fwereadejimbaker, yeah16:37
jimbakeror a juju-apt-get?16:37
fwereadejimbaker, I'm not quite sure -- there was talk of some sort of declarative magic, which also sounded kinda crackful to me, because you don't know what needs to be declared until you're running the charm, and config can change at any time16:38
jimbakerfwereade, maybe that's the right solution - apt-get and friends always are guarded by flock /usr/bin/apt-get16:38
fwereadejimbaker, honestly, I don't like that much, I feel that apt-get is a specific (and, ok, overwhelmingly common) case of a general problem16:39
fwereadejimbaker, it may be that guarding apt-get fixes close enough to 100% of problems that it's actually the best way to go16:40
jimbakerfwereade, well i do think it makes sense to support more declarative stuff in charms, eg charmhelpers16:40
fwereadejimbaker, and at least it means we *don't* have to serialize hook execution across processes16:40
jimbakerfwereade, maybe the right solution is:16:41
fwereadejimbaker, yeah, that which can be done declaratively generally should be -- I just don;t think anyone's actually thought it through in enough detail yet16:41
jimbakerwe pick a standard lock file for flock16:41
jimbakerapt-get and friends are wrapped by guards: flock /standard/file; this is on the path for any hooks16:42
jimbakerthen if a hook wants to participate in the locking, it also uses flock /standard/file/or/whatever16:42
fwereadejimbaker, cautious +1, it is pretty clean16:44
jimbakerfwereade, how does that proposal sound? then maybe the declarative stuff in charmhelpers could say: serialize: true, and it would automagically ensure all hooks get this treatment16:44
jimbakerso opt-in for hooks, but it solves the biggest problem we have now16:44
fwereadejimbaker, yeah, indeed -- would you take that proposal to the lists please? just in case someone else spots anything problematic?16:45
fwereadejimbaker, but I think I like it16:45
jimbakerfwereade, ok, sounds good to me - it's just an extension of what we've been discussing on #juju, including yesterday - so list discussion is definitely in order16:46
fwereadejimbaker, awesome16:47
=== fss_ is now known as fss
hazmatfwereade, juju-* for interfaces is reserved.. but for rel names.. does it matter?20:58
hazmatfwereade, so instead your suggesting they define hooks for the relation but not declare their requirement?21:00
hazmatfwereade, its important for those charms to be able to define hooks for those relations.. i'm failing to see why this an issue in go.21:04
hazmateffectively every generic subordinate uses them21:04
hazmatjimbaker, arosales was there something i'm missing here21:04
hazmator are we confusing relation name with interface name21:05
jimbakerhazmat, pyju restricts both relation and interface names to not be prefixed with juju-21:06
jimbakerhazmat, i don't think there's much confusion here on that21:06
jimbaker(if any ;)21:06
jimbakermore of an issue that we were verifying goju vs pyju21:06
hazmatjimbaker, not true21:07
arosaleshazmat: it sounded like there was a name space conflict, but perhaps that is not the issue21:08
hazmatthe not provide juju-info makes sense21:08
arosaleshazmat: fwereade may have more insights, but EOD for him21:08
hazmatthe change around no relation name with juju in it will cause some breakage21:09
jimbakerhazmat, please clarify what you mean by not true, i suppose my short hand is for charms to provide such relations :)21:09
hazmatjimbaker, <jimbaker> hazmat, pyju restricts both relation and interface names to not be prefixed with juju-21:09
hazmatjimbaker, that statement is false given the number of charms i linked to which do it21:10
jimbakerjuju.charm.metadata.MetaData.parse restricts the namespace here21:10
jimbakerhazmat, just looking at the code to see what is it says21:10
hazmatjimbaker, that only applies to provides21:11
hazmatjimbaker, the change he's talking about is to requires as well21:11
hazmatjimbaker, the charms i linked require on juju-info and many have 'juju' in the relation name21:12
hazmatand thus also hooks with that name21:13
jimbakerhazmat, i didn't look at juju in the relation name, but we discuss this in charm proof. i suppose goju could be relaxed in this case21:14
jimbakerneeds to be discussed here i suppose21:14
jimbakeri believe we described this as being restricted to provides, but can revisit w/ fwereade when he's back21:15
hazmatthere's close to 40 charms there that juju in the rel name.. maybe 15 unique21:15
jimbakerhazmat, ack21:17
hazmater.. 24 unique21:17
jimbakerso that's good evidence to change goju21:17

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