[08:17] <Chipaca> 👋
[08:26] <bthomas> Morning Chipaca
[08:27] <bthomas> Chipaca: If a hook changes Config will the change persist in the next hook that is called ? Of course I know one can use StoredState for persistence.
[08:28] <Chipaca> bthomas: can a hook change config?
[08:30] <bthomas> Chipaca: Have not tried or looked into code. I was under the impression it is an associative array. Guessing by your question, it can not.
[08:30] <Chipaca> bthomas: we have a test that would fail if you could
[08:30] <bthomas> Chipaca: Nice. Thanks for clarifying. Back to StoredState :)
[09:50] <bthomas> Chipaca: In https://github.com/canonical/prometheus-operator/pull/3/files Justin added a function _get_image_tag which required mocking in every test. jam felt we may have a problem with "layering" here. I do not see any other alternative than the once I have suggested in the review comment. IMHO the right "layer" for such functionality is lower down the stack, in Juju or Operator Framework. Thoughts ?
[09:55] <bthomas> Refering to this comment : https://github.com/canonical/prometheus-operator/pull/3/files#r497074340
[10:58] <facubatista> ¡Muy buenos días a todos!
[10:58] <Chipaca> facubatista: 👋❗
[10:58] <Chipaca> bthomas: looking
[10:59] <bthomas> नमस्ते facubatista
[10:59] <facubatista> hola Chipaca, bthomas :)
[11:01] <bthomas> Thanks Chipaca
[11:16] <bthomas> Chipaca: you made a suggetion to avoid mentioning class name, for example `ConfigChanged` in the doc string of `ConfigChangedEvent`. I will follow through and update all such instances. However I noticed that for these event classes, the one line summary docstring is usually written as "Represents the `config-canged` hook from Juju". Would it reasonable to alter such lines as "An event triggered on configuration change" then lo
[11:16] <bthomas> in the same doc string, metion that this event "invokes the `config_changed` hook".
[11:59] <Chipaca> bthomas: in meetings; i'll get back to you when i can
[11:59] <bthomas> no problem
[12:25] <jam> bthomas, if we were using resources directly, juju would be providing the digest for you to be using. I'm not a huge fan of hitting the docker registry from the charm (it has all sorts of implications for offline deploys, etc)
[12:25] <jam> but I can accept it as a "way to get channel like behavior for now"
[12:30] <bthomas> jam Thanks for the insight. I appreciate our current constraints. I have responded to Justin's question regarding "layering" by suggesting doing the patching at setUp and tearDown. This should obviate the impact on all tests, if it is acceptable.
[12:58] <jam> bthomas, so ideally it would be more something that we could inject into the charm via a parameter, rather than being patched (IMO). (intended to be configured, vs 'look here is a way to get what I want')
[12:58] <jam> and then we would have direct tests of the request based behavior, but split out from the other tests
[13:41] <bthomas> jam I do agree with you that using a configuration mechanism (a means of injecting) makes testing simpler. My quandary is that the present requirements do require replacing (partially) a config parameter with a dynamically (runtime) generated one.
[13:41] <jam> bthomas, so I mentioned on the bug.  I think we do want StoredState to prevent unrelated changes from causing us to upgrade a patch version of the application
[13:42] <jam> we should confirm with jldev regarding how we get "smooth transitions of application versions" when we have channels
[13:42] <jam> and try to mimic that behavior
[13:42] <jam> but my understanding is that it isn't "pods are updated automatically when things are pushed to the rocks store"
[13:43] <bthomas> got it. Thanks.
[14:24]  * bthomas nudges Chipaca regarding rewording the one line descriptions of event classes ^
[14:25] <bthomas> :)
[14:30] <aluria> hi o/ - I had a look to the grafana-operator and prometheus-operator code (to better understand the approach I should take to enable relations between charms) and I have a couple of questions
[14:31] <Chipaca> aluria: 👋
[14:31] <Chipaca> shoot
[14:32] <aluria> 1) minor doubt: I think the grafana-source interface should be set in metadata.yaml the other way around. I think it doesn't mind from the point of view of sharing data but the prometheus-operator defines grafana-source in "requires" when it should be in "provides", as it sets host/IP and port for the grafana charm to configure the datasource endpoint
[14:33] <aluria> (same, grafana-operator should have the grafana-source interface as requires, because it will use "get" methods)
[14:35] <aluria> 2) the other Q is that I've seen some code using EventSource and .emit, but I don't quite understand it. I don't see a .observe counterpart and I'm not sure if the way that would work would be similar to the reactive framework use of states (set_flag and then is_flag_set or @when)
[14:35] <aluria> in the grafana and prometheus operator charms, I think they only take care of the -joined, -changed and -departed hooks being observe. EventSource is not used
[14:37] <aluria> 3) [sorry, bonus Q] the prometheus-operator uses StoredState - iiuc, that should be used in a similar way as unitdata is used in charmhelpers? (make use of previously retrieved data to avoid a call to relation-get, etc.) ?
[14:37] <aluria> Chipaca: ^ :)
[14:41] <Chipaca> aluria: just in case, could you link to the code you're talking about?
[14:46] <aluria> Chipaca: https://github.com/canonical/grafana-operator and https://github.com/canonical/prometheus-operator
[14:47] <Chipaca> hmm, justin isn't on irc
[14:47] <Chipaca> bthomas: ^ but you're right there :)
[14:50] <bthomas> aluria With regard to Q1, it may help to think of Grafana as a "Dashboard Service Provider" and Prometheus as a data source that "requires a dashboard service".
[14:52] <bthomas> With regard to Q2 at least as far as Prometheus was concerned there was no need to emit any event. It was suffecient to just handle events emitted by Juju.
[14:52] <aluria> bthomas: re Q2, an example of using .emit is here https://github.com/johnsca/interface-mysql/blob/master/interface_mysql.py#L68 and the "on_changed" method... it looks like when the config-changed hook runs, ".emit" will trigger a -relation-changed hook, right? Is that the use case?
[14:54] <aluria> re: Q1, ok if that works for you - I usually relate "provides" with the need of "relation-set" calls, while "requires" needs "relation-get" calls
[14:55] <bthomas> aluria: a quick looks shows that that charm is emitting an event database_lost because it had a record of a database connection which now seems to have gone.
[14:56] <bthomas> aluria: with regard to Q1 you may want to further discuss this with jam and jldev who are experts on the matter.
[14:56] <bthomas> With regard to Q3, all I can say is that StoredState is used to persist data across hook invocations. Use it if you have a use case for it.
[14:58] <Chipaca> aluria: in general a component (not a charm) would create custom events and emit() them when whatever situation they're modeling happens
[14:59] <Chipaca> aluria: users of that component would observe those events if they needed to react to them, but it's fine if they don't
[14:59] <Chipaca> same as your charm doesn't need to write handlers for all juju events
[14:59] <bthomas> Chipaca: what do you mean by "component" as opposed to charm.
[14:59] <Chipaca> aluria: a database interface for example
[15:00] <Chipaca> aluria: interfaces, in general
[15:00] <Chipaca> aluria: i have seen some charms creating custom events and emiting them instead of just calling a function, but it's a bad smell
[15:00] <Chipaca> ie it wouldn't pass review
[15:01] <aluria> that would be similar to interfaces in the reactive framework, when they use set_flag and clear_flag?
[15:01] <aluria> good to know re: emitting vs calling a func directly
[15:14] <justinclark> Chipaca and aluria I'm here for any Grafana-specific questions now. Sorry for not being on here sooner
[15:21] <Chipaca> justinclark: 1 sec
 1) minor doubt: I think the grafana-source interface should be set in metadata.yaml the other way around. I think it doesn't mind from the point of view of sharing data but the prometheus-operator defines grafana-source in "requires" when it should be in "provides", as it sets host/IP and port for the grafana charm to configure the datasource endpoint
 (same, grafana-operator should have the grafana-source interface as requires, because it will use "get" methods)
[15:21] <aluria> justinclark: hey, sorry was on a call - this is the backlog https://pastebin.ubuntu.com/p/jGpsppkjHV/
 2) the other Q is that I've seen some code using EventSource and .emit, but I don't quite understand it. I don't see a .observe counterpart and I'm not sure if the way that would work would be similar to the reactive framework use of states (set_flag and then is_flag_set or @when)
 in the grafana and prometheus operator charms, I think they only take care of the -joined, -changed and -departed hooks being observe. EventSource is not used
 3) [sorry, bonus Q] the prometheus-operator uses StoredState - iiuc, that should be used in a similar way as unitdata is used in charmhelpers? (make use of previously retrieved data to avoid a call to relation-get, etc.) ?
[15:22] <aluria> oh, that will also work :D
[15:22] <Chipaca> justinclark: that's all the Qs :)
[15:22] <Chipaca> heh, sorry
[15:24] <aluria> Chipaca: thx for the info re: interfaces and the use of .emit - I have to review that and find what is the suggested approach. I'll have a close look to the database interface (shared before), the prometheus interface (https://git.launchpad.net/interface-prometheus/tree/operator.py#n74) and see what it fits me in my current case
[15:27] <Chipaca> justinclark: wrt the 'v' prefix, you could use packaging instead of semver
[15:27] <Chipaca> ie from packaging.version import parse
[15:27] <Chipaca> parse('1.2.3') == parse('v1.2.3')
[15:29] <justinclark> Chipaca thanks - let me take a look. I *really* don't like the current handling of the 'v' prefix
[15:51]  * facubatista -> lunch
[15:53] <bthomas> Nice. I am liking package.version. If that works for you justinclark then there may be no need to retain both original tag and parsed tag. Kudos Chipaca
[16:01] <justinclark> bthomas it looks like Prometheus has inconsistent version strings sometimes so I think it would still be safer to retain the original tag. But this does get rid of the need to remove 'v' which is really nice.
[16:04] <bthomas> justinclark: exactly I did notice that. As mentioned in my review comment that is why I found your solution to keep both original tags and parsed tags an acceptable solution to deal with this evil situation :). But thanks to Chipaca looks like you have a better solution now.
[16:06] <justinclark> Well, even with packaging, it is not straightforward how we consistently reconstruct version strings like: "v2.21.0-rc.0" and "0.16.0rc1". Both of which are real examples of tags Prometheus has
[16:07] <justinclark> I'm just thinking we should be safe and keep the original string rather than assume we can recreate the original.
[16:07] <bthomas> sucks
[16:07]  * bthomas start barking loudly 
[16:09] <bthomas> justinclark: they seem to be more consistent on recent versions though.
[16:09] <bthomas> anyway you decide
[16:10] <justinclark> I saw that too. I think I'll keep the current version for now just in case the format changes in the future. Hopefully it will save future developers the headache.
[16:11] <bthomas> I am tempted to ask the question on the prometheus forum if the have a well defined format for the version tags ? ;-)
[16:24] <Chipaca> a shame packaging doesn't hang on to the original string, but ¯\_(ツ)_/¯
[16:26] <justinclark> Haha that would certainly give me confidence to reconstruct the version, bthomas.
[16:27] <bthomas> yep. Again perhaps we should highlight your use case and create an issue on their issue tracker.
[16:47]  * justinclark lunch
[17:47] <aluria> cory_fu: hi - would you have ~10min? I'm trying to understand what this does https://github.com/johnsca/charm-gitlab-k8s/blob/c2423f4ec2718a0b5acebe0242f88aa0a1706f39/src/charm.py#L38
[17:47] <aluria> I guess I don't understand the second arg (self), and if it calls the on_database_lost method
[17:48] <aluria> I did see the .emit call comes from https://github.com/johnsca/interface-mysql/blob/master/interface_mysql.py#L97
[18:38] <Chipaca> aluria: that no longer works, but when it did it was equivalent to self.framework.observe(self.mysql.on.database_lost, self.on_database_lost)
[19:59]  * facubatista eods, will be back on Friday