| tlm | hpidcock: if you checkout bootstrap_test.go #668, open to ideas for fixing this hacky function | 03:23 |
|---|---|---|
| wallyworld | thumper: will you have a chance to look at the modelcache race PR? | 03:26 |
| wallyworld | tlm: to avoid a bit of churn, can you move the NewK8sWatcherFunc (and strings version also) type definition back to the original location in the code? | 03:31 |
| hpidcock | tlm: it's a test, not too concerned. But essentially you *can* use go mock to mock that.... but it's probably not worth the effort tbh. | 03:32 |
| tlm | I might throw a panic in their if we step outside of bounds. Might make it a bit more clear if it gets added to hpidcock | 03:32 |
| tlm | taking a look wallyworld | 03:33 |
| wallyworld | tlm: can't we look at the params passed to the func to see what watcher to return? | 03:33 |
| thumper | wallyworld: looking now | 03:34 |
| tlm | not in this case. When we spoke I thought that was the case | 03:34 |
| tlm | I thought about just putting all the watchers in a slic inc and index, If index > panic | 03:35 |
| hpidcock | tlm: see Call and RecordCallWithMethodType in go mock... you can mock calls like this.. but I'm totally happy with the way you have it | 03:35 |
| tlm | i'll have a read | 03:35 |
| wallyworld | also, don't panic - return an error | 03:36 |
| wallyworld | and let the test fail | 03:36 |
| tlm | wallyworld: happy to move those functions back but they make no sense where they where (defined miles away from where they are used). | 03:44 |
| wallyworld | tlm: i don't mind a lot - they were used near where they ariginally were also, approx line 125 | 03:45 |
| wallyworld | and all the func types were together | 03:45 |
| hpidcock | wallyworld: do we have schema's for mongo documents anywhere, or is it just the state code that deals with the structure of docs? | 03:46 |
| wallyworld | hpidcock: all the monfo docs should be confined to state. depends on the age of the code, but we typically expose a struct which wraps the mongo doc | 03:48 |
| wallyworld | tlm: there's still a bunch of import issues | 03:52 |
| wallyworld | plus stuff what was previously private and now unnecessarily exported | 03:52 |
| wallyworld | very close though | 03:53 |
| hpidcock | I guess we now use monfodb | 03:53 |
| hpidcock | :) | 03:53 |
| wallyworld | ha de ha ha | 03:53 |
| tlm | fixing them now :) | 03:53 |
| tlm | made them public as I couldn't see much reason to keep them private for a non const var, will update | 03:54 |
| kelvinliu | wallyworld: https://github.com/juju/juju/pull/11145 changed the config format, could u take a look again? ty | 03:54 |
| wallyworld | tlm: public functions etc by rights need comments so easier to keep struff private etc | 03:54 |
| tlm | okey doke | 03:55 |
| wallyworld | kelvinliu: will do, just let me get yet another doubles espresso first | 03:55 |
| tlm | go tripple | 03:55 |
| kelvinliu | going to grab some food n double shot espresso, come back later | 03:57 |
| hpidcock | I'm going to do the same... food, octo-shot of coffee | 04:01 |
| wallyworld | kelvinliu: let me know if my comments are unclear | 04:47 |
| kelvinliu | wallyworld: HO? | 04:57 |
| wallyworld | ok | 04:57 |
| nammn_de | manadart: as we discussed before. A smaller patch in the series of patches for remove-spaces https://github.com/juju/juju/pull/11183 | 10:32 |
| nammn_de | manadart: while at it, did you have time to look at the email I sent you yesterday evening? | 10:34 |
| ec0 | curious behaviour in 2.7.1, running kubernetes-e2e 'test' action: juju run-action kubernetes-e2e/0 test -> ERROR "action-6" is not a valid action tag | 10:50 |
| ec0 | has anyone seen this before? | 10:50 |
| manadart | ec0: Are the client, model and controller all 2.7.1? | 11:19 |
| nammn_de | manadart achilleasa stickupkid: this seem to cause some racy condition: https://github.com/juju/juju/blob/ff32067866004a0a04d5dba8e281dea7857f33ea/cmd/juju/status/status_internal_test.go#L6096 . It may update the model later the the call right after that to run the status output. How would you guys solve that? | 11:19 |
| ec0 | manadart: yep all 2.7.1 | 11:50 |
| ec0 | interesting, let me double check client | 11:51 |
| ec0 | ahah, that'll be in manadart | 11:51 |
| ec0 | 2.6.10 client | 11:51 |
| manadart | ec0: Yes, I think it will be unable to parse the new tag form. | 11:52 |
| achilleasa | so, regarding my yest question about that odd test, I found what made it work: https://github.com/juju/juju/blob/develop/state/ports.go#L455. Looks like this was only enforced when appending a port range and not when creating the doc or when a port range gets closed | 11:52 |
| achilleasa | guess I will move it into the Build for the txn | 11:53 |
| ec0 | that was it manadart, snap refresh juju --channel=stable got me 2.7.1, and the same action worked properly | 11:53 |
| ec0 | thanks! | 11:53 |
| manadart | ec0: Good to hear; no problem. | 11:53 |
| nammn_de | manadart: I will remove the state removespaceOps for now as this will be eventually patched again. I started with the cmd, as this was mostly done and wanted to start with the easier and smaller part | 12:02 |
| nammn_de | manadart: pushed the changes | 12:03 |
| nammn_de | Do we have somewhere some functions to indiciate that the ID corresponds to a unit/application ect. This seems to be scattered everywhere but not on one common place to me | 12:47 |
| nammn_de | manadart: ah just realized, do you have 2 min to talk about the email? | 13:52 |
| manadart | nammn_de: I am back in the HO. | 13:53 |
| achilleasa | jam: so I finally tracked down the issue with the broken test. The txn was indeed failing as expected. The new Build impl iterates the list of ports from the original port model (I removed the redundant copy bits). Turns out that attempt #1 caused my build code to return ErrNoOperations... why? because of *this* https://github.com/juju/juju/blob/develop/state/ports.go#L417 | 14:15 |
| achilleasa | so the code behaved as if you tried to open an already opened range twice and obviously that's a noop | 14:16 |
| achilleasa | removing that line makes all tests happy again | 14:17 |
| hml | achilleasa: i’m going to work on moving the spike-v2 from process-hook to dispatch. tease out anything from the spec comments. | 14:31 |
| achilleasa | hml: IIRC there was a mismatch with an envvar name but I think it got dropped from the new spec | 14:32 |
| hml | achilleasa: ack | 14:33 |
| stickupkid | manadart, I can't replicate bug 1860865 | 15:12 |
| mup | Bug #1860865: [2.7.1] Incorrect container architecture (i386, not amd64) used after a host upgrade to Focal - jujud fails to start <juju:Triaged> <juju 2.7:Triaged> <https://launchpad.net/bugs/1860865> | 15:12 |
| stickupkid | installed: 2.7.1 (10357) 71MB classic | 15:14 |
| stickupkid | installed: 3.19 (13162) 67MB - | 15:14 |
| stickupkid | juju and lxd versions | 15:14 |
| stickupkid | this is in a multipass vm | 15:14 |
| manadart | stickupkid: And you had a controller up before the upgrade? | 15:28 |
| stickupkid | yeah | 15:29 |
| manadart | stickupkid: Let me know when you are done with rick_h and free to HO. | 15:30 |
| stickupkid | manadart, ping | 15:44 |
| manadart | stickupkid: Does the schema generation not handle embedding properly? | 16:08 |
| manadart | stickupkid: Any chance to jump back on HO quickly? | 16:09 |
| achilleasa | rick_h: I have converted 5/8 uniter calls into model operations which I can now compose together. I think it would be best to get these into an initial PR and then do a second PR for the rest which is more complex (AddUnitStorage for example). wdyt? | 17:27 |
| hml | achilleasa: dispatch spike: https://pastebin.ubuntu.com/p/rTxVJt9sTP/ | 17:28 |
| achilleasa | hml: awesome! | 17:32 |
| rick_h | achilleasa: sounds like a plan | 17:40 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!