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

TheMuemorning07:31
fwereade__TheMue, heyhey07:39
TheMuefwereade__: hello07:39
fwereade__TheMue, just in case you have perspective on this: ISTM that there is nothing preventing a charm from publishing multiple relations with the same name, so long as they have different roles... do you know if this is intentional?07:40
TheMuefwereade__: sorry, dunno. net yet went deeper into relations constraints07:41
fwereade__TheMue, no worries :)07:42
TheMuefwereade__: phew ;)07:42
fwereade__TheMue, it's just that I saw you at the exact moment I was deciding I didn't know the answer myself :)07:43
TheMuefwereade__: yeah, I would have liked to give you the missing link07:45
wrtpfwereade__, TheMue: mornin'08:16
TheMuewrtp: bonjour08:17
Arammoin.08:49
TheMueAram: moin moin09:06
fwereade__Aram, wrtp, TheMue: does anyone know why the juju-info relation has global scope?09:16
fwereade__bcsaller, actually, if you're around, then you will be sure to know: ^^09:17
TheMuefwereade__: sorry again, but it seems i have to take a deeper look at relations to get a better understanding09:19
wrtpfwereade__: that does seem a bit odd09:57
wrtpfwereade__: isn't the whole point of juju-info to support subordinates' relationship with their principal?09:57
fwereade__wrtp, that had indeed been my conception of it09:57
fwereade__wrtp, hmm, I'm suddenly wondering whether we should be occupying the juju-* namespace for hooks as well as relations10:05
fwereade__wrtp, otherwise people could implement hooks for the provider side of juju-info10:06
wrtpfwereade__: hmm, interestin10:06
wrtpg10:06
fwereade__wrtp, which becomes icky if those hooks suddenly appear during a charm upgrade, with a bunch of hooks already "run"10:06
wrtpfwereade__: i'm not quite sure i understand10:07
fwereade__wrtp, it might not be a big deal, I'm not quite sure10:08
fwereade__wrtp, I can't figure out whether there's any value in implementing provider-side hooks for juju-info10:09
wrtpfwereade__: i don't think we can occupy the juju-* name space entirely, otherwise you won't be able to implement requirer-side hooks for it10:09
wrtpfwereade__: hmm, i can see that there *might* be10:09
fwereade__wrtp, ha, just realised10:10
wrtpfwereade__: although it's perhaps dodgy.10:10
fwereade__wrtp, even if there is value it's fundamentally dodgy anyway10:10
fwereade__wrtp, if we have multiple relations with the same name then the "what hook do we run" question becomes, I think, insoluble10:11
wrtpfwereade__: why would we have multiple relations with the same name?10:11
fwereade__wrtp, we already do... everything implicitly provides juju-info, and some things also require it10:12
wrtpfwereade__: doesn't a principal provide exactly one juju-info relation?10:12
fwereade__wrtp, yes, but so does a subordinate10:12
fwereade__wrtp, everything provides exactly one10:12
wrtpfwereade__: ah, so you've got the same relation in provider and requirer roles10:12
fwereade__wrtp, everything requires one or zero10:12
fwereade__wrtp, yeah, essentially, but they're not really the same relation10:13
wrtpfwereade__: yeah sorry,10:13
wrtpfwereade__: i meant the same relation name10:13
fwereade__wrtp, cool10:14
wrtpfwereade__: so it makes sense that you can't specify a juju-info hook10:14
fwereade__wrtp, no, but yu have to10:14
fwereade__wrtp, otherwise the whole relation is worthless10:14
wrtpfwereade__: because if you're requiring a juju-info hook, you'd name the relation something different, no?10:14
wrtpfwereade__: tbh i haven't yet seen a good use for the juju-info relation10:15
fwereade__wrtp, I haven't done an exhaustive check but I've seen at least two charms with a require named juju-info10:16
wrtpfwereade__: did you see what they used it for?10:16
fwereade__wrtp, here is one one example http://jujucharms.com/~david-duffey/precise/ddclient/hooks/juju-info-relation-joined10:17
fwereade__wrtp, here is another: http://jujucharms.com/~ted/precise/application-start/hooks/juju-info-relation-joined10:18
wrtpfwereade__: it's interesting that they both specify scope: container10:21
wrtpfwereade__: i suppose that actually it makes sense to allow non-locally-scoped juju-info relations, because requirers can get local scope if they want10:22
fwereade__wrtp, I'm not sure we get anything out of that ability though10:23
fwereade__wrtp, I suspect it just misleads10:23
fwereade__wrtp, I *think* that what I would like is:10:23
wrtpfwereade__: it provides the ability for one service to watch any other service in a generic way10:23
fwereade__wrtp, every subordinate implicitly requires it; every principal implicitly provides it; write all the hooks you want; don't allow cross-role name collisions10:26
wrtpfwereade__: how many of those statements are true today?10:27
fwereade__wrtp, just about none :/10:27
wrtpfwereade__: i'm not sure about the first one10:27
fwereade__wrtp, except possibly for write-all-the-hooks-you-want, but it's worthless due to name collisions10:27
wrtpfwereade__: why should every subordinate implicitly require it?10:28
fwereade__wrtp, the subordinates don't have to do anything with it, just as principals don't have to do anything with the one they implicitly provide10:28
wrtpfwereade__: i thought *all* services provided a juju-info relation10:28
wrtpfwereade__: it kinda makes sense to me that that might be the case10:28
fwereade__wrtp, yeah, I think that subordinates shouldn't10:29
wrtpfwereade__: why not?10:29
wrtpfwereade__: i think it could make sense to think of juju-info as something entirely orthogonal to the subordinate-principal relationship10:30
fwereade__wrtp, because we have charms that both provide and require juju-info, and that's obviously crack10:30
fwereade__wrtp, that's a nice idea, but I can't see how to make it work10:30
fwereade__wrtp, (without changing existing charms...)10:31
fwereade__wrtp, ok, alternative that might work10:31
wrtpfwereade__: i'm not sure. if we say that all charms implicitly provide juju-info, and juju-info relation hooks refer to the requirer side, i think that *might* work10:31
fwereade__wrtp, everything implicitly provides juju-info unless it explicitly requires juju-info10:31
fwereade__wrtp, apart from anything else, surely the fact that we can have unresolvably ambiguous relation specs is enough to sink the idea that name collisions are ok10:33
wrtpfwereade__: one possible way would be to move in a backwardly-incompatible way and prohibit people declaring a relation with the name "juju-info"10:33
wrtpfwereade__: given that very few people seem to use this functionality so far, that might not be so bad10:34
wrtpfwereade__: and then all the problems go away10:34
fwereade__wrtp, could do, I guess -- but that feels like a punt on the big issue, which is that we apear to allow relation name collisions in charms10:34
wrtpfwereade__: you can declare several relations with the same name?10:35
wrtpfwereade__: that does seem like crack10:35
fwereade__wrtp, yeah10:35
wrtpfwereade__: i wonder if any charms do that10:35
wrtpfwereade__: (other than the ones that declare juju-info, of course)10:35
fwereade__wrtp, any with a juju-info relation do10:35
fwereade__wrtp, ;p10:35
fwereade__wrtp, I hope not :)10:35
wrtpfwereade__: i think it makes sense to reserve all relations with the prefix "juju-"10:36
wrtpfwereade__: or actually...10:36
wrtpfwereade__: yeah, it makes sense to do that; then we can add more in the future without breaking charms.10:37
wrtpfwereade__: but i think people could still write juju-info hooks10:38
wrtpfwereade__: i don't really see why that's a problem yet10:38
fwereade__wrtp, they are actually all meant to be reserved, I think :/10:39
wrtpfwereade__: if we insist that noone can redeclare juju-info, then we don't have any ambiguity10:39
wrtpfwereade__: in which case that ddclient charm is bogus, right?10:39
wrtpfwereade__: (the other one is ok, as it doesn't name the relation "juju-info")10:40
fwereade__wrtp, hell, the hook naming totally threw me there10:41
fwereade__wrtp, yeah, that sounds sensible to me10:41
fwereade__wrtp, I could swear I'd seen code explicitly forbidding explicit juju-* relations10:41
wrtpfwereade__: we don't know that that charm actually works...10:42
=== Aram2 is now known as Aram
fwereade__wrtp, haha :/ I had hoped that charm store entry procedures were a little stricter, but I guess we already know they aren't all perfect10:46
fwereade__wrtp, can you think of any reason not to insert the implicit relation in ReadMeta?11:13
wrtpfwereade__: that's a good question11:14
wrtpfwereade__: my gut says it's probably not a great idea, but...11:14
fwereade__wrtp, yeah, I too feel discomfort, but I don't think any other place works11:15
wrtpfwereade__: can't it be done in state, when the charm is added?11:17
wrtpfwereade__: i think the charm package should prohibit the declaration of relations with a juju- prefix, but i'm not sure it should actively declare them.11:18
wrtpfwereade__: that seems to me like something that's appropriate for the system implementation to do.11:19
wrtpfwereade__: i'm kind of thinking that at some point in the future, we might see charms being used in different contexts, some of which might have a different set of implicitly declared juju- relations.11:19
fwereade__wrtp, you may be right... I guess it's just that I'd prefer that the unit agent be able to determine whether a potential relation is valid by reading its own charm instead of having to hit remote state for it11:20
fwereade__wrtp, actually that's no bug deal it's just a method that takes a charm.Charm11:20
fwereade__wrtp, yeah, sgtm, cheers11:20
wrtpfwereade__: cool11:20
wrtpfwereade__: gustavo may well think differently, of course!11:21
fwereade__wrtp, I understand that caveat to be implicit in everything I hear11:21
fwereade__wrtp, or think ;)11:21
wrtplol11:21
fwereade__wrtp, hm, the python explicitly disallows *provides* with a juju-* name or interface11:24
wrtpfwereade__: i think it should disallow requires too11:24
fwereade__wrtp, ISTM that we should disallow any juju-* name (or just "juju")11:24
fwereade__wrtp, but I'm not sure we have any reason to mess with the interface11:24
wrtpfwereade__: any juju- name11:24
fwereade__wrtp, so "juju" should be allowed?11:25
wrtpfwereade__: yeah, why not?11:25
fwereade__wrtp, because it is misleading to see a "juju-info-relation-joined" hook11:26
wrtpfwereade__: i'm not sure i see why11:27
fwereade__wrtp, I'm almost tempted to say that `starts with "juju"` should be the criterion11:27
wrtpfwereade__: why is juju-info-relation-joined misleading, even if there is a juju relation?11:28
fwereade__wrtp, hold on, it's crack anyway11:28
fwereade__wrtp, application-start has one of those11:28
fwereade__wrtp, but it'll never get run afaict11:29
wrtpfwereade__: why not?11:29
fwereade__wrtp, the only explicit relation it has is juju; it may run if it uses its implicit juju-info relation11:29
wrtpfwereade__: if you have a juju-info hook, then it must refer to the provider side, right? (assuming you're not allowed to explicitly declare a juju-info relation)11:30
wrtpfwereade__: so there's no ambiguity and... no problem?11:31
Aramwrtp: btw, regarding our discussing about " and =, = and == were in 10th edition unix, just catched my eye yesterday :).11:35
fwereade__wrtp, guess so... still worried about the inconsistencies on charm upgrade when we add such hooks11:36
Aramalso, the rc manual in 10th edition manual mentions plan 9, I wonder when did plan 9 development started.11:36
wrtpAram: yeah, i think i used "=" first; then i added "==" (without knowing about the 10th edition version); then i moved to rc and started using - and -- instead11:36
wrtpfwereade__: what kind of inconsistencies are you thinking about?11:36
wrtpfwereade__: it would be easy to disallow juju-info hooks in the first instance, anyway11:37
wrtpAram: i remember seeing plan 9 around just after i'd left university, around '91. charles had built his own clone of it (called Orbit) before it was made publicly available.11:38
Aramreally? did he make any code available?11:39
wrtpAram: i might have a pirated copy on magtape somewhere :-)11:40
wrtpAram: he got the whole comp sci year using it for a year or so11:40
fwereade__wrtp, most relations won't even be joined if the charm doesn't explicitly provide them; but the juju-info relation will be processed as usual11:50
fwereade__wrtp, if we upgrade to a version that does provide hooks, it won't see the expected events11:51
fwereade__wrtp, it will see departs of units that never joined, etc11:51
wrtpfwereade__: i see.11:51
fwereade__wrtp, this *might* not be a big deal in practice but it involves dropping guarantees that STM to be useful11:52
wrtpfwereade__: i think that that's probably good enough reason to disallow implementation of juju-* hooks11:52
wrtpfwereade__: it kinda makes sense that the system is providing all interactions with juju- relations.11:53
wrtpfwereade__: one could see it as implicitly declaring the necessary hooks11:54
wrtpfwereade__: so it doesn't make sense for a charm to redefine them11:54
fwereade__wrtp, yeah, this sounds reasonable11:55
fwereade__wrtp, that then means that we should disallow "juju" as a relation name as well, otherwise we won't be able to write juju-relation-joined hooks11:55
fwereade__s/otherwise/because/11:56
wrtpfwereade__: i'm not sure i follow11:56
fwereade__wrtp, disallow implementation of juju-* hooks => juju-relation-joined not ok => cannot meaningfully declare a "juju" relation11:57
wrtpfwereade__: i don't get the first inference. we disallow implementations of hooks for relations with a juju- prefix. but the relation name in juju-relation-joined is "juju" not "juju-"11:58
fwereade__wrtp, ah, I thought yu were advocating the simpler "no hook can start with `juju-`" rule11:59
fwereade__wrtp, which honestly I would prefer11:59
wrtpfwereade__: i think it's easier to think in terms of relation names12:00
wrtpfwereade__: (HasPrefix(name, "juju-") || name == "juju") is more complex and arbitrary, IMHO, than just the HasPrefix test12:00
fwereade__wrtp, I think specifying that no files in charm/hooks can start with "juju-" is the absolute simplest way to get what we need12:01
wrtpfwereade__: and i don't see that we'll ever need to define a "juju" relation.12:01
fwereade__wrtp, why should we allow others to?12:01
wrtpfwereade__: because it's a convenient name to use for the requirer side of a juju-info relation?12:02
fwereade__wrtp, doesn't this presuppose that we won't have other juju- relations?12:02
wrtpfwereade__: i don't think so.12:02
fwereade__wrtp, it's only a good name so long as juju-info is the only juju- one available12:02
wrtpfwereade__: a better name would probably be principal-info, right enough12:03
wrtpfwereade__: tbh i wouldn't mind if we just reserved all juju* names.12:04
fwereade__niemeyer, heyhey12:04
wrtpniemeyer: yo!12:04
fwereade__wrtp, that's what I'd prefer12:04
niemeyerGood morning!12:04
fwereade__niemeyer, I was wondering, did you see the prereq of the CL you approved last night? I have a suspicion it may have got caught in the miscommunication12:08
niemeyerfwereade__: Let me check12:08
fwereade__niemeyer, https://codereview.appspot.com/6678046/12:08
fwereade__niemeyer, you have reviewed it once butthat was a while ago12:08
niemeyerfwereade__: Ah, sorry.. I didn't realize it was a pre-req.. I reviewed the other one in an attempt to unblock you, but I has fail12:09
fwereade__niemeyer, np at all, I have other things I have been thinking about usefully12:09
fwereade__niemeyer, I will definitely want a call about charms and relations at some point12:10
niemeyerfwereade__: I'm game at any time12:10
fwereade__niemeyer, it's just that I'm getting peckish and will probably disappear for luch soonish ;p12:10
niemeyerfwereade__: Sounds like a good plan :-)12:10
fwereade__niemeyer, ok, I will be back in a little while, hopefully the various issues will settle into some sort of order in my mind :)12:12
wrtpniemeyer: a bootstrap can "succeed" several times, because the only way it can find out if the environment is already bootstrapped is by reading the environment's storage, which might not be available because of eventual consistency.12:14
wrtpniemeyer: it's possible that moving to using tags might help this issue, but i'm not sure.12:14
niemeyerwrtp: We shouldn't allow for that to happen12:15
niemeyerwrtp: Otherwise it's ad-hoc12:15
wrtpniemeyer: do you have an idea for how we might prevent it?12:16
wrtpniemeyer: we could put a sleep at the start of Bootstrap, i suppose12:16
niemeyerwrtp: We can read the information we just wrote, for example12:16
niemeyerwrtp: Before confirming it worked12:16
wrtpfwereade__: how does that help?12:17
wrtpniemeyer: how does that help?12:17
wrtpfwereade__: (didn't mean to address that to you, sorry)12:18
wrtpniemeyer: BTW could we have a chat about TLS certs some time today?12:18
niemeyerwrtp: Well, the idea is ensuring data we just wrote is visible12:21
wrtpniemeyer: doing that does not ensure that, unfortunately12:21
wrtpniemeyer: a read can succeed and then fail12:21
wrtpniemeyer: (there's one live test that prints a *sigh* message when that happens, and i see it reasonable often)12:22
wrtpniemeyer: it would probably reduce the frequency of Bootstrap succeeding twice in succession, but i don't think it would stop it entirely.12:23
niemeyerwrtp: It's not just about Bootstrap.. this isn't right either:12:26
niemeyer                r, err = storage.Get(name)12:26
niemeyer                if err == nil {12:26
niemeyer                        break12:26
niemeyer                }12:26
wrtpniemeyer: i was just looking at that12:26
wrtpniemeyer: yes, that's wrong.12:26
wrtpniemeyer: storage.Get does the retry anyway, so it's unnecessary for the tests to do that12:26
niemeyerwrtp: Eventual consistency sucks to deal with, and we should take the bullet to offer people a sane API12:26
wrtpniemeyer: i agree.12:26
wrtpniemeyer: but in some cases i'm not sure it's possible12:27
niemeyerwrtp: If accessing data is *entirely* random, then I can't see why people use S312:27
wrtpniemeyer: it's not entirely random. it works... eventually.12:27
niemeyerwrtp: Yep, so we should wait until it does12:28
wrtpniemeyer: for the record, i see the *sigh* message relatively often.12:28
wrtpniemeyer: we do!12:28
wrtpniemeyer: but in these cases, the request succeeds, so there's nothing to wait for12:29
niemeyerwrtp: Obviously a request succeeding doesn't seem to mean much12:29
wrtpniemeyer: if we delete a file, we can sometimes fetch it even after the delete request. that request is succeeding, but it shouldn't. it will fail eventually though, but the thing doing the Get can't know that it was expected to fail and hence retry.12:30
niemeyerwrtp: I understand what eventual consistency means, and I also understand that S3 has it12:32
wrtpniemeyer: i'm not sure how you expect me to fix the problem while removing the loops in jujutest then12:34
niemeyerwrtp: I'm thinking as well12:39
niemeyerwrtp: So, I think the issue is on loading, rather than on writing12:42
niemeyerwrtp: It'd suck to wait a long time for things to show up when we're writing sequentially several charms or whatever12:43
niemeyerwrtp: But it seems that in general files we read from S3 are in well known locations, that should be there12:43
wrtpniemeyer: i don't yet see where you're going with this12:44
niemeyerwrtp: Bootstrap doesn't tell you that there was a previous bootstrap because it fails to get the previous file12:44
niemeyerwrtp: We already have a retry strategy on Get.. why is it not working?12:45
wrtpniemeyer: look at the start of the Bootstrap method12:45
wrtpniemeyer: we fail when the Get *succeeds*12:45
niemeyer<niemeyer> wrtp: Bootstrap doesn't tell you that there was a previous bootstrap because it fails to get the previous file12:46
wrtpniemeyer: yeah, i was wrong there12:46
wrtpniemeyer: it can only know if there was a previous bootstrap if it can get the file which was created by the previous bootstrap.12:47
niemeyerYep12:47
wrtpniemeyer: actually, Bootstrap should do the loop itself12:47
wrtpniemeyer: because it knows that it's expecting the file not to be there.12:48
wrtpniemeyer: that would erase the jujutest bootstrap loop at any rate, if not the others.,12:48
wrtpniemeyer: it means Bootstrap will always take 5 seconds when the environment is already bootstrapped, but that might not be a problem.12:52
niemeyerwrtp: Hmm12:52
niemeyerwrtp: Why would it do the loop itself?12:53
niemeyerwrtp: Get does the loop already12:53
wrtpniemeyer: Get only loops if the file is not found12:53
niemeyerwrtp: We just have to improve Get until it works more reliably12:53
niemeyerwrtp: Yes, and that's what we need12:53
wrtpniemeyer: in this case the file *is* found, so it won't loop12:53
wrtpniemeyer: and that's a problem for Bootstrap, because it gives an error when the file is found.12:54
niemeyerwrtp: If the file is found, Bootstrap can fail12:54
wrtpniemeyer: exactly.12:54
niemeyerwrtp: Immediately12:54
niemeyerwrtp: I mean it *can* fail12:54
wrtpniemeyer: that's a problem if we do a Destroy followed by a Bootstrap.12:54
niemeyerwrtp: Destroy kills the whole bucket12:55
wrtpniemeyer: yes, but we might still be able to fetch that bucket even after it's been killed12:55
wrtpniemeyer: which will cause Bootstrap to fail inappropriately.12:55
niemeyerwrtp: Okay, so perhaps we should make Destroy more reliable instead12:55
niemeyerwrtp: Because that's the weak link12:56
wrtpniemeyer: so we make Destroy always take 5 seconds?12:56
niemeyerwrtp: We can have slightly more deterministic logic by having it retry until it can no longer see the file in a few different tries12:57
niemeyerwrtp: That means we don't slow down tests much12:57
wrtpniemeyer: that doesn't necessarily mean that a subsequent fetch won't still succeed12:57
niemeyerwrtp: Heh12:57
niemeyerwrtp: It won't ever mean that, whatever we do12:57
niemeyerwrtp: Even hours later S3 can still pull off a TA-DA! moment12:58
niemeyerwrtp: But we can at least try to bring some sanity12:58
wrtpniemeyer: yeah12:58
wrtpniemeyer: but it means we still need the Bootstrap loop if we're to make the tests reliable12:58
niemeyerwrtp: I don't see why.. bootstrap uses loadState which uses Get12:59
wrtpniemeyer: ... which can succeed even after the bucket has been destroyed and we've verified that we get a 404 error.12:59
niemeyerwrtp: We've just said we'd improve Destroy?13:00
niemeyerwrtp: What has to change in Bootstrap?13:00
wrtpniemeyer: that's the "verified that we get a 404 error" bit13:00
wrtpniemeyer: nothing has to change in Bootstrap. i'm talking about the loop in the test that calls  Bootstrap.13:00
wrtpniemeyer: (the one that triggered this discussion)13:00
niemeyerwrtp: Erm..13:00
niemeyerwrtp: Yes.. I'm a bit lost now.. all we've been saying above is to make things more reliable13:01
niemeyerwrtp: If we still have to loop in tests, the whole point is moot13:01
wrtpniemeyer: yes, but it won't be that reliable, even with the change above.13:01
niemeyerwrtp: Why?13:01
wrtpniemeyer: because verifying that the Get fails after Destroy doesn't verify that get Get called in Bootstrap won't subsequently succeed.13:02
wrtps/get Get/the Get/13:02
wrtpniemeyer: personally, i *think* the loop in the test is the lesser evil. much better would be if we could talk to *something* in amz that was fully consistent.13:05
niemeyerwrtp: If the Get sequentially fails several times at Destroy, and then Bootstrap can see the file again, too bad.. let the test blow13:05
niemeyerwrtp: Much better is if we don't use S3 at all.. that's where we have to go13:06
wrtpniemeyer: how many times do we try? what if this causes the test to fail 10% of the time?13:06
niemeyerwrtp: We improve it until it's 1% or 0.01%13:06
wrtpniemeyer: i'm not entirely sure that avoiding S3 will help here.13:06
niemeyerwrtp: Erm?13:06
niemeyerwrtp: Everything we've been talking about for the past hour is about S313:07
wrtpniemeyer: what's the alternative? tags? surely they'll suffer from a similar problem?13:07
niemeyerwrtp: An internal storage13:07
wrtpniemeyer: how does that help with Bootstrap?13:07
niemeyerwrtp: True..13:07
niemeyerwrtp: Either way, let's not derail13:07
niemeyerwrtp: The test is great.. it's showing the API is flaky.. there's nothing to fix there13:07
wrtpniemeyer: what about the other tests that loop?13:08
niemeyerwrtp: Same thing.. we have that shows storage.Get failing, and you say it fails often13:08
niemeyerwrtp: Sounds to me like we should improve storage.Get too13:08
wrtpniemeyer: by causing it to fetch several times?13:09
wrtpniemeyer: it does already retry on error13:09
niemeyerwrtp: Or perhaps try more often13:09
niemeyerwrtp: We should have a live test specifically for it, showing whether it works reasonably well or not13:10
wrtpniemeyer: ISTM that these tests are doing something that people won't be doing much in normal usage - we don't really care if something succeeds when it shouldn't most of the time.13:10
niemeyerwrtp: ISTM that we don't know how people will be using it13:10
niemeyerwrtp: destroy+bootstrap is not uncommon at all, for example13:11
wrtpniemeyer: i'm not sure what the Get test you're suggesting would do, and how Get might be improved. i don't think we want to retry on Get. nor, probably do we want to slow down every Put or Delete by doing a sequence of Gets after it.13:14
wrtps/retry on Get/retry when Get succeeds/13:15
niemeyerwrtp: It would verify how reliable Get looks like when we put something13:16
niemeyerwrtp: We want to solve the instability you see in the test by fixing the API, rather than by looping in the test13:17
niemeyerwrtp: We want to avoid looping in the test, because it shows very unstable behavior in the API itself.. we don't want that to be our high-water mark for all the providers.13:18
wrtpniemeyer: i'd like to fix the API (and we *have* fixed it when things fail), but i'm not sure how we can fix it without slowing everything down to full-eventual-consistency pace.13:18
niemeyerwrtp: I've just explained13:19
wrtpniemeyer: i fully agree that it's unfortunate13:19
wrtpniemeyer: ok, so suppose i write the test and it shows that in 40% of cases, a Get succeeds when it should not.13:19
wrtpniemeyer: what then?13:19
niemeyerwrtp: Then that's REALLY BAD isn't it!?13:20
niemeyerwrtp: If Destroy+Bootstrap fails 40% of the times, we suck I think13:20
wrtpniemeyer: that's S3 for you :-(13:20
niemeyerwrtp: Nope.. that's juju developers for you13:20
niemeyerwrtp: S3 seems to work fine for a lot of people13:20
niemeyerwrtp: storage.Get succeeding is fine13:21
wrtpniemeyer: we can easily make Destroy+Bootstrap fully reliable by making Bootstrap wait for eventual consistency to resolve before returning an error, as Get does.13:21
niemeyerwrtp: It's the other case that is a lot more rare: there are very few spots where we care about content being *deleted*13:21
wrtpniemeyer: Get("nonexistent-thing") will currently always take 5 seconds.13:22
niemeyerwrtp: I don't see how that's relevant13:22
wrtpniemeyer: i agree. but that's what these tests are showing.13:22
niemeyerwrtp: Bootstrap uses loadState, which uses Get.. Bootstrap is fine as it is13:22
wrtps/showing/testing/13:23
wrtpniemeyer: our current technique for avoiding eventual consistency issues is to try to avoid errors.13:24
wrtpniemeyer: so, if we're about to return an error, we try a few times to make sure that we've *really* got an error.13:24
wrtpniemeyer: ISTM that Bootstrap fits into that category.13:24
niemeyerwrtp: Sorry.. can we get to actual use cases so we can start to funnel the conversation towards agreement?13:25
niemeyerwrtp: We're talking for more than an hour, so it's time to reach some conclusions13:25
niemeyerwrtp: Destroy+Bootstrap fails and we want it to work..13:26
niemeyerwrtp: We can make that more reliable by having Destroy wait until the file at least looks gone13:27
wrtpniemeyer: my suggestion is to make Bootstrap not fail until the bootstrap bucket definitely isn't disappearing13:27
niemeyerwrtp: I don't know what that means13:28
niemeyerwrtp: Probably because thre are three negatives13:28
wrtpniemeyer: your suggestion is *more* reliable, but it still isn't reliable. i believe my solution is reliable (well, as reliable as our Get heuristics)13:28
wrtpniemeyer: this is what i suggest for the start of the  Bootstrap method :http://paste.ubuntu.com/1285011/13:29
niemeyerwrtp: We're almost in agreement13:30
wrtpniemeyer: the down side is that a failed Bootstrap gets slower. but that's what happens with a failed Get too, so comparable cases, i think.13:30
niemeyerwrtp: The difference is that you're doing the verification that should be dong in Destroy within Bootstrap13:32
niemeyerwrtp: That's effectively what this is doing.. it's waiting until Destroy actually takes place.. we can do that in Destroy itself13:33
wrtpniemeyer: i'm not sure we can do that and ensure that the subsequent Bootstrap will succeed, without waiting for the full eventual consistency timeout.13:34
wrtpniemeyer: and perhaps that's actually best - we'd just delay destroy-environment for a while.13:35
niemeyerwrtp: That's true13:36
niemeyerwrtp: Okay, sounds good13:36
wrtpniemeyer: so we add a sleep to Destroy?13:36
niemeyerwrtp: What? :)13:37
wrtpniemeyer: or... what sounds good to you?13:37
niemeyerwrtp: Your proposal..13:37
wrtpniemeyer: ok, cool.13:37
niemeyerwrtp: I think storage.Get likely deserves some improvement too to sort out that issue you see frequently13:38
wrtpniemeyer: i'll still need to leave the loops in some of the other jujutest tests, i think.13:38
niemeyerwrtp: I'd prefer to nail down problems rather than looping13:38
niemeyerwrtp: For all the reasons we already covered13:38
niemeyerwrtp: We're nailing one of them13:38
wrtpniemeyer: i only see it frequently because the test is checking Get after Delete, which i don't think is something that we care too much about in real-world code.13:38
niemeyerwrtp: Let's solve that first one, and then we see the others13:39
niemeyerwrtp: So let's not do that13:39
wrtpniemeyer: remove that test?13:39
niemeyerwrtp: Well, I suppose this is testing Delete?13:39
wrtpniemeyer: yes13:39
wrtpniemeyer: there's also one in the test that tests List, i think.13:40
niemeyerwrtp: List is a tough one, because we don't know what we're waiting for..13:41
wrtpniemeyer: in fact it's all in TestFile13:41
wrtpniemeyer: yup13:41
niemeyerwrtp: I guess those loops for the storage method itself are fine, at least for now13:43
niemeyerwrtp: It's the environment interaction we care the most about13:43
wrtpniemeyer: yeah13:43
wrtpniemeyer: i'm happy to have Bootstrap work a little more reliably.13:43
niemeyerwrtp: Me too13:44
niemeyerwrtp: I think storage.Get may need some tweaking too13:44
niemeyerwrtp: But we'll see13:45
wrtpniemeyer: what kind of tweak are you thinking of?13:45
niemeyerwrtp: Perhaps just reducing the time between retries13:45
niemeyerwrtp: So we don't increase the overall time further but improve the method a tad13:45
wrtpniemeyer: i'm not sure that helps here. we're talking about the case where it doesn't retry at all, because the Get succeeds.13:46
wrtpniemeyer: we could retry even when the fetch succeeds, but i *think* that's unnecessary.13:47
niemeyerwrtp: I was considering something else, but it doesn't really matter right now.. sorry for the noise.13:47
wrtpniemeyer: np13:47
wrtpniemeyer: i'm sorry i'm bad at explaining things!13:48
niemeyerwrtp: I don't think you're bad at explaining things.13:49
wrtpniemeyer: it felt like i wasn't explaining things well above, but we got there!13:51
niemeyerwrtp: I don't think it was a problem in the explanation..13:53
niemeyer<wrtp> niemeyer: nothing has to change in Bootstrap. i'm talking about the loop in the test that calls  Bootstrap.13:54
niemeyerwrtp: That was 1h ago..13:54
niemeyerwrtp: It took 1h for us to agree to solve the actual problem without looping in tests.13:54
wrtpthat was in response to talking about making Destroy try the get, i think13:56
fwereade__niemeyer, take a look at http://paste.ubuntu.com/1285056/ -- I think it roughly covers the areas I'm thinking about13:56
niemeyerwrtp: <wrtp> niemeyer: i'd like to fix the API (and we *have* fixed it when things fail), but i'm not sure how we can fix it without slowing everything down to full-eventual-consistency pace.13:58
niemeyerwrtp: Etc etc13:58
wrtpniemeyer: to be fair, i had suggested the current solution some time before my "nothing has to change" remark13:59
wrtpniemeyer: and our API is still broken (Bootstrap is less so now, happily), and we can't fix it13:59
niemeyerwrtp: Nevermind.. You rock.14:00
niemeyerfwereade__: Checking it out14:00
* wrtp wishes he did14:00
niemeyerfwereade__: 0) that seems crazy indeed14:01
niemeyerfwereade__: The name is precisely the way in which the charm uniquely identifies the relation14:01
fwereade__niemeyer, cool14:03
wrtpniemeyer: this might do it: https://codereview.appspot.com/6696043/14:04
niemeyerfwereade__: 1a) +114:04
wrtpniemeyer: (waiting on live tests to complete a few times before i believe it)14:04
niemeyerfwereade__: 1b) "but allowing freedom to implement any interfaces": I don't think we should allow people to implement juju-* interfaces14:04
fwereade__niemeyer, they have to implement it to talk to juju-info14:05
fwereade__niemeyer, (whose interface is also juju-info)14:06
niemeyerfwereade__: Okay, that's requiring the interface, specifically14:06
niemeyerfwereade__: Although, you could say that means implementing it too14:07
niemeyerfwereade__: So my bad in the wording14:07
fwereade__niemeyer, and honestly I don't see any reason to *stop* people from implementing non-juju-namespaced relations that happen to be acceptable substitutes14:07
niemeyerfwereade__: What I meant is that we shouldn't allow people to provide juju-info14:07
fwereade__niemeyer, it'd be kinda dumb but harmless I think14:07
fwereade__niemeyer, ah, not harmless then?14:07
niemeyerfwereade__: No, juju-* is reserved14:08
niemeyerfwereade__: We shouldn't break people's charms if we decide to implement juju-mama tomorrow14:08
niemeyerfwereade__: If we don't reserve it, we can14:08
niemeyerWe can break, I mean14:08
fwereade__niemeyer, ok, sgtm14:09
fwereade__niemeyer, so: you can't call a relation juju or juju-*; you can't provide an interface called juju or juju-*; anything else is ok?14:09
niemeyerfwereade__: +114:12
niemeyerfwereade__: 1c) Charm metadata is definitely not the place to insert implicit relations14:13
niemeyerfwereade__: Otherwise they'd not be implicit14:13
niemeyerfwereade__: If we inserted, that would affect several places we don't want to touch14:14
niemeyerfwereade__: e.g. the store14:14
niemeyerfwereade__: and it'd also mean that old charms don't get new implicit relations14:14
fwereade__niemeyer, that applies to the metadata file... when about the Meta type?14:15
* fwereade__ regards "when about" with horror14:15
niemeyerfwereade__: LOL14:15
niemeyerfwereade__: I was about to put that in my vocabulary.. you have to watch out when you talk to me14:16
fwereade__niemeyer, (actually, derail for now, it will be relevant again soo)14:16
fwereade__niemeyer, nah, just an incompetent edit14:16
niemeyerfwereade__: Meta reflects the metadata14:16
niemeyerfwereade__: It's actually stored in the store14:16
niemeyerfwereade__: a terrible evil will fall upon us if we hack that nice piece of immutable information14:17
fwereade__niemeyer, right, cool, the "doesn't feel right" is accurate14:17
niemeyerfwereade__: Okay, in sync, so going to 214:18
niemeyerfwereade__: 2a) +1.. I think that was covered in 1b14:18
fwereade__niemeyer, yeah, there is some overlap14:18
niemeyerfwereade__: I mean, +1 to the (no)14:18
fwereade__niemeyer, cool14:18
niemeyerfwereade__: 2b) Perhaps nothing.. let's handle them silently within the uniter if it reaches it, and blow up at the door when bundling/loading the charm14:19
niemeyerSo the uniter doesn't have to know about these conventions14:20
fwereade__niemeyer, in code terms, I was thinking of just refusing to load such abominations14:20
fwereade__;)14:20
niemeyerfwereade__: That's what I meant.. we don't have to special case in the uniter, because we don't bundle them14:21
fwereade__niemeyer, cool, perfect14:21
niemeyerfwereade__: 2c) Yes, people should certainly be able to run a hook for juju-info.. but it's not clear if that's the question you asked14:21
fwereade__niemeyer, oh ok -- I was expecting to be able to relate *to* juju-info, and run hooks in repsonse to a counterpart's juju-info, but I was not expecting charmers to implement, say, juju-info-relation-joined14:23
fwereade__niemeyer, what does that tell you other than "a subordinate exists"?14:23
fwereade__niemeyer, except it doesn't even tell you that14:23
fwereade__niemeyer, because juju-info has global scope14:23
fwereade__niemeyer, it just means "something you don't know about is in a relation with you"14:24
niemeyerfwereade__: Okay, let's break that apart14:24
fwereade__niemeyer, and I can't figure out how that information is useful to anyone14:24
niemeyerfwereade__: juju-info-relation-joined means the relation *name* is juju-info, which is.. hmm.. interesting14:25
niemeyerfwereade__: We said before we'd disallow it, but I'm not sure we should, now that I think of it14:25
fwereade__niemeyer, I have one concern there14:26
niemeyerfwereade__: First, because it doesn't really matter.. that's the user-provided name for the relation14:26
niemeyerfwereade__: We don't really care, I think14:26
niemeyerfwereade__: Second, because we offer convenient shortcut notation in charms that make relation-name == relation-interface14:26
niemeyerfwereade__: So, I think relation *names* that have juju* sound okay, in principle. What do you think?14:27
fwereade__niemeyer, the specific thing I want to block is an author who responds to changes he shouldn't know about, by doing things in response to the implicit juju-info relation changing14:28
niemeyerfwereade__: Before we derail, does the above sound sane?14:28
fwereade__niemeyer, blocking juju entirely seemed like the simplest and clearest way to ensure the situation didn't come up in future14:28
niemeyerfwereade__: is there a reason to prevent a relation *name* from being named juju*14:28
niemeyerfwereade__: Blocking a relation name doesn't do anything.. it has absolutely no semantics attached to it14:29
niemeyerfwereade__: Other than being an identifier14:29
fwereade__niemeyer, I thought it want just a reserved-for-=future-use deal14:29
fwereade__niemeyer, s/want/was/14:29
* fwereade__ is typing, er, "dynamically" today14:30
niemeyerfwereade__: The point I'm considering is that: a) It's useful to have it because we have syntax that makes relation-name == relation-interface convenient, and we have relation interfaces called juju-*;14:30
niemeyerfwereade__: b) There's nothing to reserve. It's an identifier without any semantics attached to it other than being a way to reference the relation by the local charm author14:31
fwereade__niemeyer, ok, we have one relation interface called juju-info, at this stage, that *everything* provides14:31
niemeyerfwereade__: interface != name14:31
fwereade__niemeyer, if we do a shortcut "juju-info" relation then the name will collide14:31
niemeyerfwereade__: Everything I said above is about the relation name, not the interface14:32
fwereade__s/interface //14:32
fwereade__niemeyer, we can't allow juju-info because the name collides with the implicit relation14:32
niemeyerfwereade__: If a name collides, it will blow up.. that's a separate constraint that should necessarily be enforced for every relation, despite its name14:32
fwereade__niemeyer, if we allow juju-* relation names we are setting ourselves up for future collisions14:33
niemeyerfwereade__: I don't see how that's relevant at all14:33
niemeyerfwereade__: Or perhaps, more clearly, I don't see how we'd be setting ourselves up for future collisions14:34
niemeyerfwereade__: Relation names are a local namespace14:35
fwereade__niemeyer, ISTM that we will not be free to pick sensible names for future implicit relations, because other charms might already be using those names for something different14:35
niemeyerfwereade__: I don't think that's true14:35
niemeyerfwereade__: Can you provide a short example? It'll elucidate the point14:36
fwereade__niemeyer, imagine we wanted to introduce juju-info, but charms already existed which use that relation name... what is the appropriate course of action? break the charms, or just call it "juju-inof-no-really-this-is-the-official-one"? ;p14:37
fwereade__niemeyer, I don't know what other implicit relations we may want to introduce14:37
niemeyerfwereade__: We introduce the *interface* juju-info, and absolutely nothing breaks at all14:37
fwereade__niemeyer, so what do we name the relation then?14:38
niemeyerfwereade__: We don't.. it's not our name.. we don't care14:38
fwereade__niemeyer, it *is* our name14:38
fwereade__niemeyer, we implicitly provide juju-info14:38
niemeyerfwereade__: Ahh, I see. You're wondering about the provider side14:39
niemeyerfwereade__: It's a good point.. hmm14:39
fwereade__niemeyer, I'm a little bewildered - I *think* I'm just reiterating that names should be unique within a charm -- across roles14:39
niemeyerfwereade__: across roles?14:39
fwereade__niemeyer, a provider called foo and a requirer called foo?14:40
niemeyerfwereade__: Ah, no no.. that's definitely not ok14:40
niemeyerfwereade__: Must be unique14:40
niemeyerfwereade__: I just never thought about the charm referencing the provider relation locally14:40
niemeyerfwereade__: The implicit one, that is14:40
niemeyerfwereade__: Again, it's a good point.. it just escaped me14:40
fwereade__niemeyer, yeah, I only came across it this morning14:41
niemeyerfwereade__: So here is a suggestion that preserves my original intention, and I think sorts out the problem you bring: we do allow juju* relation names, as long as they match the interface name14:41
niemeyerfwereade__: In other words, it's fine for a requirer juju-info relation to be named juju-info14:42
* fwereade__ thinks a bit14:42
fwereade__niemeyer, I'm not sure that helps14:42
niemeyerfwereade__: Ah, indeed14:42
fwereade__niemeyer, what does `svc1:juju-info` refer to?14:42
niemeyerfwereade__: requirer and provider would conflict14:43
niemeyerDmad14:43
niemeyerDamd14:43
niemeyerfwereade__: Okay, +1 on not allowing it.. it's clearly non trivial and I'm digging a hole14:44
fwereade__niemeyer, cool, cheers14:44
niemeyerfwereade__: Okay, another point on your comment: juju-info has global scope14:45
niemeyerfwereade__: I don't think that's the case14:45
niemeyerfwereade__: The *relation* has whatever scope the requirer gives it14:45
niemeyerfwereade__: Otherwise we'd not be able to use juju-info for the very reason it was created14:45
fwereade__niemeyer, ok, yes -- what I mean is that it *allows for* globally scoped relations, and I'm not sure it has any value in that instance14:46
fwereade__niemeyer, I am almost certainly missing something here though14:46
niemeyerfwereade__: It provides the ip address of the related unit14:46
niemeyerfwereade__: So there's *some* value14:46
fwereade__niemeyer, yeah, I guess we could have an automated try-to-hack-this-box charm we could relat to everything :014:47
fwereade__niemeyer, ok, objections withdrawn :)14:47
niemeyerfwereade__: That said, I'm not sure about why this is relevant to be honest.. it's just another relation, that can be global or container, and behaves like anything else in that sense14:47
niemeyerfwereade__: Am I missing something?14:47
fwereade__niemeyer, I think it's just a derail, sorry :)14:48
niemeyerfwereade__: Indeed, but given that we've derailed.. can we agree it's just like any other relation in that sense?14:48
niemeyerfwereade__: I just want to make sure I'm not missing yet another aspect14:49
fwereade__niemeyer, yes, certainly14:49
niemeyerfwereade__: Cool, thanks14:49
niemeyerfwereade__: (to me there's value in things not being special :-)14:49
niemeyerSo, where were we..14:49
niemeyer2c was last I think14:50
niemeyerAh, and we didn't yet answer it14:50
niemeyerfwereade__: People should be able to write requirer hooks for juju-info relations14:50
fwereade__niemeyer, my feeling still leans toward "no" -- the idea of juju-info, in particular, is that it's what yu relate to when the other charm doesn't know anything about you14:51
fwereade__niemeyer, ok, yu mean relations with the juju-info interface, not the name, right?14:51
niemeyerfwereade__: Yes14:51
fwereade__niemeyer, I have no arguments there14:51
fwereade__niemeyer, but that will not be a hook called juju-info-anything14:51
niemeyerfwereade__: -1 as well on juju-info *provider* hooks14:51
fwereade__niemeyer, it might be called, say, principal-info-something14:52
niemeyerfwereade__: I don't think the name of the hook matters much14:52
niemeyerfwereade__: It could be called foo-relation-joined and still be a juju-info hook14:52
fwereade__niemeyer, ok, I am thinking from the name perspective here, because those are the things that can lead to collisions14:52
niemeyerfwereade__: We've already decided on the name14:53
niemeyerfwereade__: I'm talking about relation hooks that respond to relations with the juju-info interface14:53
fwereade__niemeyer, let me restate what I'm wondering14:53
fwereade__niemeyer, given the relation naming restrictions, would it be ok for us to just declare the whole juju namespace in the hooks dir out of bounds?14:54
fwereade__niemeyer, if we *don't* then we run the risk of one of those names matching a future hook and firing as a hook when it's not expected14:54
fwereade__niemeyer, that is always a risk with any file in that dir to be fair14:55
niemeyerfwereade__: That doesn't look like 2c14:55
fwereade__niemeyer, ok, smaller question14:56
fwereade__niemeyer, is it ever sane to have a file named "juju-info-relation-joined" in your hooks dir?14:56
niemeyerfwereade__: It does the exact same thing as a file named can-I-has-catz14:56
fwereade__niemeyer, how do we stop it from running? special-casing in the uniter?14:58
niemeyerfwereade__: Why would we have to special case?14:58
niemeyerfwereade__: It does absolutely nothing.. there's nothing interesting about that name14:58
fwereade__niemeyer, it will be executed when the provider joins the relation, won't it?14:58
fwereade__niemeyer, unless we explicitly avoid running all such hooks14:59
niemeyerfwereade__: Aha, okay..14:59
niemeyer<niemeyer> fwereade__: -1 as well on juju-info *provider* hooks14:59
niemeyerfwereade__: So you want to avoid these hooks by scanning the directory and banning everything hooks/juju*?15:01
fwereade__niemeyer, essentially, yes15:01
fwereade__niemeyer, or possibly explicitly ignoring them in the uniter15:02
fwereade__niemeyer, either way smacks of icky special-casing, not sure which is worse15:02
niemeyerfwereade__: That sounds like a good idea15:02
niemeyerfwereade__: I think we should do better, in fact15:03
niemeyerfwereade__: Ban *all* unwanted files from hooks/*15:03
niemeyerfwereade__: We can't do that in the short term, but we should start warning people about that asap15:03
fwereade__niemeyer, hmmmmmmm a lot of people use a nunch of symlinks to a single implementation file15:03
niemeyerfwereade__: That works, as long as the implementation file is one of the hooks15:03
fwereade__niemeyer, can't remember offhand, but I'm not sure it is15:04
niemeyerfwereade__: Understood.. I'm sure it'll break what people do right now15:04
fwereade__niemeyer, (in general, in the cases I've seen...)15:04
niemeyerfwereade__: Which is why we cannot push in the short term, but can start warning asap15:04
fwereade__niemeyer, ok, regardless this is a direction statement of which I approve, I will add some deprecation warnings15:04
fwereade__niemeyer, and also explicitly skip juju-* "hooks" in the uniter, I guess?15:05
niemeyerfwereade__: It avoids the future-hooks issue, and establishes a good convention on which the juju* ban works15:05
niemeyerfwereade__: I'd prefer to explicitly forbid *those* hooks right away15:06
niemeyerfwereade__: The uniter is too late15:06
niemeyerfwereade__: It'll blow people up way down the pipeline15:06
fwereade__niemeyer, ok, SGTM -- error on Read hooks/juju-*; warning on Read hook/anything-not-referenced-elsewhere?15:06
niemeyerfwereade__: +115:07
fwereade__niemeyer, ok, great15:07
niemeyerfwereade__: Wait, hmm..15:07
niemeyerfwereade__: There are forward compatibility issues I thnk15:07
niemeyerfwereade__: It'd mean an old juju version cannot deploy any charms that expose a new hook implementation15:08
fwereade__niemeyer, hmm15:08
niemeyerfwereade__: We don't have to solve that now15:09
niemeyerfwereade__: Let's agree on what seems clear: no hooks/juju*15:09
fwereade__niemeyer, perfect15:09
niemeyerAwesome15:09
niemeyer2c) Check15:10
niemeyer2d) Block them when reading?15:10
fwereade__niemeyer, +115:10
niemeyerCool15:10
fwereade__niemeyer, 2e, 2f covered15:10
niemeyer2e) Definitely not15:10
fwereade__niemeyer, no, block when reading15:10
niemeyerCool15:10
niemeyer315:10
niemeyer3a) Yes15:11
fwereade__niemeyer, note that we should be careful about charm store releases once we've tightened these up15:11
niemeyerfwereade__: Yeah, I think it's okay15:12
niemeyerfwereade__: But we'll see15:12
fwereade__niemeyer, at least the 2 footnoted charms will be refused, and probably others15:12
niemeyerfwereade__: 2b) I don't understand the point15:12
niemeyerfwereade__: Probably lost on "charm URLs match are for"15:13
fwereade__niemeyer, er, I apparently cannot haz craggar15:13
fwereade__er, grammer :/15:13
fwereade__GAAAH15:13
* fwereade__ takes a deep breath15:13
niemeyer:)15:14
fwereade__niemeyer, ISTM that the right way to accomplish 3a is to Assert that the service's charms are the same charms we used to determine that the endpoint list was OK15:14
fwereade__niemeyer, it shouldn't in itself be controversial, it's just setting up 3c15:14
niemeyerfwereade__: I don't think that's the case15:14
fwereade__niemeyer, cool, go on15:15
niemeyerfwereade__: I think I see what you mean, actually, and agree15:16
fwereade__niemeyer, ah, ok, great15:16
niemeyerfwereade__: We need to assert that the charm still has the relation15:16
niemeyerfwereade__: There's a red-herring in that description regarding the charm URL not having changed, which isn't the case, but it doesn't matter15:17
fwereade__niemeyer, please explain, I don;t see it15:17
niemeyerfwereade__: "charm URLs match are for the15:18
niemeyer    charms from which those endpoints have been determined"15:18
niemeyerfwereade__: The determination of the endpoints is done at time T1.. the adding of relation is done at T215:18
fwereade__niemeyer, ISTM that the services' charm urls (and lifes) are exactly what we need to assert are what we expect15:18
fwereade__niemeyer, agree15:19
niemeyerfwereade__: Things may change in between.. the important fact is that at T2 the relation is still sane15:19
fwereade__niemeyer, ok, indeed, we probably do want to retry the validation in that case15:19
niemeyerfwereade__: Cool, in sync again15:19
niemeyerSo, 3b check15:20
fwereade__niemeyer, yep15:20
niemeyerfwereade__: 3c) Doesn't seem entirely the case, for the reasons described15:21
fwereade__niemeyer, for the first attempt, it does seem wrong to redo the work we just did15:21
niemeyerfwereade__: I'm not entirely comfortable with changing what an Endpoint is because of that minor need15:22
niemeyerfwereade__: and endpoint is a high-level description of one side of the relation15:23
niemeyerfwereade__: It's not bound to any charm15:23
fwereade__niemeyer, yes, but we can see very neatly whether or not it will apply to a given charm15:23
Aramuniter tests fail because of:15:23
Aram[LOG] 4.30775 JUJU git command failed: exit status 115:23
Arampath: /tmp/gocheck-894385949183117216/0/agents/unit-u-0/charm15:23
Aramargs: []string{"pull", "/tmp/gocheck-894385949183117216/0/agents/unit-u-0/state/deployer/current"}15:23
AramM.juju-charm15:23
AramUdata15:23
AramMhooks/start15:23
AramAignore15:23
AramMrevision15:23
Aramanybody seen this?15:23
Aramthis is from trunk15:23
fwereade__niemeyer, this is actually maybe the point at which things will be clearest if you read to the end and then ask questions again15:23
fwereade__Aram, grar, would you paste me the full test output?15:24
Aramok15:24
fwereade__Aram, I suspect my unracing in a particular step was not as good as it might have been15:24
fwereade__niemeyer, it's definitely the part I am least certain about15:24
niemeyerfwereade__: Yeah, I understand.. it seems to do exactly what 3c suggests15:27
niemeyerfwereade__: IOW, associate an endpoint with a charm15:27
niemeyerfwereade__: This makes the model more complex in a few different ways15:27
fwereade__niemeyer, *optionally* do so, but yes15:27
niemeyerfwereade__: Not optionally.. always.. except we now have *two* endpoint types, so we must be qualifying them15:27
niemeyerfwereade__: All seems like changing something clear into something that requires further effort to understand and communicate15:28
niemeyerfwereade__: The need you expose seems relevant15:28
niemeyerfwereade__: But I think we can solve the need without refactoring what an endpoint means15:28
niemeyerfwereade__: In the short term, it sounds a lot easier to move forward by simply having AddRelation fail if the endpoints don't exist at the transaction time15:30
fwereade__niemeyer, ok...15:30
fwereade__niemeyer, the proposal also I think incorporates an additional perspective that I didn't mention15:30
niemeyerfwereade__: That's the common denominator of all of the points and interface changes, I think, so having that encapsulated under that one operation feels like a significant win15:31
fwereade__niemeyer, which is that I think I will be wanting to validate endpoints against *deployed* charms15:32
niemeyerfwereade__: Sounds sane15:33
fwereade__niemeyer, I think that endpoint matching/checking go very well together with charms15:33
fwereade__niemeyer, without having to involve state15:33
niemeyerfwereade__: charm.HasEndpoint(endpoint)?15:34
fwereade__niemeyer, nearly -- a container-scoped one should match a global one that the charm declares15:34
fwereade__niemeyer, actually yeah HasEndpoint covers that15:34
fwereade__niemeyer, the thinking is generally that this block of data, excluding the service name, appears to be relevant and useful in the charm package on its own15:35
niemeyerfwereade__: I don't think so, unless you change what an endpoint is15:35
niemeyerfwereade__: The concept of "endpoint" began its life as that "service:relationname" we use in the command line15:36
niemeyerfwereade__: To represent one side of the relation in an unambiguous way15:36
fwereade__niemeyer, hm, fair enough -- maybe I'm asking for a rethink of charm.Relation instead?15:37
fwereade__niemeyer, which lacks Role and something else15:37
niemeyerfwereade__: I don't know.. I don't know what we're after15:37
fwereade__niemeyer, I think it would be very useful to have a type, regardless of its name, which I could use with charms to determine what charms can talk to what other charms15:38
fwereade__niemeyer, at the moment RelationEndpoint roughly givesus that, but the service name is a distraction from that POV15:39
niemeyerfwereade__: Can we cover the problem we're trying to solve first?15:39
fwereade__niemeyer, I would like to be able to manipulate implementations of charm.Charm in such a way as to be able to easily ask questions about what can relate to what15:41
niemeyerfwereade__: This is still talking about the API you want.. can we talk about the *problem* first?15:41
Aramfwereade__: http://paste.ubuntu.com/1285248/15:41
Aramfwereade__: only one test failure in that log, if I run it without piping in a file I get more errors, if I pipe in a file I get only one error, or no failure at all.15:41
fwereade__niemeyer, the problem is charm upgrades15:42
niemeyerfwereade__: Okay.. what happens with charm upgrades?15:42
niemeyerfwereade__: The uniter has a single local charm that is deployed, and an upgrade candidate15:42
niemeyerfwereade__: What do we have to do about that/15:42
niemeyer?15:42
fwereade__niemeyer, I'm more thinking of a charm that has *not* yet been upgraded; detecting a relation that was added to the service since the service was upgraded, but which the local charm does not know about15:43
fwereade__niemeyer, I don't think the uniter needs to worry about upgrades15:43
fwereade__niemeyer, the idea is that we don't even allow upgrades if they would break relations that currently exist15:44
fwereade__niemeyer, but that, when we see a relation, we have no guarantee it's not for a newer version of the charm, to which we have not yet upgraded15:44
fwereade__niemeyer, and so by comparing the ... endpoint ... of the relation against the current charm we can know we shouldn't actually enter15:45
fwereade__niemeyer, so I would like a Thing representing the relation-from-charm-perspective, or endpoint-but-not-about-service, or whatever we call it -- that Name/interface/Role/Scope quartet15:47
niemeyerfwereade__: Okay.. so the goal is knowing that the established relation is not in fact known to the current charm, even though it's known to the service15:47
fwereade__niemeyer, yes15:47
niemeyerfwereade__: Awesome, thanks15:47
fwereade__niemeyer, (and also, taking a charm and determining what ... endpoints ... it exposes)15:47
niemeyerfwereade__: So how do we guarantee that?15:47
fwereade__niemeyer, sorry about the vocab, it's just that endpoint STM to be the losest word we have to the thing I'm trying to describe15:48
niemeyerfwereade__: Let's pleast not loose it15:48
niemeyerfwereade__: We have a well defined meaning for endpoint15:48
fwereade__niemeyer, yep, ok15:48
niemeyerfwereade__: Charms don't have endpoints.. they have relation names to uniquely identify the relation15:48
fwereade__niemeyer, the other word is "relation", which is unhelpful in this context because the name relation is more tightly bound to state in my mind than it is t charm15:49
fwereade__niemeyer, but it may be what we're after15:49
fwereade__niemeyer, Peers/Provides/Requires are all map[string]Relation15:49
niemeyerfwereade__: Interesting15:49
niemeyerfwereade__: Okay15:50
fwereade__niemeyer, the field and the key supply those 2 pieces of information15:50
niemeyerfwereade__: It's certainly not ideal, and I don't blame you for the confusion at all15:50
niemeyerfwereade__: I'm not clear enough myself15:50
fwereade__niemeyer, but the Relation type itself feels like it would be more useful if it included them15:50
niemeyerfwereade__: Back to the point, though15:50
fwereade__niemeyer, possible restatement: the unit of compatibility is the charm, not the service, and I am currently very interested in compatibility15:51
niemeyerfwereade__: How do we tell if a charm supports a relation that is compatible with what the state service is telling us?15:51
niemeyerfwereade__: What's the info that tells us whether we're good to go or not?15:51
fwereade__niemeyer, maybe interface and role is all we *need* there?15:52
niemeyerfwereade__: We must take the name in consideration too15:52
fwereade__niemeyer, ha, yes indeed, they need to match15:52
niemeyerfwereade__: Imagine cache-db vs. data-db15:52
fwereade__niemeyer, I'm not clear on scope for some reason15:53
fwereade__niemeyer, I don't think that's an issue of charm compatibility, I think that is a service-level thing15:53
niemeyerfwereade__: I think it matters too15:53
niemeyerfwereade__: If nothing else, because it means what is *established* needs to change15:54
fwereade__niemeyer, ah ok at runtime?15:54
niemeyerfwereade__: Yes, I'm still keeping the problem statement in mind:15:54
niemeyer<niemeyer> fwereade__: Okay.. so the goal is knowing that the established relation is not in fact known to the current charm, even though it's known to the service15:54
fwereade__niemeyer, feels like we just shouldn't allow upgrades that change the meanings of established relations at all15:55
niemeyerfwereade__: +115:55
=== hazmat is now known as kapilt
fwereade__niemeyer, (determining that is another use for the cabability I am touting)15:56
niemeyerfwereade__: And by established we mean non-Dead/removed15:56
fwereade__niemeyer, yep15:56
niemeyerAwesome15:56
niemeyerfwereade__: So, problem solved?15:56
niemeyerfwereade__: Hmm.. no15:57
niemeyerfwereade__: Because we can still establish a new relation15:57
fwereade__niemeyer, dunno -- you seem to be implicitly agreeing that the Name/Iface/Role/Scope quartet may be a sensible data type for several situations, but I'm not actually sure yu are15:57
fwereade__niemeyer, ah yeah -- we can establish a new relation while old charms are still not upgraded15:58
niemeyerfwereade__: Uh.. I'm not agreeing or disagreeing with that15:58
niemeyerfwereade__: I'm still trying to see what is the problem we have and solve it15:58
niemeyerfwereade__: So what we use to determine if a relation is known to the current charm, so that it may be established, is its <name,iface,role,scope>... okay15:59
fwereade__niemeyer, the problem is that we need to do compatibility-checking things with charms in several situations: determining possible endpoints; determining valid charm upgrades; ensuring sanity in an edge case on the uniter15:59
niemeyerfwereade__: I'm still trying to solve one single problem16:00
fwereade__niemeyer, I am looking for APIs that makes sense for all these needs, which feel like different applications of what should be the same tools16:00
niemeyerfwereade__: Sorry, I'm slow..16:00
niemeyerfwereade__: I cannot cover all needs at once.. we just determined that there should be no API changes at all for adding a relation16:01
niemeyerfwereade__: Now there's another case that I'm trying to follow up with you16:01
niemeyer<niemeyer> fwereade__: So what we use to determine if a relation is known to the current charm, so that it may be established, is its <name,iface,role,scope>... okay16:02
fwereade__niemeyer, sorry, wait, I'm not sure I did agree with that 100% -- I agreed that RelationEndpoint is the right data to keep in state, not that it's the best way to express our desires to AddRelation16:02
fwereade__niemeyer, I'm really not trying to be difficult here16:03
niemeyerfwereade__: That's not how it feels16:03
fwereade__niemeyer, the difficulty is in relations that do not correspond to those declared in the charm16:04
niemeyerfwereade__: Sounds good.. I was trying to get the context for that statement, and just now I think I'm understanding what you're trying to do16:05
niemeyerfwereade__: When we add a relation, we do so against the latest charm16:05
fwereade__niemeyer, sorry, I'm still feeling my way around it myself16:05
niemeyerfwereade__: that is associated with the services16:05
niemeyerfwereade__: We prevent upgrades that modify established relations16:06
niemeyerfwereade__: Which gives us a guarantee that any unit that has an *established* relation, has a charm with that specific relation matching that of the tip charm associated with the service16:07
niemeyerfwereade__: Which is comfortable and good and sane16:07
fwereade__niemeyer, yes, agreed16:07
niemeyerfwereade__: What we cannot guarantee, though, is that when we establish a *new* relation, that relation may be established by all running units that are not up-to-date with the latest charm16:08
fwereade__niemeyer, with a slight wrinkle in the case of the provider relation named juju-info, which is not declared explicitly by any charm, and it's not clear where it comes from16:08
fwereade__niemeyer, yes, agreed16:08
niemeyerfwereade__: The important fact here, then, is that a *new* relation that was established may be *incompatible* with the charm the uniter is running, and compatibility as we agreed is determined by <name,iface,role,scope>16:09
fwereade__niemeyer, yes, agreed16:10
niemeyerfwereade__: Awesome16:10
niemeyerfwereade__: Another important factor that is a consequence of some of these agreements, is that once we observe a relation in state, we have the guarantee that as long as that relation exists, it will remain compatible with the tip charm, whatever that charm is16:11
fwereade__niemeyer, yes indeed16:11
fwereade__niemeyer, (I *think* that actually *some* scope changes are safe... if the only existing relation is itself a downscoped global relation, it's safe for the underlying relation to go from global to container)16:14
fwereade__niemeyer, but that is just a detail16:14
niemeyerfwereade__: The problem feels straightforward then..16:15
niemeyerfwereade__: endpoint := relation.Endpoint(service) ; if endpoint supported by local charm { move on }16:16
fwereade__niemeyer, agreed...16:17
fwereade__niemeyer, the code I am trying to discuss is the " if endpoint supported by local charm" bit16:17
fwereade__niemeyer, which is the same sort of question I will be asking about various "endpoints" and charms in several situations16:18
niemeyerfwereade__: Sorry, I thought that was part of the coverage above16:19
niemeyerfwereade__: ... and compatibility as we agreed is determined by <name,iface,role,scope>16:19
niemeyerfwereade__: endpoint.SupportedBy(charm) comparing <name,iface,role,scope>?16:20
fwereade__niemeyer, ok, which we do not have collected into a handy type, other than RelationEndpoint, which has an extraneous service name -- if it didn't have that, it would live very happily as a type in the charm package16:21
niemeyerfwereade__: It's not extraneous.. it's part of the definition of what an endpoint is since we first used the endpoint term16:21
niemeyerfwereade__: and by charm I really mean *state.Charm16:22
niemeyerfwereade__: Or at least the charm interface16:22
fwereade__niemeyer, why *state.Charm? all we *should* need to answer the important questions is charm.Charm16:23
niemeyerfwereade__: Which might work with both16:23
fwereade__niemeyer,16:23
fwereade__niemeyer, yeah, in fact it's just Meta() I think16:23
niemeyerfwereade__: Cool.. charm.Charm still feels good, though16:23
fwereade__niemeyer, it does!16:23
niemeyerfwereade__: For docs, if nothing else16:23
fwereade__niemeyer, and being able to answer these sorts of questions in the charm package feels good too :)16:23
niemeyerfwereade__: Feels irrelevant to any problem stated so far16:24
fwereade__niemeyer, all the data is already available in the charm package... all the questions are purely about charm relation compatibility, divorced from service... it's just that we don't have a handy type in the charm package expressing it16:26
fwereade__niemeyer, this seems strange to me16:27
niemeyerfwereade__: There are only so many battles we can win at a time16:27
fwereade__niemeyer, when I consider what that type should look like, I see that it bears notable similarities to -- but is not the same as -- RE16:27
niemeyerfwereade__: If all we need is a trivial method on state.RelationEndpoint to say whether it is compatible with a charm or not, I don't see why we to break down the concept of endpoint we have today, split types, change the charm package, etc etc16:28
fwereade__niemeyer, and a way to get all those endpoints from charms16:29
niemeyerfwereade__: Why?16:29
niemeyerfwereade__: I haven't seen that need so far16:29
fwereade__niemeyer, add-relation foo bar16:29
niemeyerfwereade__: yes?16:30
fwereade__niemeyer, we need to go through all possible combinations to determine whether or not it's ambiguous16:31
niemeyerfwereade__: Yes, and that's in state16:31
niemeyerfwereade__: Because all of the data this applies against is in state16:31
niemeyerfwereade__: endpoints, err := state.InferEndpoints("foo", "bar")16:32
niemeyerfwereade__: relation, err := state.AddRelation(endpoints)16:32
niemeyerfwereade__: Sorry, I'm starving.. will be back soon16:34
fwereade__niemeyer, so at the very least we need to get the service twice and the charm twice for every service in a relation?16:34
fwereade__niemeyer, np, might be a little longer myself16:34
fwereade__niemeyer, ttyl16:35
=== kapilt is now known as hazmat
* wrtp is off for the evening. night all.17:53
niemeyerwrtp: Night17:55
wrtpniemeyer: please can we have a discussion about tls certs tomorrow...18:14
niemeyerwrtp: Of course18:14
=== andrewsmedina is now known as fuicomprarumciga
fwereade__niemeyer, tyvm for reviews21:01
fwereade__niemeyer, sorry that one was still a bit scrappy21:02
fwereade__niemeyer, and fwiw I don't feel a strong need to pursue the not-actually-Endpoints discussion -- in initial sketches the code seemed to be telling me it wanted to do that, but there's nothing fundamentally wrong with keeping the functionality in state, and I will try to cast it from my mind ;) :-)21:04
fwereade__niemeyer, ty for valuable discussion earlier :)21:05
niemeyerfwereade__: It was my pleasure21:18
niemeyerfwereade__: Also excited about the rest of the code21:18
niemeyerfwereade__: Lots of good stuff, and not much to change21:19
fwereade__niemeyer, excellent :)21:19
fwereade__niemeyer, a thought: is a container-scoped peer relation ever valid?21:27
niemeyerfwereade__: Wow..21:28
* niemeyer thinks21:28
fwereade__niemeyer, maybe with multi-endpoint peers it would be... although it sort of makes my brain hurt a little21:30
fwereade__niemeyer, it actually sounds icky enough that I'd like to forbid it, and we can consider the implications at a later date if anyone wants to use to use it for anything21:31
fwereade__niemeyer, I can't imagine any use for a container-scoped peer relation, even there, thant isn't better addressed as a separate pro/req relation21:33
fwereade__niemeyer, ofc, my imagination is limited ;)21:33
niemeyerfwereade__: I think it'd be pretty bizarre21:33
niemeyerfwereade__: THe only way it could happen is to have two subordinates of the same type21:34
niemeyerfwereade__: Since naturally we cannot have two principals in the same container21:34
fwereade__niemeyer, which I guess is not impossible, but pretty damn weird21:34
niemeyerfwereade__: Yeah21:34
niemeyerfwereade__: I'm not sure if we should prevent it actively, though21:35
niemeyerfwereade__: If we have to special case it to prevent, I'd say let's do nothing21:35
niemeyerfwereade__: If we have to special case to support, I'd say let's not support it21:35
fwereade__niemeyer, sgtm, ++less-code21:36
fwereade__niemeyer, btw, I was also just wondering if we really want to scan the hooks dir every time we read a charm... maybe it would be cleaner to check at bundle time?21:38
fwereade__niemeyer, sorry, context is for disallowing juju* files in the hooks dir21:38
fwereade__niemeyer, assuming that every charm we run has passed through a bundling stage does not seem unreasonable21:39
fwereade__niemeyer, but maybe it's not sensible, they are just zip files21:40
niemeyerfwereade__: Hmm21:40
niemeyerfwereade__: Yeah21:40
fwereade__niemeyer, scan-every-read then?21:43
niemeyerfwereade__: Sounds fine21:44
fwereade__niemeyer, yeah, could be worse ;p21:44
niemeyerfwereade__: True :)21:45
fwereade__niemeyer, I'm going to bed, but https://codereview.appspot.com/6713057/ should be nice and simple22:32
niemeyerfwereade__: Thanks much23:15

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