[08:32] ｇｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｏｄｍｏｒｎｉｎｇ！ [08:41] 𝐺𝑜𝑜𝑑 𝑀𝑜𝑟𝑛𝑖𝑛𝑔 [08:41] [08:50] bthomas: how're things going? [08:53] Chipaca: I working on how to refactor prometheus charm test cases to use ops.testing.Harness. The Harness supports testing the whole Charm class. Some of the test cases in the prometheus charm test utilties like "adapter/k8s.py" which makes rest calls to Kubernetes API server. This does not seem to fit the Harness framework. I am ignoring this for now and looking at what all can be refactored. [08:55] Also reading a bit of some of the python standard library docs [09:14] morning Chipaca [09:14] morning jam: looks like you are still jet lagged [09:15] bthomas, at one point I was thinking to always use my local time. Right now, I just always say good morning. [09:15] It is always morning somewhere [09:16] :-) It must be night there and you are awake. [09:16] bthomas, I haven't moved to the US yet. it is only 1pm here. [09:16] ah [09:16] I'm on a plane next Wednesday. [09:17] fly cartesian [09:20] Chipaca, what voice is ChanServ giving you? [09:21] jam: a blue one [09:51] what a nightmare this logging thing [09:51] reloading a module does not reset its globals, so test_log's reset_logging was wrong [09:52] and it broke logassert for some reason i haven't been able to figure out [09:57] mmm, pytest-xdist → ops test suite finishes in 12 seconds [10:08] Chipaca, shiny. How does xdist handle mock patching, etc. Just farmed out subsets to different processes? [10:08] jam: yep, just farmed out [10:08] jam: different from pytest-parallel which does threading (and breaks spectacularly) [10:08] Chipaca, I don't know if you ever used it, but 'subunit' from Robert Collins had a bunch of similar ideas [10:09] used it for syncdaemon, a couple of lives ago [10:10] Which component is responsible for ensuring that status of a pods in an application unit deployed by a charm is correctly set ? Is it Juju/Kubernetes, the Operator Framework or the Charm ? [10:10] bthomas, the charm sets a basic status, Juju tempers that status as it knows that the pod hasn't finished starting yet. [10:11] PR operator#373 opened: fix up logging tests impacting global state so much [10:11] jam: ^ that was biting me yesterday when trying to test the storage heuristics branch [10:17] Chipaca, responded [10:21] jam: re²sponded [10:21] :) [10:21] rerere [10:22] reeee [10:33] Chipaca, probably here is better for active conversation: What I'd like is that for whatever code path we are executing that is calling setup_root_logging, refactor it to not be called in the path that the test suite needs. [10:52] jam: i could add a no_logging bool on main [10:52] Chipaca, from a side point, why are we triggering main itself for that many tests... [10:53] jam: well, it is test_main :-p [10:53] jam: I'll see if it can be pared down a bit, maybe i got carried away? [10:53] the ones that call out to subprocess shouldn't need this [10:53] Chipaca, if it is only test_main then we are probably ok [10:54] (and test_log, but that is also explicit about logging) [10:54] yep, only test_main [10:54] It just felt like lots of test suites, but that is just because we use inheritence to do scenarios [10:54] and I think I should be able to take it off the _TestMain subclasses [10:54] I thought the inherited TestMain were using subprocess [10:55] exactly, i'll see if those were needed [10:55] give me 5 [10:58] jam: indeed i can drop all the _TestMain decorators [10:59] that was silly of me, got carried away [11:00] jam: better now? [11:01] Chipaca, yep. I'm curious why you need TestDispatch, something there where we are doing direct main() interaction? [11:01] jam: all the test cases there call main directly [11:02] ¡Muy buenos días a todos! [11:02] jam: via the one _check method [11:02] morning facubatista [11:02] Chipaca, approved [11:02] Morning facubatista [11:02] facubatista: 👋❗ [11:02] https://www.ubuntusoftware.net/about/ looks like a trademark infringement waiting [11:03] what the [11:03] and with a german phone [11:03] Hell, almost certainly is an infringement [11:04] They aren't using the Circle of Friends at least [11:05] Chipaca, how is it breaking logassert? maybe we could improve logassert's resiliance? [11:05] facubatista: I don't know :-) logassert doesn't see the logs [11:05] facubatista: probably because test_log was doing logging.shutdown(); importlib.reload(logging) [11:06] ja [11:06] facubatista: not sure logassert can be made resilient to that :) [11:06] (and reload doesn't do what you'd expect with globals) [11:07] Chipaca, it doesn't re-load the module? [11:08] facubatista: it reloads it, but the module's dict is reused [11:08] facubatista: also, [11:08] If a module imports objects from another module using from ... import ..., calling reload() for the other module does not redefine the objects imported from it — one way around this is to re-execute the from statement, another is to use import and qualified names (module.name) instead. [11:09] makes sense, yes [11:10] it looks like test_log can be improved, yes [11:10] facubatista: and logging uses weakref which makes the whole thing harder to reason, for me [11:10] facubatista: #373 [11:10] PR #373: fix up logging tests impacting global state so much [11:10] Chipaca, any feedback on https://github.com/canonical/charmcraft/pull/99 so I can land that for Facu? [11:10] PR charmcraft#99: charmcraft/jujuignore.py: Allow extending the list of patterns [11:11] jam: me? reviewing _other_ people's code? [11:11] * Chipaca apologises [11:12] jam, if you could change the "from .." thingie, it would be great, thanks! [11:12] facubatista, I just pushed the change [11:12] awesome, thanks! [11:12] Chipaca, apologizing for reviewing someone else's code? Or for misspelling apologises :P [11:14] * Chipaca makes guises for Apollo [11:16] jam: not sure why the test of relative imports changes so suddenly, for me [11:17] year ye, hear ye: {**d} is weird [11:19] Chipaca, I thought it would be (.*d) :0 [11:20] Chipaca, well, you might create .hidden file, but you'd never create a ..hidden file [11:20] I think it breaks the feeling of a path [11:21] ../foo is normal [11:21] ..foo is not [11:21] jam: if d is a map object (as in collections.abc.Mapping), {**d} creates a copy of it (like dict(**d)) [11:21] if a and b are mappings, {**a, **b} creates a new, merged one [11:22] ah, I'd never tried to do that with {}. [11:22] bah, in both cases it's not a copy/merged one but a dict with the k/v [11:22] new from 3.5 [11:22] don't like it [11:22] :) [11:22] was reminded of it by somebody just now [11:22] bah humbug I say [11:22] all them modern bows and arrows [11:22] Chipaca, I suppose it is a syntax like list[:] [11:23] but {**dict} doesn't quite ring the same to me [11:23] hush, next they'll be doing [*list] [11:23] waaait [11:23] that already works [11:23] /o\ [11:31] Chipaca, I will be missing the next meeting. The finished processing my wife's passport but I have to go pick it up *right now* or they keep it over the weekend. [11:32] jam: go go go [11:32] Chipaca, our scientists were so preoccupied with whether or not they could, they didn’t stop to think if they should [11:32] [*list] [11:33] jam: ask me about [*a, *b] [11:34] Chipaca, that looks to my eyes as creating [[1,2,3], [4,5,6]] [11:34] jam: no that would be just [a,b] [11:34] clearly * is the asplode operator [12:40] flake8 gives me W503. But if I fix that it gives me W504. Since W503 is deprecated I am adding it to the ignore list. [12:58] bthomas: maybe your flake8 is old? [12:59] bthomas: because that change was merged in 2018 [12:59] Chipaca: Thanks. my flake8 version = 3.7.9 . [13:03] bthomas: huh, i can't get those to trigger here [13:03] * Chipaca tries a bit more [13:03] Chipaca, hi good morning. I am in process of setting the environment variable in my pod spec do you know how can we setup for conaintaners somehow env: is not recognized [13:03] ops.model.ModelError: b'ERROR json: unknown field "env"\n' [13:04] I am using the template to define the pod spec and replacing the variable. [13:04] narindergupta: can you show me the code? [13:06] Chipaca, I started with this http://paste.ubuntu.com/p/v6GXk2RRKW/ but this is my current template file http://paste.ubuntu.com/p/pfFNs4vPwS/ [13:06] As I may ask to add more feature in k8s related to resources [13:07] Chipaca, as K8s get auto scalability feature based on resource usage which is good way to do autoscaling as well. [13:08] Chipaca, this is my http://paste.ubuntu.com/p/PGgR2Ccnt4/ charm.py [13:09] Chipaca: hmmm . It was triggering on this commit of mine https://github.com/balbirthomas/charm-k8s-prometheus/commit/d4a82c43d1e8f56a9bf1970dbf4aae73ebffb817 (second hunk) [13:11] * bthomas rushes to get coffee before standup [13:27] narindergupta: I think your problem is that in that first link, the 'env' block is indented such that it isnside a port [13:29] narindergupta: I suspect most of those things aren't meant to be inside a port [13:29] Chipaca, correct it is not the port let me fix that and restart [13:30] I was trying to follow this https://raw.githubusercontent.com/kubernetes/website/master/content/en/examples/application/cassandra/cassandra-statefulset.yaml [13:30] Chipaca, may I know what else is covered apart from port, env, readinessProbe, [13:31] narindergupta: covered by what? [13:32] By framework like when I tried resources, securityContext and lifecycle I got the same error [13:32] narindergupta: because you're putting them under 'ports', and they don't go there [13:32] narindergupta: this has nothing to do with the framework [13:32] Oh ok [13:32] Chipaca, meeting? [13:32] facubatista: trying to [13:38] Chipaca, here is new modified file with correct syntax and hopefully it will work http://paste.ubuntu.com/p/s49Xhztf3Y/ [13:41] Chipaca, same error ops.model.ModelError: b'ERROR json: unknown field "resources"\n' [13:41] application-charm-k8s-cassandra: 13:40:47 ERROR juju.worker.uniter.operation hook "start" (via hook dispatching script: dispatch) failed: exit status 1 [13:43] narindergupta: is this file supposed to have the same structure as the cassandra-statefulset.yaml you linked? [13:46] Chipaca, this is pod spec I am planning to create through template and the way Cassandra deployed on kubernetes is the one from link I shared which I am trying to replicate through juju. [13:47] Chipaca, while writing Kafka charm I used this http://paste.ubuntu.com/p/ZvCf8Drt3R/ [13:50] narindergupta: https://discourse.juju.is/t/updated-podspec-yaml-new-features/2124 [13:50] narindergupta: in particular, a lot of those things need to be under 'kubernetes' [13:51] narindergupta: look for 'k8s specific container attributes' [13:51] Oh ok [14:41] facubatista: https://github.com/canonical/charmcraft/pull/116 fwiw [14:41] PR charmcraft#116: include install info in README [14:42] ack [14:49] facubatista: also plz can haz https://github.com/canonical/operator/pull/373 [14:49] PR #373: fix up logging tests impacting global state so much [14:49] facubatista: so i can merge it into the storage thing [14:50] Chipaca, reviewing [14:59] facubatista: thanks! [15:06] PR operator#373 closed: fix up logging tests impacting global state so much [15:31] PR operator#374 opened: use controller-side storage automatically in some cases [15:43] facubatista: i'm getting test failures on charmcraft master [15:44] nice! (not) [15:45] facubatista: can you test if https://paste.ubuntu.com/p/4G3vG6zz9B/ still works for you in your particular env? [15:46] Chipaca, how are you running the tests? [15:46] facubatista: in a virtualenv [15:46] Chipaca, thanks I was able at least start the pod now as I have to remove few things and check back later the issues with template [15:46] facubatista: using pytest [15:46] narindergupta: good to hear [15:48] Chipaca, but that works for me... see https://pastebin.canonical.com/p/5vDkCkZYHM/ , l.1 is how I normally run the tests, l.22 is inside the venv [15:48] facubatista: you seem to always have a PYTHONPATH i guess :) [15:55] Chipaca, oh, yes [15:55] Chipaca, testing your patch... [15:58] Chipaca, it worked, although I'd prefer this: https://paste.ubuntu.com/p/qZWs56W43k/ [15:59] facubatista: if you can reproduce a scenario where that is needed, sure :) [15:59] i almost wrote exactly that, but, aiui if you don't need it outside you won't need it inside [16:00] capisci? [16:02] Chipaca, is `export PYTHONPATH=` the way to "delete" a envvar? [16:02] facubatista: unset [16:03] facubatista: although most things don't differentiate between empty and unset [16:04] Chipaca, see https://paste.ubuntu.com/p/8p26CqbBvT/ [16:05] I need the PYTHONPATH to the venv be reused, no matter if I had it before or not [16:06] Chipaca, forgot to include in that pastebin that my PYTHONPATH is normally set to '/home/facundo/.bin/pymods' (so the envvar is there, totally unrelated to any venv) [16:06] * facubatista brb [16:45] facubatista: you there? [16:46] yeap [16:47] Chipaca, yeap [16:48] facubatista: something funky going on with the environ for test_main breaking setting TMPDIR, but i figured a workaround [16:50] TMPDIR? [16:50] facubatista: explanation in a pr soon [16:50] incenption tests are hard [16:50] inception ones too [16:52] facubatista: https://github.com/canonical/charmcraft/pull/117 [16:52] PR charmcraft#117: a couple of fixes for tests [16:53] facubatista: fixes the leftover tmp dirses [16:53] and with that i think i eod [17:17] * Chipaca EODs [18:13] justinclark, easy PRs for you to start reviewing... [18:13] this is trivial: https://github.com/canonical/charmcraft/pull/116 [18:13] PR charmcraft#116: include install info in README [18:13] this is easy in the fix, but may lead you to read some code: https://github.com/canonical/charmcraft/pull/117 [18:13] PR charmcraft#117: a couple of fixes for tests [18:14] and this one is kind of easy too, and dives you right into complex code: https://github.com/canonical/charmcraft/pull/118 [18:14] PR charmcraft#118: Some changes in preparation for big "include everything but jujuignored" branch [18:27] Awesome. I'll start going through these facubatista. [18:28] justinclark, awesome! let us know any questions [19:42] Chipaca, do we know how to get IP address of a pod? [20:06] facubatista: I know you're working on issue #39, not sure how far along it is, but I just noticed the README[.md] is not included, hence, no info in charmstore uploads. Miight be nice to pick up README and README.md in the mean time in current code, unless you're close. This file can't be relocated to /src to be pulled in automatically. [20:06] PR #39: Change Relation.apps to Relation.app [20:07] drewn3ss, I'm close on finishing #39, PRs are already proposed [20:07] PR #39: Change Relation.apps to Relation.app [20:08] sweet, ta! [20:08] charmcraft#39, that is [20:08] Issue charmcraft#39: Include all project files (whatever is present there) [20:32] * facubatista eods