[08:19] Chipaca: yep, have added an approving review overnight (had deliberately held off til then to avoid confusion about whether it's ready to land before you guys have had a chance to review it) [08:19] mthaddon: 👍 [09:04] Chipaca: could you recommend some well made operator charm, so I can check that there is nothing wrong with my juju/microk8s setup ? [09:06] hmm [09:06] :) [09:07] looking for one that's public and doesn't need a whole lot of other stuff around it [09:08] 👍 [09:08] bthomas: https://code.launchpad.net/~mattermost-charmers/charm-k8s-mattermost/+git/charm-k8s-mattermost [09:09] Thanks Chipaca : will give it a shot [09:09] bthomas: requires a custom image, instructions in the README [09:09] will read [10:11] The mattermost charm uses https://code.launchpad.net/~stub/interface-pgsql/+git/operator as a submodule . And the interface-pgsql uses opslib as a submodule. In the opslib/pgsql sub-directory of the interface-pgsql there is a link to ".." which creates a cyclic inode structure. [10:12] Even though I am able to clone the mattermost charm using my launchpad credentials cloning it recursively fails [10:12] Even submodule update fails. [10:24] bthomas: pgsql uses opslib as a submodule? [10:24] not sure i understand what that means [10:25] i guess i could check out that git repo again [10:27] Chipaca: interface-pgsql has a pgsql/opslib subdirectory in that subdirectory there is a link to ".." [10:32] bthomas: I don't see that [10:33] bthomas: maybe you updated the mod beyond what the charm specifies? [10:34] When I tried to build the mattermost charm, even though it built the charm it gave an error saying pgsql was not found. Upon digging further I found that it used a git submodule. There is no mention of this in the readme. [10:34] So I am not sure if it is ok to use the built mattermost charm without fixing the pgsql issue. [10:35] bthomas: how did you get the submodule? [10:35] Chipaca: even if I do git clone git+ssh://balbir-thomas@git.launchpad.net/~stub/interface-pgsql/+git/operator interface-pgsql I can see the link [10:36] bthomas: right, but the charm doesn't say 'grab the dev tip' [10:38] bthomas: or does it/ [10:39] Chipaca: Not sure. But the standard way to pull git submodules fails with mattermost charm. For example if I do "git clone --recursive git+ssh://balbir-thomas@git.launchpad.net/charm-k8s-mattermost". Tis fails to clone submodules. [10:40] Building mattermost charm without submodule produces an error, even though charm gets built. [10:40] bthomas: did you try cloning with the https url? [10:40] bthomas: it's possible you're missing the git-in-launchpad recommended git config; the .gitmodules file depends on that [10:41] Chipaca: Just did "git clone --recursive https://balbir-thomas@git.launchpad.net/charm-k8s-mattermost" with same result. I was not aware of git-in-launchpad. [10:42] * bthomas googling git-in-launchpad [10:42] bthomas: https://help.launchpad.net/Code/Git [10:42] Thanks Chipaca : will read [10:42] bthomas: the 'configuring git' section [11:07] Chipaca: you were right. It was the git config. Thanks. [11:07] bthomas: \o/ [11:34] Is it expected that when I try and push a charmcraft built charm to the charmstore it'll fail with "ERROR open mycharm.charm/bundle.yaml: not a directory" ? [12:03] Chipaca: do you think you'll have time to review https://code.launchpad.net/~axino/charm-k8s-gunicorn/+git/charm-k8s-gunicorn/+merge/389554 soon? Junien has another MP that he's working on that depends on that but want to make sure there's not too much rebasing needed [12:22] gnuoy: pushing it using the 'charm' tool? yes that won't work (unpack to a directory and push that) [12:23] ack, thanks [12:24] gnuoy: 'charmcraft upload' pushes to staging using the new APIs, if that's what you want though [12:24] and that one does take the .charm (and not a directory) [12:24] That is not what I want, but good to know, thanks. [12:26] mthaddon: i've still got one from deej to review, and then one from justin, and then that one [12:27] Chipaca: could i ask for this one to go above deej's? It's a day older, and there's another MP that depends on it, whereas deej's doesn't have another one depending on it [12:27] mthaddon: i'll swap it with that one then [12:27] perfect, thx [13:34] bthomas, did you get Mattermost running? I quickly tried deploying yesterday based on the README and it failed for me. [13:34] https://git.launchpad.net/~mattermost-charmers/charm-k8s-mattermost/tree/README.md [13:35] justinclark: Yes mattermost was running. It was idle because i did not provide it a database connection. But it was not my purpose to use it. I just wanted to check if it can be deployed and the container runs. [13:37] Ah okay. Thanks bthomas. It was probably the database for me too but got sucked into learning about cross model relations rather than going further with mattermost. [13:55] justinclark: where was the charm you wanted reviewed? [13:55] https://github.com/justinmclark/grafana-charm-base/blob/master/src/charm.py [13:55] Thanks Chipaca! [13:57] Just FYI, jam suggested a change yesterday in an issue I opened. That is still WIP. Here's the issue: https://github.com/canonical/operator/issues/392 [14:02] Pushed some other changes I made last night Chipaca. The charm deploys to microk8s just fine by itself, but it is quite uninteresting without relating a data-source for the dashboards. That still needs to be tested [14:03] :) [14:03] ok [14:03] justinclark: you're using 'charmcraft build'? [14:03] Yes. [14:03] justinclark: ok. You should probably embrace it more then :) [14:04] justinclark: right now you're getting ops via a git submodule [14:04] justinclark: instead of requirements.txt i mean [14:04] justinclark: and the 'fixpath' hack is not needed [14:08] Chipaca: ah yes that needs to be a more prominent TODO. I used an out of date example when first starting and just didn't bother changing it as testing was working as expected. [14:09] justinclark: the 'configured' and 'started' storedstate things were red flags but they're not used at all so that's alright (just drop them) [14:09] justinclark: you're using f-strings which don't work on python 3.5 (which you'll get in xenial) so you might want to drop those too [14:10] justinclark: and you're setting state around a pod.set_spec which _i think_ (but am waiting to hear back from juju people) doesn't make sense [14:11] because statuses are immediate but set_spec is not [14:11] IIRC at least :) [14:11] jam: if you're around already, maybe you can answer that? (haven't heard back from #juju) [14:11] morning Chipaca. set_spec is not immediate, it doesn't happen until the hook exits [14:11] (in 2.8) [14:12] jam: right, that one i know [14:12] jam: but setting status is immediate, right? [14:12] Chipaca: status = Blah is immediate, yes [14:12] so, yeah, that pattern is broken [14:12] so you can show progress during an operation [14:12] this one: https://github.com/justinmclark/grafana-charm-base/blob/master/src/charm.py#L350 [14:13] jam: right but using it show progress for something you're scheduling to happen later won't dtrt [14:13] in their defense, set_spec() does _sound_ immediate [14:13] Chipaca: correct. except that Juju filters Active status for the case of 'the pod hasn't actually started yet', and overrides it until the application pod has finished starting. [14:13] maybe we should've called it set_spec_sometime_in_the_future_at_your_leisure_please_no_rush [14:14] jam: oh that's sneaky [14:14] Chipaca: relation-set sounds immediate, too [14:14] Chipaca: this is a case where, Mark found out that 'leader-set' was immediate, which breaks the 'transactionality' that all other relation-get/relation-set code has. [14:15] So we went on a hunt for other things that violated that [14:15] and found pod-spec-set. [14:15] So we pushed for it to be changed [14:15] but having worked with charmers for much longer [14:15] I don't feel like pod-spec-set is an interaction *with other charms* it is an interaction with the charm and its application [14:16] so I no longer feel it should be transactional [14:16] (we don't put a transaction around 'systemctl start foo' [14:16] however, we do need to deal with how it is currently, and not how it might theoretically work [14:16] Also, the new work gets rid of pod-spec-set for charms running as sidecars [14:17] yep [14:19] jam: does the 'pod hasn't actually started yet' override thing mean that the code i linked to would currently dtrt w/2.8? [14:22] Chipaca: I don't think it would stay in the "maintenance with 'setting pod spec'" but otherwise yes [14:22] (eg, it uses Juju's 'pod not started yet' message, though I don't know the exact words) [14:22] k [14:29] * justinclark thanks Chipaca and jam for feedback :) [14:31] * justinclark pushes python 3.5 under the rug and blows things up [14:31] :) [15:00] justinclark: one silly thing I'd also do in your charm is make the log.error call for missing fields mention exactly what was missing, instead of everything that's required [15:07] Chipaca: will do. Can you elaborate on the 'fixpath hack'? [15:08] I'm not sure what that hack is. [15:08] justinclark: your charm.py does 'import fixpath' [15:08] justinclark: fixpath.py just adds lib to sys.path [15:08] justinclark: this was needed before charmcraft took on the job of setting up the environment for the charm [15:09] justinclark: it's a hack, which is why you need to tell pyflake to ignore it [15:13] I see. Thanks. [19:13] EOWing here [19:13] bye all! see you tuesday [19:41] have a great weekend ChanServ [19:41] dang, gone already :)