#smooth-operator 2020-03-23
<niemeyer> Good morning all
<niemeyer> Is everyone out today?
<jam> well, all except you and I
<niemeyer> :)
<niemeyer> Hi
<niemeyer> So yeah, we discussed that issue on Friday a bit
<niemeyer> Do you have the logs?
<niemeyer> Let me copy & paste into the PR for the record, and then we can cover it further
<niemeyer> Done
<niemeyer> Somehow this doesn't look like an active conversation..
<jam> sorry, otp will be back in 20 or so
<jam> back
<niemeyer> I'll step out for lunch.. we can catch up in the standup
<jam> thanks for posting the context in review.
<jam> ok
<jam> niemeyer: so your vote on github is still Needs Fixing, so it will block landing. I don't know if you want to change that to Comment and I'll have someone else do a review, or whether you're happy with where it is now.
<niemeyer> jam: That review point is still open, so I'm happy to have another look once you feel it's ready to go
<jam> niemeyer: enabled by default and use 'emit' was just pushed
<niemeyer> jam: Thanks.. I thought it wasn't implemented in terms of individual emitting methods yet.. glad it's almost ready
<niemeyer> I'm joining the snap standup and will be back shortly
<jam> so currently I haven't exposed a "emit_relation_changed" as an explicit user request (vs enabling 'update_relation' to trigger a relation changed event). If we want to expose the 'emit' functions we can, but we can also add that later.
 * Chipaca waves
<Chipaca> i'm only here for a couple of meetings today, but hiya
<niemeyer> o/
 * Chipaca EODs
<niemeyer> Have a good evening
#smooth-operator 2020-03-24
<Chipaca> good morning!
<niemeyer> Moin moin!
<Chipaca> jam: niemeyer: have you guys talked about the testing pr any more than what's on github?
<niemeyer> We did.. but I think we still misunderstood each other and need to sync again
<niemeyer> The enable events method with a charm instance was unexpected to me at least
<Chipaca> niemeyer: I think reading the __init__ method, the way charm is passed in changes
<Chipaca> niemeyer: ie in initialize(charm) instead of Harness(charm)
<Chipaca> bah, MyCharm*
<Chipaca> i think that's making things too granular ( â harder to use)
<Chipaca> and would prefer just __init__() doing the initalization work (could just call _initialize)
<niemeyer> I still don't get it.. wasn't the create_harness method receiving a charm type? Why did it change?
<Chipaca> I think it was but I might misremember
<Chipaca> https://github.com/canonical/operator/pull/146/commits/c2639df120efe2c56235511c814e71644dd640d0 is where that changed
<Chipaca> and I think on its own it makes sense
<Chipaca> that is: don't pass in the charm class, then do all the setup you need, and then initialize
<Chipaca> thus initialize kicks off events
<Chipaca> I don't think it makes sense in conjunction with explicit enable/disable methods
<Chipaca> especially with the default of enabled
<Chipaca> niemeyer: this also explains why enable takes the charm :-)
<Chipaca> in fact I think I like the approach of build without the charm, set up, then initialize the charm to start handling events; it feels very natural
<Chipaca> at this point enable/disable_events are handles for doing fancier stuff (but I don't see the need for passing in the charm to enable, beyond "disable sets it to None was the easiest way of disabling events"
<Chipaca> )
<niemeyer> Chipaca: I don't see how it makes sense..
<niemeyer> Chipaca: The charm isn't created by the developer in practice anyway
<niemeyer> Sorry, created is not a good term here
<niemeyer> I mean it's not instantiated
<Chipaca> niemeyer: i assume the no-make-sense bit is enable_events taking a charm instance?
<niemeyer> Was talking about the argument itself
<niemeyer> But yes, that's where the conversation started
<niemeyer> Also, disabling resetting it
<Chipaca> niemeyer: sorry, which argument?
 * Chipaca now lost
<niemeyer> Chipaca: The conversation we're having just now?
<Chipaca> niemeyer: ah :)
<Chipaca> i thought you meant argument to the method
<niemeyer> Ah, no, sorry
<Chipaca> niemeyer: yes, agreed, having disable/enable fiddle with the charm on the handler is weird / wrong
<Chipaca> niemeyer: but the constructor without the charm class, and then explicit initialize, feels alright to me
<Chipaca> niemeyer: it means in most cases you won't need to poke at enable/disable at all
<Chipaca> if we agree on that, then we can look at how to make enable/disable not fiddle with the charm instance
<niemeyer> I had to jump into a call at 10, but we can keep going in a bit
<Chipaca> if we don't, then the other coherent thing is to not have initialize, and just enable/disable, that makes the need to pass the charm instance in go away
<Chipaca> the code to not pass in the charm instance to enable/disbale is probably the same in both cases, and it's not pretty but not terrible either (unless i miss something)
<Chipaca> but it's all hidden from the dev, so that's alright
<Chipaca> the passing it in is a wart exposed to the dev, or at least it feels like it --- if there was a non-warty reason i don't see it
<Chipaca> no probs, i have things i need to read :)
 * Chipaca goes read
<niemeyer> > 10:01:28 niemeyer: but the constructor without the charm class, and then explicit initialize, feels alright to me
<niemeyer> If that's true I must be missing something about how we'll use it
<niemeyer> We went from "this really needs to return the two arguments because that's what we're testing" to "the charm can be created separately and needs to be fed manually by calling some method"
<Chipaca> niemeyer: it's initialize(MyCharm), not initialize(charm), right?
<niemeyer> What's "initialize"?
<Chipaca> niemeyer: instantiate the charm and plug it in to the framework?
<niemeyer> Sorry, I'm completely lost
<Chipaca> niemeyer: https://github.com/canonical/operator/pull/146/commits/c2639df120efe2c56235511c814e71644dd640d0#diff-6964ca15c08db5ba857f6517369815d9R72
<niemeyer> Yes, I don't get any of that
<niemeyer> In my mind it was "harness = Harness(MyCharm)"
<Chipaca> niemeyer: also with some yaml, right?
<niemeyer> Optionally I think, but yes
<Chipaca> niemeyer: and that does a couple of things: creates a framework that has a model with a testing backend, and instantiates the charm class hooked into that framework
<niemeyer> Yeah, it does all the boring boilerplate
<Chipaca> niemeyer: initialize() splits out the second of those two things into an explicit method
<niemeyer> Because....?
<Chipaca> niemeyer: because then you can tweak the model by adding relationships and whatnot, without having the charm hooked up to it yet
<Chipaca> ie without handling events
<niemeyer> Chipaca: Which events?  There can't possibly be any events to consume at this stage, right?
<Chipaca> niemeyer: if you do: harness.add_relation(...), and the charm was already plugged in, it'll get the relation_created (say)
<Chipaca> niemeyer: or add_relation_unit() would trigger relation_joined
<Chipaca> etc
<niemeyer> Chipaca: That's intended.. to disable that behavior you'd call disable_events()
<niemeyer> Which perhaps should really be called disable_hooks, actually, since we don't want to disable internal events I think
<niemeyer> but that's an aside
<Chipaca> niemeyer: right, having initialize() means that the thing starts with events disabled, and then you hook things up and they're enabled, in a natural way
<Chipaca> niemeyer: at that point we could probably not have enable/disable at all, unless we found some advanced scenarios where they made sense
<niemeyer> Chipaca: It sounds to me like you're trying to retrofit the design into what we have in the PR
<niemeyer> Chipaca: The PR includes initialize, disable_events, and enable_events
<Chipaca> heh
<Chipaca> maybe
<niemeyer> Chipaca: And __init__, which is a prefix of "initialize" :)
<niemeyer> Chipaca: So the rationale you just provided is at least not the reason why it ended up like this
<Chipaca> to be clear, I _don't_ like passing the charm into enable
<niemeyer> Chipaca: And when you call update_config, will we run hooks or not?
<Chipaca> niemeyer: when?
<Chipaca> :)
<Chipaca> seriously: in which scenario (with initialize? without?), and at what point (after enable/disable/initialize/frob/...?)
<niemeyer> You pick it.. do you want to fire hooks every time after your preferred point of initialization?
<niemeyer> Can you foresee reasons not to?
<niemeyer> How do you handle it?
<Chipaca> hah, 1 sec
<Chipaca> niemeyer: jam already did https://github.com/jameinel/operator/pull/3/files fwiw
<niemeyer> It would be nice to have jam with us at some point.. is he working today?
<Chipaca> he is, or at least, he just merged master
<niemeyer> Why are we both having this conversation without us?  Is this channel a group chat or not?
<niemeyer> I don't care if we use IRC, or Telegram, or telnet.. but we need to have people communicating if we expect this thing to work at all
<Chipaca> I shall enquire
<jam> I am around, I just don't sit with IRC in my view
<niemeyer> jam, Chipaca: I don't care about which chat you guys use, but you need notifications active on the channel where you team works
<Chipaca> I should've pinged jam earlier in this, is all
<jam> I think I was probably making coffee when you pinged earlier, my apologies.
<niemeyer> Chipaca: No, not really.. this is the team channel.. it won't work if you need to ping people to pay attention whenever there's a conversation
<jam> niemeyer: Chipaca: so as I mentioned in my PR response, decoupling the charm being instantiated from the Harness felt like a nicer separation of concerns. Especially for code that wanted to test changes to the model without events, it felt much more natural to not have the charm created yet
<jam> (especially, for test_model.py, but I realize that is a bit of a special case of testing the framework itself)
<jam> *some* sort of helper function is currently necessary because of the need to have unique Class definitions, which I will be looking at next.
<niemeyer> Isn't the charm the entire thing the harness is testing, with extreme coupling so that you can manipulate its fake backend model to arbitrary degrees?
<niemeyer> Isn't every emit_foo() method going to be injecting events into that charm?
<niemeyer> Every other updating method effectively going to be invoking things into it?
<jam> events are definitely coupled to the charm instance, as that is the only way to generate hook events.
<niemeyer> What is the harness doing that it *not* coupled to the charm?
<jam> Though the fact that we've stated "we may not want to have events emitted yet" felt like a case of setting up the Model/Framework without a Charm, and then instantiating the charm once the setup was done felt quite natural
<jam> All of the state-of-the-model is underneath charm. Such that the charm can go look at it, but doesn't require a charm to exist for the model to be updated.
<Chipaca> to me, the question goes back to how we expect this to be used more
<niemeyer> That's fine.. it's what Chipaca was suggesting above. We can postpone the creation of the charm and have an explicit method for that. It doesn't change everything else we discussed, though:
<niemeyer> - We shouldn't make people play puzzle on every test to assemble the harness
<niemeyer> - There's no reason for disable_event/enable_event to take a charm and kill the internal charm instance
<niemeyer> - We may need to stop emitting hooks and make that manual so that people can try to trigger particular cases
<niemeyer> - The default case is supposed to be straightforward
<niemeyer> Having something like a harness.start() method would address Chipaca's suggestion.. but we still need to address everything else
<Chipaca> niemeyer: what's the puzzle you refer to in the first point?
<niemeyer> Chipaca: "... to assemble the harness"?
<jam> so the current proposed syntax if we rename 'initialize' to 'start' and change enable/disable to use a boolean would be:
<jam> https://paste.ubuntu.com/p/s4QcKGX3Xr/
<jam> niemeyer: Chipaca ^^
<Chipaca> sorry, having to check on the boys' home work
<Chipaca> back now
<jam> we have to have the charm meta to initialize a Model
<niemeyer> Let me quickly draft something
 * Chipaca waitws
 * Chipaca waits, also
<jam> if you have a different syntax that feels more natural, having examples are good. I've had these in tests, etc, but I'm happy to pull them out explicitly and discuss them.
<niemeyer> https://paste.ubuntu.com/p/XfjJ95NfB4/
<Chipaca> niemeyer: could make the yaml an optional arg on the constructor
<Chipaca> instead of having it as a method (because then you have to think about what happens if you call it after you've started or whatever)
<niemeyer> Chipaca: You could, but this is usually long string
<niemeyer> Chipaca: harness = Harness(Charm) feels solid
<niemeyer> Chipaca: You don't have to think if you don't allow it to take place
<jam> niemeyer: so one thing that I was using this for testing is testing components that aren't themselves a charm. And we don't have a metadata.yaml for CharmBase
<niemeyer> jam: I'm a bit lost.. CharmBase is the base class for charms.. charms have metadata.. ?
<jam> niemeyer: if I'm testing MysqlClient, it doesn't have its own charm. it will be used by several charms, and does consume events.
<jam> we need 'name' from metadata.yaml
<Chipaca> i need to step out but will catch up when i return
<niemeyer> jam: How does that change things in the context above?
<jam> niemeyer: so if you just pass CharmBase, there isn't a metadata.yaml, even though it does have defined events
<niemeyer> jam: We can just have a stock metadata.yaml, but that won't have any endpoints that will be useful for MysqlClient..
<niemeyer> jam: The only reasonable answer I can tihnk of is you actually define a trivial test charm and metadata to test it.. but I'm not sure if you're thinking of specific issues?
<jam> so I've been going with explicit metadata for each instantiation, which does have the nice property that you declare your needs right before you start testing them.
<jam> but I can understand when testing a real charm, that ends up copying the metadata you've already had to declare
<jam> niemeyer: does an optional __init__ parameter seem ok? I'm not sure about changing metadata late with set_meta()
<niemeyer> jam: Both of these cases seem covered in the example paste I provided above.. so I'm still not entirely sure if you're trying to point something that I'm completely oblivious to
<niemeyer> jam: What do you mean "late"?  Isn't the entire point of the start() method to enable a setup phase?
<jam> niemeyer: so start() for *me* was to delay when the Charm was instantiated, but to still have a Model that you can populate. If you can't do anything before start() then there is no reason to decouple it from __init__
<niemeyer> jam: Exactly.. that's what I'm saying as well
<jam> Model can't be instantiated before Meta is finalized, as it needs it for things like "what relations will I know about". So you can't have 'set_meta' to set it without replacing the Model.
<jam> niemeyer: so with the code that I proposed, you can perfectly populate Model with relation data, config data, etc, all without a Charm.
<niemeyer> I see
<jam> and then when you *do* create a Charm, it has access to a populated Model.
<niemeyer> jam: Do you populate the model, or do you populate the backend?
<jam> niemeyer: the backend.
<niemeyer> jam: The backend needs meta?
<jam> niemeyer: _ModelBackend doesn't need meta, but you pass in the backend to Model in its __init__ and Model does need both Backend and Meta.
<niemeyer> jam: If you populate the backend, and it doesn't need meta, can we create the model at start time?
<jam> niemeyer: and Framework which is a base for everything also needs Model, but that, again, could be constructed late.
<jam> niemeyer: so yes, we could fix Meta, create Model and Framework and Charm during start
<jam> and then have set_meta() refuse if Model has been created.
<niemeyer> start() is a bad name as we have a hook with that name.. begin(), bake(), setup()
<niemeyer> setup is also a bad name as that's what we've been doing before we call the method
<niemeyer> .begin() is likely the better choice of those, as it has the feeling of "alright, we're ready and will start doing things now"
<jam> initialize(Charm), instantiate(Charm), create(Charm), begin(Charm)
<jam> charm = harness.begin(Charm)
<jam> doesn't quite read naturally to me, but I could live with it. I do like it meaning the 'start of testing that I'm going to be doing'
<niemeyer> harness = Harness(Charm)
<niemeyer> harness.begin()
<niemeyer> "harness = Harness(Charm)" is what we had agreed on the sprint, and still feels right to me
<jam> ok
<niemeyer> We can cover the reasons again once more, but let's do that in the call
<niemeyer> I'm going to step out for lunch now
<jam> np
 * Chipaca running late
<jam> Chipaca: niemeyer: patch updated. I went with meta as an __init__ parameter instead of set_meta because we need it for a default unit_name. There could be other ways around that (having the user supply an optional unit_name might be better).
<jam> I did implement the 'look for metadata.yaml if not supplied', and added a test for it.
<jam> It is a test that is 'impure' vs the rest of the tests. (Everything else is just python objects, but that involves manipulating the filesystem, and importlib stuff.)
<jam> Anyway, EOD for me. Have a good afternoon/evening.
<niemeyer> Thanks, have a good one
 * Chipaca soft-EODs (need to pop back later to wrap up some things, but dinner now)
#smooth-operator 2020-03-25
<Chipaca> good morning all!
<Chipaca> why is my TODO box just growing :(
<jam> Chipaca, clearly because you aren't doing anything :)
<Chipaca> :'(
<jam> Chipaca, thoughts on 'read_relation_data' vs 'get_relation_data' ? I was avoiding having it too close to 'relation_get' but maybe putting it at the front is different enough.
<Chipaca> jam: i think get is better because read has me think it's more expensive than just getting something from memory
<jam> Chipaca, np. I'll push up that change. Will you have a chance to look at the updates since you looked last?
<Chipaca> jam: yep!
<Chipaca> jam: what sample charms do we have?
<jam> Chipaca, charms using the operator framework?
<Chipaca> yep
<Chipaca> ones that we point people at when they want to start using it
<Chipaca> even better, the ones we ourselves worked on
<jam> https://github.com/johnsca/charm-gitlab-k8s/
<jam> https://github.com/tvansteenburgh/charm-kine/blob/master/lib/charm.py was a fairly early version of the framework but still mostly sound
<jam> https://github.com/relaxdiego/charm-k8s-prometheus/ is using it, but probably isn't what I would point people to
<jam> I haven't reviewed https://github.com/gnuoy/ceph-iscsi/ which is also using the operator framework
<Chipaca> thank you
<jam> dmitrii was working on https://github.com/dshcherb/charm-nats-1 and https://github.com/dshcherb/charm-cockroachdb I believe
<Chipaca> i wouldn't call relaxdiego's work something i'd point people at
<Chipaca> (quite the opposite in fact)
<Chipaca> so agreed on that :)
<jam> indeed. I was working on interface-pgsql but was struggling to test it to know that it really was doing what I wanted. Hopefully this eventually gets back around to it.
<Chipaca> jam: tests unhappy
<Chipaca> jam: (need to de-path some things for 3.5)
<jam> yeah, I was worried about that.
<jam> Chipaca, pushed a fix that passes on 3.5
<Chipaca> jam: ð
<jam> so shall I push the button or would you like to?
<Chipaca> jam: sorry, i meant good on fixing it
<Chipaca> jam: am now doing a full pass
<jam> ah, gotcha
<jam> will wait for you
<jam> (working on follow up
<Chipaca> jam: i'll push the button when i'm done (want to go over the pr prescription to make sure it's current?)
<Chipaca> jam: bad news :-(
<Chipaca> jam: lots of silly things, mostly in docstrings
<Chipaca> jam: good news, if those are the things i'm asking for changes on, we're so almost there
<Chipaca> jam: note not all comments are requests-for-changes though, some are just me rambling / musing
<Chipaca> oh dang nearly half eleven and i haven't made the boys do any schoolwork
<Chipaca> jam: also, not mentioned in the comments there, is that enable/disable should raise a stink if they're called before begin()
<jam> Chipaca, should they? it would let you default to disabled events as soon as the charm is instantiated.
<Chipaca> jam: ah, if that works, a'ight :)
<Chipaca> of course it'd work :)
<Chipaca> ok, fair
<jam> Chipaca, it isn't that I need it to, just that it feels odd to go out of my way to prevent it, when it isn't really wrong per se
<Chipaca> jam: yep, got that
<Chipaca> jam: belay my "should raise a stink" comment :-p
<jam> Chipaca, so *I* would like to have harness.begin() return the charm instance, though for now I've gone with you have to access it as an attribute. If you look at the various tests, they're ready to interact with 'charm' after begin()
<jam> thoughts??
<Chipaca> jam: i see no harm in returning it as well as keeping a copy
<Chipaca> (as long as it's clearly documented that that's what we do, which shouldn't be hard)
<jam> so there isn't a huge difference, it is https://paste.ubuntu.com/p/8tSdcb74s3/ vs https://paste.ubuntu.com/p/Cj4k8n94Gw/
<jam> anyway, I'll leave that for a future follow up. It did feel like Gustavo had a strong preference that you would access it as an attribute.
<Chipaca> jam: ok
<Chipaca> it's about what feels right and consistent in the api
<Chipaca> having the two-argument builder thing was strange, and there i'd agree for having it as an attribute without a doubt
<Chipaca> having it split out as a separate step that returns it feels much better than the two-arg thing
<Chipaca> the attribute thing is still alright
<Chipaca> Â¯\_(ã)_/Â¯
<Chipaca> so, yes, let's leave it for a future iteration
<jam> so for 'charm_dir'.... It is weird to set charm_dir if you are using CharmBase, but it does make sense to set it if you are using a real charm.
<jam> Chipaca, I definitely think that if you have a valid charm_dir, you'd want to use the charm_dir / 'metadata.yaml'
<jam> Chipaca, one thing I really liked about Harness, was that it was in-memory only, and this change breaks that.
<Chipaca> :) well, _can_ break it
<Chipaca> it can still be all in memory, and that's nice
<jam> (if you look at the test suite, I pulled in importlib, pathlib, shutil, sys, and tempfile in order to test the 'metadata.yaml' handling.)
<Chipaca> and it's nice that it falls back to disk, and not touching it if you know what you're doing
<Chipaca> so we can still write tests that are all in memory and super fast because of it, for any and all things we write, while supporting simplicity and DRY for actual charm writers
<jam> Chipaca, it is also a place where we assume the disk layout of charms (that your Charm class is exactly in a subdirectory of the top charm dir)
<Chipaca> jam: yeah, i'd like to change that at some point, but can be a followup
<jam> it is the advised layout, and you can override it. so maybe that's fine. I wonder about taking the charm_dir itself
<Chipaca> it's fine for now and will mostly dtrt
<Chipaca> jam: it _could_ be done the other way around: find the yaml, set charm dir to the dir that holds it
<Chipaca> that is, walk up from the .py looking for it
<Chipaca> but as i say, later
<Chipaca> jam: i do expect charm_dir to be only useful to people actually having a metadata.yaml so there is that (and, we can add it as a keyword argument to override like with yaml)
<jam> Chipaca, so I went ahead and set charm_dir if we read metadata.yaml, I'll push that up.
<facubatista> Muy buenos dÃ­as a todos!
<jam> facubatista, good morning and welcome back.
<facubatista> hola jam :)
<jam> Chipaca, I think I've addressed all your review comments. Thanks for the thorough review. I appreciate it.
<Chipaca> Facu: buen dÃ­a caeza!
<Chipaca> facundo__: ETOOMANYFACUNDOS
<Chipaca> on the _other_ hand, i do still have an open positon
<Chipaca> hmmmmmm
<facundo__> Chipaca, internet's not best day
<Chipaca> facundo__: buen dÃ­a caeza :)
<facundo__> Chipaca, hola :)
<Chipaca> jam: care to update the description and land it?
<Chipaca> facubatista: last chance to review / ask for changes in the testing pr :-D
<facubatista> Chipaca, land it, too big too old to keep pointing to details, we can adjust details in the future
<Chipaca> facubatista: <3
<jam> Chipaca, +1
<jam> Chipaca, landed.
<jam> thanks for your help
<facubatista> jam, niemeyer, question about "config-changed" event: do juju guarantee that all events are delivered in order and nothing is skipped? like, is there 0.000% chance that events won't arrive out of order, or any could be missed?
<facubatista> is really a charm able to react to config "deltas" (as it's being talked in #189) or it always should manage current state?
<jam> facubatista, juju intentionally doesn't include data in events, so if the config changes 2x you might get a single 'config-changed' event. But you are guaranteed to get an event as long as the config is different than the last time we told you about it.
<jam> it is generally an 'at-least-once' model with coalescing.
<facubatista> jam, that is a promise we can rely on?
<jam> put another way, once your config-changed exits, we record the version of config that we told you about. we then watch to see if there is a new version, and will eventually run config-changed until those numbers match.
<jam> we *might* run relation-changed, or some other hook in the mean time.
<facubatista> jam, I'm trying to understand if this is how "it works in general" or "a solid block we can rely on to build other behaviours"
<jam> facubatista, so for all data that we want to tell the charm about. the associated hook returning successfully is what we use to update the version that the charm has seen.
<jam> so we store "we have shown Charm config version 5", and we decide to fire config-changed if config version != 5. So we may not show you version 6, if you're already at version 8 by the time the charm can run config-changed.
<jam> but we don't update the version the charm has seen until config-changed returns with success.
<facubatista> jam, where this version number comes from?
<facubatista> jam, also, how the charm actually "sees" the config?
<facubatista> (sorry if these questions are too basic, I'm still diving in)
<jam> facubatista, every update to the config (from a user doing 'juju config foo=bar') causes the Version to increment by one.
<jam> the actual version counter is not exposed to the user or charm anywhere.
<jam> the charm 'sees' the config by executing the 'config-get' hook tool that is in $PATH during a hook execution.
<facubatista> ah, the charm doesn't see the version! that is important
<jam> (_ModelBackend is where we interface with Juju's hook tools)
<facubatista> jam, config-get is synchronous? the charm process is blocked and keeps running until it gets back the whole config?
<jam> facubatista, so the other aspect is that Juju fixes the content that config-get will return during the lifetime of any hook.
<jam> (once it returns a value, that value is cached for the rest of the hook execution.)
<jam> config-get is synchronous, but that matters less because we want to make sure that a charm calling it at the beginning of a hook, or at the end of a hook sees the same content.
<jam> and *that* is the version that we record the charm as seeing.
<facubatista> jam, we record that the charm saw version N because it answered ok to config-changed, or because we got a config-get from it?
<jam> facubatista, because it answered ok to config-changed.
<facubatista> jam, thanks for all the info, I have a better picture of this now
<jam> facubatista, np
<facubatista> jam, do you have an example at hand of how a complex "config" would look? the response to a config-get, I mean...
<jam> facubatista, config-get returns a YAML of key-value as defined by the charm's config.yaml schema. which can have quite a bit in it.
<jam> eg: https://api.jujucharms.com/charmstore/v5/postgresql-203/archive/config.yaml
<jam> has about 60 config options.
<jam> now, they can't be nested, so it is all top-level keys, with values that might be a string/int/float
<facubatista> jam, so it could be super big, but not complex as it's not nested and with restricted data types
<facubatista> thanks
<jam> yep
 * facubatista -> lunch
 * facubatista is back
<Chipaca> facubatista: welcome back :-p
<facubatista> :)
<facubatista> niemeyer, Chipaca, I went back and forward regarding the best way to structure this, I like it how it's now, but not 100% happy: https://docs.google.com/document/d/1zKwzo26bNVFgohaRB_kdUCsFPY3OcjfcpQklGpkOHpc/
<facubatista> in any case, everything I thought of is there, so we can tune it
<Chipaca> soft EOD from me. TTFN!
<niemeyer> facubatista: Want to have a quick call on it?
<facubatista> niemeyer, still around?
 * facubatista was doing a little gym
<niemeyer> facubatista: Yeah, just responding to something else, back soon
<facubatista> niemeyer, I'm here, ping me and we can jump to https://meet.google.com/veq-yfqm-kdk
<niemeyer> Alright, done withat
<niemeyer> with that
<niemeyer> Joining..
<niemeyer> facubatista: ping
<facubatista> niemeyer, going
 * facubatista brb
 * facubatista is back
<facubatista> Chipaca, I'm heavily changing the Debugger spec, I will let you know when it's done
<facubatista> but will need to be tomorrow
 * facubatista eods
#smooth-operator 2020-03-26
<jam> morning all
<Chipaca> 1 2 3 tasting
<Chipaca> jam, niemeyer, do you read me?
<niemeyer> Moin!
<jam> morning Chipaca
<Chipaca> Woo, it works! Good morning
<jam> back in a bit, working on math homework :)
<Chipaca> I was running low on things, went to one supermarket yesterday after dinner and there was a bunch of stuff I couldn't get. So I had an early breakfast and came to a different supermarket only to find it was only open for OAPs until 9... Hence irccloud
<Chipaca> On the upside I got everything but tea, now
<Chipaca> facubatista: thank you for letting me know. "checkpoint" is just a note to myself, "this is how far I got"
<Chipaca> glad you were able to get deeper into it with gustavo y'day
<Chipaca> hmm, how can we avoid pydoc showing the license as the module's description?
<jam> If there are 3 spheres of radius 1, inside a cylinder of height 2 with radius just enough to hold the spheres, what is the fraction of volume of the cylinder occupied by the spheres.
<jam> been a long time since I did that kind of math.
<jam> was kind of fun
<Chipaca> facubatista, jam, niemeyer, WDYT? https://github.com/chipaca/operator/blob/docs-take-2/README.md
<facubatista> Muy buenos dÃ­as a todos!
<niemeyer> Hey hey
<Chipaca> facubatista: ï½ï½ï½ï½ï½ï½ï½ï½ï½ï½ï½ï½ï½ï½  ï½ï½ï½ï½ï½ï½ï½ï¼
<facubatista> alÃ³ alÃ³
<facubatista> jam, volume of the cylinder minus 2 * volume of the sphere
<Chipaca> facubatista, jam, niemeyer, to be clear i'm asking if you like the README, the TBD links are still TBD :)
<facubatista> (as you had two spheres)
<facubatista> Chipaca, will read
<niemeyer> Chipaca: I like how it starts, but there's something missing before "testing your charms"
<Chipaca> niemeyer: "breaking your charms"
 * Chipaca hides
<niemeyer> Chipaca: The part of the current document that explains the overall strategy for writing the charm seems important
<Chipaca> niemeyer: I can put the minimal requirements for writing a charm there, then
<Chipaca> the current "recommendation" for the layout is not nice, and I'd like us to stop recommending it, or at least to not make it so front-and-centre
<niemeyer> Chipaca: Yeah, it seems important to provide some kind of "north" to people that open an editor and need to do something
<Chipaca> sounds good
<Chipaca> I had that, but then it got too long winded and so i nuked it. I should've instead pared it down... :)
<Chipaca> also, note i mention rpdb but that's just because i wanted to see how it tasted, having a "require this, and optionally this other thing to get this extra bit"
<Chipaca> and it seems to be fine
<facubatista> Chipaca, even before; I think it's valuable to mention at least once that this is about charms in the intro before getting started
<Chipaca> I'll try :)
<facubatista> I don't want people reading "The Operator Framework provides a simple, lightweight, and powerful way of encapsulating operational experience in code" and thinking "nice, k8s"
<Chipaca> what is k3s, btw?
<facubatista> "nice, ansible"
<facubatista> etc
<Chipaca> so I should mention juju and charms in that same sentence, then
<Chipaca> I think we still had a tiny bit of space before the mobile rendering got pushed off screen, so it should work
<facubatista> "The Operator Framework provides a simple, lightweight, and powerful way of building charms, the best way to encapsulate operational experience in code."
<Chipaca> facubatista: *juju* charms?
<facubatista> Chipaca, or maybe the charm word itself is a link to somewhere which includes what is a charm
<Chipaca> googling "what is a juju charm" is not a nice look
<facubatista> Chipaca, shall we provide a PR on that readme? how we can add comments?
<Chipaca> facubatista: I'll push a PR shortly
<Chipaca> i linked to the WIP one to see if I was completely MFT
<niemeyer> k3s is a self-contained k8s..  Think microk8s, but a different implementation of it
<Chipaca> looks like somebody took a planer to the 8
<facubatista> Chipaca, "MFT"?
<Chipaca> er
<Chipaca> facubatista: urinating outside of the appropriate receptacle
<Chipaca> probably a grulic-ism
<facubatista> is understable also in the rest of .ar, I guess
<Chipaca> :)
<jam> Chipaca, k3s is a minimialist install/installer of kubernetes. https://k3s.io/
<jam> Missing the F'n Target
<facubatista> Chipaca, use capital P in Python unless you're really meaning the command; we'll not use rpdb; "dive into the tutorial" I thought it was telling me to go off and read more about juju and charms, but it looks it's an operator itself tutorial?;
<Chipaca> ok wrt python
<facubatista> Chipaca, "pydoc3 ops.testing has the details for that" you're telling me to clone the project and run a command just to see more about testing harness? gross
<Chipaca> tutorial is our tutorial
<Chipaca> facubatista: until we have something better, that is the best i can do
<Chipaca> i mean, yes, gross, but what else should i do?
<Chipaca> extra gross because pydoc's description will be the copyright (eww)
<facubatista> ack
<Chipaca> jam: heh, i like that backronim :)
<Chipaca> backronym
<jam> seems they are conducting "National Sanitization" over the weekend, and will be restricting grocery store access, etc. So I need to do grocery shopping before the weekend, and chances are good they will get overwhelmed if I'm late.
<jam> My wife just found out from her work, so we're heading out. I'll try to be back for standup.
<Chipaca> jam: good luck!
<Chipaca> facubatista: https://github.com/canonical/operator/pull/190 FWIW
<Chipaca> I'm going for a run, and then a quick lunch
<facubatista> everybody: this is refreshed now https://docs.google.com/document/d/1zKwzo26bNVFgohaRB_kdUCsFPY3OcjfcpQklGpkOHpc/
<Chipaca> facubatista: ack
<facubatista> niemeyer, Chipaca, this is refreshed with all we talked about: https://docs.google.com/document/d/1zKwzo26bNVFgohaRB_kdUCsFPY3OcjfcpQklGpkOHpc/
<facubatista> note that having one env var is nice and simple, it complicates a little the case for multiple hooks, there is one comment (the only one I left) for this
 * facubatista -> lunch
 * facubatista -> lunch, for real now
 * Chipaca intercepted facu before his lunch
<Chipaca> mea culpa, mea culpa, etc
 * facubatista is back
 * Chipaca celebrates
<Chipaca> I'm going to go ahead and EOD
<Chipaca> y'all be good
 * facubatista eods
#smooth-operator 2020-03-27
<Chipaca> good morning all!
<mthaddon> o/
<Chipaca> mthaddon: how's things?
<mthaddon> staying healthy so far thanks - and you?
<Chipaca> mthaddon: healthy *and* sane!
 * Chipaca is winning
<mthaddon> :)
<Chipaca> mthaddon: I keep on going back to your 'operator framework documentation' document, and i'm thankful for it every time
<mthaddon> ah cool, yeah, I saw you'd started work on rewriting the README. Glad that's proving useful
<Chipaca> t0mb0: ð
<facubatista> Muy buenos dÃ­as a todos!
<Chipaca> facubatista: ï½ï½ï½ï½ï½  ï½ï½ï½ï½ï½ï½ï½ï¼
<facubatista> hola Chipaca
 * Chipaca takes a braek
<facubatista> the Framework.observe method gets whatever we pass it as a callback and checks isinstance(observer, types.MethodType)
<facubatista> why it has to be a method?
<facubatista> it exploded to me, I was doing a quick         self.framework.observe(self.on.upgrade_charm, lambda e: logger.info("====== upgrade ok"))
<Chipaca> I don't know why, when I saw that I thought "that should be something like `callable`"
<facubatista> let's see if somebody has a reason for that, otherwise I'll fix it
<Chipaca> facubatista: maybe better filed as a bug, people seem off today
<niemeyer> facubatista: Events are persisted
<niemeyer> facubatista: We don't want to store code in the database
<niemeyer> facubatista: Hopefully the error method was indicative of what it needed to be, or was it not?
<niemeyer> s/error method/error string/
<Chipaca> niemeyer: I don't understand how one thing follows from the other
<facubatista> RuntimeError: Observer method not provided explicitly and function type has no "on_upgrade_charm" method
<facubatista> I understand now "Observer" as I read the framework code
<niemeyer> facubatista: That's not entirely clear
<facubatista> niemeyer, right
<niemeyer> facubatista: Might be improved indeed
<niemeyer> Chipaca: Which things thing follows what?
<facubatista> Chipaca, we store the *method name*, not the method itself
<Chipaca> niemeyer: events are persisted, we don't want to store code in the database, so the handler needs to be a method
<facubatista> Chipaca, later we do a getattr, I guess
<Chipaca> we store the method name? why?
<niemeyer> Chipaca: Deferring
<Chipaca> but we stored the event, and the charm is initialized so all the handlers are hooked up
<niemeyer> Chipaca: we don't call the same observer twice unless it asks to be called again
<Chipaca> why would we store the method name?
<niemeyer> Chipaca: I just responded that one.. :)
<Chipaca> no you didn't :)
<niemeyer> Such a productive conversation
<Chipaca> yes
<Chipaca> you are being obtuse
<Chipaca> please stop
<niemeyer> Chipaca: I've been trying to explain things, and meanwhile you've been saying the equivalent of "I don't understand it".. unless you ask intelligent questions, I can't help you
<facubatista> niemeyer, if I get an event foo, and I defer it, why can't we store "foo", then in the next run, we deliver "foo" to whatever is hooked at that time?
<niemeyer> facubatista: Because of that issue above: we don't call the same observer twice unless it asks to be called again
<niemeyer> facubatista: For that to happen, we need an identity for that "it asks"
<niemeyer> facubatista: How would you do that for a lambda?
<Chipaca> ok, now I understood, thank you
<facubatista> but if I deferred it, I'm not asking to deliver me the event in the future? the problem is if I change the method or callable (charm upgrade) in the middle?
<Chipaca> facubatista: but you might have multiple handlers for the event
<Chipaca> facubatista: and only some of them deferred
<facubatista> ah, right
<facubatista> niemeyer, Chipaca, thanks!
<facubatista> this is the kind of details we need to be sure is in the docs :)
<Chipaca> facubatista: and in the errors :-D
<niemeyer> facubatista: To be honest, I've been always double-hearted about the whole deferring thing
<niemeyer> It might not pull its weight
<niemeyer> It's also worth noting in this context that we err on the safe side..
<niemeyer> Despite the apparent way it works, internally handlers don't really ask for deferring an event
<niemeyer> They do the opposite: the ack the event as handled
<niemeyer> The act of deferring prevents that acknowledgement
<facubatista> niemeyer, deferring event across runs complicates a lot of little things (not only the code, also the developer thinking of the model) -- are there cases where deferring the event is the only sane option?
<facubatista> (with "model" I didn't mean the juju model, but the "how my charm works and handle all system complexities")
<niemeyer> This is a long way to say that deferring doesn't in fact trigger a complex path into the code.. we _always_ persist events, and then trigger a common path that handles pending events
<niemeyer> That's what makes me feel a bit less bad about the complexity..
<niemeyer> Our logic makes it harder for events to be lost, even if you never call defer() at all
<niemeyer> facubatista: I would simply not explain any of that complexity until really required.. if people have to understand the details of that stuff, we failed
<facubatista> niemeyer, mmm... if we don't persist it, and we crash while handling it... the Agent would not re-send it as we never acked it?
<niemeyer> facubatista: I'm a bit confused about the context for this question.. we always persist it, right?
<facubatista> (don't really know how the Agent works in this regard)
<niemeyer> facubatista: Yeah, that's what I was trying to explain above: we *always* persist it
<facubatista> niemeyer, yes, we always persist it; I'm challenging that decision :)
<niemeyer> facubatista: This is not triggered by deferring
<niemeyer> facubatista: The difference in deferring is actually very small in terms of overall behavior
<niemeyer> facubatista: You simply flip a switch saying "do I remove this event now that I handled it?"
<niemeyer> facubatista: Everything else is exactly the same
<facubatista> niemeyer, yes, because we always persist it
<niemeyer> Yeah
<facubatista> niemeyer, bear with me a little in this other POV
<niemeyer> facubatista: Which means that on crashes the event will be sent again
<facubatista> niemeyer, what if we NOT always persist it? what if we persist it only when deferring? we may crash while handling it, yes, but shouldn't the Agent re-send the event?
<niemeyer> facubatista: Not necessarily, and our events don't come only from the agent
<facubatista> ah, right, it may be a pure internal event
<niemeyer> facubatista: This entire logic works any time something calls emit()
<facubatista> niemeyer, great, I see now all the picture, thanks!
 * facubatista adds a note to improve that error message
<niemeyer> facubatista: np, and it sounds healthy to continue challenging such options
<Chipaca> niemeyer: question: in what situations do we expect snapshot/restore to be used?
<niemeyer> Chipaca: Hopefully not much.. it's the underlying mechanism that enables StoredState, which is a much nicer interface for normal development
<Chipaca> niemeyer: so https://github.com/johnsca/interface-mysql/blob/master/interface_mysql.py is not kosher then?
<Chipaca> maybe it predates storedstate?
<niemeyer> Chipaca: EventBase is not an Object at the moment.. I had an exchange here with facubatista recently covering that.. he was going to do some research to see how hard it would make it a more usual object
<niemeyer> I think that's the only odd case we have
<niemeyer> I won't be around for the standup today, but please drop me a note here if you need something
<facubatista> niemeyer, comments on this would be great, thanks! https://docs.google.com/document/d/1zKwzo26bNVFgohaRB_kdUCsFPY3OcjfcpQklGpkOHpc/edit#
 * facubatista brb
 * facubatista is back
 * Chipaca wishes all standups went like that
<axino> ah so this is where the cool kids hang
<axino> hang out
<axino> (is probably better English !)
<facubatista> hello axino
<Chipaca> mthaddon: https://github.com/canonical/operator/blob/06d242ff31b4a5cc6e7e829f9e8f99c6974def86/tbd_examples.md FWIW
<Chipaca> axino: welcome!
<axino> o/
<Chipaca> mthaddon: I'm just staging stuff in that PR while I finish getting our docs ducks docked
<mthaddon> Chipaca: nice! definitely like the format, makes sense
<Chipaca> this assumes i understood the questions tho :-D
<niemeyer> facubatista: Replied with a few comments
<niemeyer> facubatista: In general it looks nice
<niemeyer> Hello new people
<facubatista> niemeyer, ack
<facubatista> niemeyer, we are in the same page, then; there's one extra case annotated now in the spec, but I need to check with juju one detail about multiple parameters
<facubatista> and confirmed!
<facubatista> (multiple --breakpoint parameters is possible in juju)
<facubatista> there are current cases for this, e.g. --overlay in juju deploy
<niemeyer> I'd just accept --breakpoint=a,b,c as you suggested
<facubatista> niemeyer, in reality, if the user goes and write --breakpoint=a,b,c, juju would set up JUJU_BREAKPOINT to "a,b,c", and it will work just the same :)
<niemeyer> Right
<niemeyer> As long as you define that this is the behavior :)
<facubatista> the Spec is in good shape
<facubatista> yay, my first spec here :p
<niemeyer> ð ð ð ð ð
<Chipaca> facubatista: it's even better than that
<Chipaca> facubatista: it's _the_ first spec here :-D
<facubatista> jajaja
 * facubatista -> lunch
 * facubatista is back
<Chipaca> hmm
<Chipaca> facubatista: i just set the approved date on that spec, but then thought
<Chipaca> maybe we should ask our juju friends first :)
<facubatista> Chipaca, what about?
<facubatista> I mean, ask them?
<Chipaca> facubatista: whether they're ok with the changes to debug-hook
<Chipaca> can't imagine a scenario where they block it though
 * Chipaca might just be short of imagination
<facubatista> Chipaca, it's a step in the milestone, but it's a good idea indeed to gain consensus in the spec itself
<facubatista> Chipaca, who would be the persons to contact about this?
<Chipaca> facubatista: I'll see if rick_h has a moment
<Chipaca> facubatista: ok, no answer on that front, so I'd say let's leave it with jam
<facubatista> YES YES YES
 * facubatista dances the success dance
 * facubatista eods and eows
<facubatista> see you all on Monday
<Chipaca> aw man i missed the success dance
<niemeyer> Never too late for dancing..
