timClicks | addyess, techalchemy: agreed - there's lots of useful refactoring in there | 00:41 |
---|---|---|
techalchemy | timClicks, most significatly adding compatibility is important, I'm installing a fork so i can use python 3 which is a bit annoying | 00:51 |
timClicks | whelp | 00:51 |
timClicks | is juju's over-the-wire communication mechanism, e.g. server-side sockets, documented anywhere publicly? | 01:12 |
wallyworld | not that i now of | 01:12 |
wallyworld | there may be an old wiki page somewhere | 01:13 |
timClicks | *websockets | 01:15 |
timClicks | wallyworld: okay that's fine.. I'll put that down as another todo | 01:15 |
wallyworld | one of many | 01:17 |
wallyworld | hpidcock: adding a new operation tag for the actions work https://github.com/juju/names/pull/102 | 02:36 |
hpidcock | wallyworld: looking now | 02:37 |
wallyworld | ta | 02:37 |
wallyworld | hpidcock: i hate go dep, bring on go mod https://github.com/juju/names/pull/103 | 03:28 |
hpidcock | one of us one of us one of us! | 03:28 |
wallyworld | ta. i hope this works | 03:29 |
wallyworld | and it did, dep ensure is happy now | 03:34 |
tlm | hpidcock: I think we may need to change this watcher design a little, the go routine has broken these tests because they wants such strict ordering | 03:47 |
tlm | got time for a HO ? | 03:48 |
wallyworld | tlm: it's not possible to use expectanytimes? | 03:49 |
wallyworld | and /or expect after etc? | 03:49 |
wallyworld | can jump in standup if you want | 03:50 |
tlm | just looking at this now. Can you give me 20, will try and get a better example of the problem I have hit | 03:50 |
wallyworld | ok | 03:50 |
tlm | wallyworld: might be user error, if you got 10 minutes would love to rubber duck it | 03:59 |
wallyworld | ok | 03:59 |
timClicks | signing heading off for the week | 04:36 |
tlm | cya | 04:36 |
hpidcock | oops, how did you go tlm? | 04:40 |
tlm | good think we found a better path | 04:43 |
hpidcock | the gift that keeps on giving | 04:44 |
wallyworld | tlm: it's ok if the watcher stuff misses 2.7.2 - it's not ciritical as nothing is broken per se. we can finsih it up monday | 06:50 |
tlm | wallyworld: yeah thats fine | 06:55 |
tlm | PR will be in my monday | 06:55 |
tlm | just a bit of boilerplate to mock all this in | 06:55 |
=== parlos is now known as parlos_afk | ||
=== parlos_afk is now known as parlos | ||
=== parlos is now known as parlos_afk | ||
nammn_de | manadart: added the changes and phase to skip the model if its not controllermodel for settingsdelta. | 10:02 |
nammn_de | in regards to specprep call for removespace: Should we include rename-space or would you say it's done and its not needed? | 10:02 |
nammn_de | https://github.com/juju/juju/pull/11143 | 10:02 |
manadart | nammn_de: I don't think we need to spend time with specification given how far it is along - we'll just do what we need to land the patch. | 10:04 |
nammn_de | manadart: i also updated the qa steps in the pr to match the change | 10:04 |
manadart | nammn_de: I will review 11143 when I get a chance. I need to make sure I get this critical fix for 2.7.2 done first. | 10:05 |
nammn_de | manadart: great, sure no worries! Gimme a ping, when you find the chance for the spec talk | 10:06 |
=== parlos_afk is now known as parlos | ||
stickupkid_ | achilleasa, you free ? | 10:41 |
achilleasa | sure | 10:41 |
achilleasa | daily? | 10:41 |
stickupkid_ | yeah | 10:41 |
stickupkid_ | damn it, I missed a PR | 11:09 |
stickupkid_ | :( | 11:09 |
stickupkid_ | it's the one where I re-wrote the firewall migration import as well | 11:14 |
=== parlos is now known as parlos_afk | ||
manadart | stickupkid_: Got a sec for HO? | 11:15 |
stickupkid_ | sure | 11:16 |
=== parlos_afk is now known as parlos | ||
=== parlos is now known as parlos_afk | ||
achilleasa | manadart: quick HO? | 11:43 |
manadart | achilleasa: OMW | 11:46 |
=== parlos_afk is now known as parlos | ||
manadart | Fix for the machine address space ID upgrade step: https://github.com/juju/juju/pull/11167 | 12:26 |
stickupkid_ | well that back port went smoother now I added the missing PR | 12:35 |
stickupkid_ | yisss, go vet passed, we've got futher than last time :| | 12:43 |
stickupkid_ | such a good idea to have this static analysis! | 12:43 |
* stickupkid_ pats self on back | 12:44 | |
achilleasa | manadart: from a quick inspection of some of the uniter calls (OpenPorts) in particular, I think it might be better to have the state bits return back ModelOperation instances which then get aggregated in a single operation | 12:45 |
achilleasa | there are parts that have side-effects (e.g. update doc fields once the txn completes) | 12:46 |
achilleasa | s/doc fields/model fields/ | 12:46 |
achilleasa | we could of course throw in an adaptor if the core doesn't care about Done and only implements Build | 12:47 |
manadart | achilleasa: Happy to take a look if you get a draft up. | 12:49 |
achilleasa | manadart: not quite there yet but will try to get something up for comments | 12:50 |
manadart | nammn_de: You want to chat before standup? | 12:55 |
nammn_de | manadart: sure! | 12:55 |
=== parlos is now known as parlos_afk | ||
=== parlos_afk is now known as parlos | ||
achilleasa | manadart: here is an example for OpenPorts: https://paste.ubuntu.com/p/2jhgH2xgW7/ I have patched the original OpenPorts method to fetch and apply the operation. The facade will eventually call into OpenPorts (through unit.OpenPorts and friends) and compose the modeloperation with other ops | 13:23 |
achilleasa | composing the txn bits is not trivial though; if an error occurs we won't know which []txn.Op caused it to pass the error to the Done method | 13:24 |
stickupkid_ | achilleasa, can you not change how txn works? | 13:25 |
=== parlos is now known as parlos_afk | ||
* achilleasa runs away | 13:25 | |
rick_h | lol | 13:26 |
achilleasa | will provide more info in standup. Maybe I am missing something | 13:26 |
=== parlos_afk is now known as parlos | ||
stickupkid_ | achilleasa, RunTransactionObserver might be your friend | 13:34 |
stickupkid_ | https://paste.ubuntu.com/p/7V85FZTbrW/ | 14:11 |
stickupkid_ | it did pass integration tests :) | 14:11 |
rick_h | guild if anyone wants to have fun with the community check out https://github.com/juju/gomaasapi/pull/85 | 14:21 |
nammn_de | manadart: I took a quick look and qa on your pr. Added something I saw | 14:25 |
hml | achilleasa: RE: 11166, i’m on the fence for both points. We can chat it out? | 14:30 |
achilleasa | meet you in daily in 1m | 14:31 |
hml | achilleasa: omw | 14:31 |
hml | rick_h: want to join our conversation. ^^ | 14:39 |
nammn_de | manadart: can a machine have an address in that space, but not a constraint? | 14:44 |
manadart | Yes. But if it is not constrained, we still allow renaming. | 14:49 |
nammn_de | manadart: makes sense, thanks | 14:50 |
manadart | nammn_de: Added another review to you patch. | 14:50 |
manadart | nammn_de: Also, a suggested simplification for you op/factory: https://pastebin.canonical.com/p/4vgXNZYQVR/ | 14:50 |
rick_h | hml: sorry, was rebooting/etc still going? | 14:52 |
rick_h | wheee waving from Description: Ubuntu Focal Fossa (development branch) | 14:53 |
manadart | nammn_de: Thanks for the review. Can you or stickupkid_ approve it? | 14:56 |
rick_h | looks like folks are done. hml let me know if you want to catch up | 14:56 |
stickupkid_ | manadart, i didn't do q&a... nammn_de can you approve once you've done it | 14:57 |
nammn_de | done | 14:57 |
manadart | Ta. | 14:57 |
hml | rick_h: we had to stop. can catch up now? | 15:11 |
rick_h | hml: omw | 15:11 |
nammn_de | manadart: regarding your comment: https://github.com/juju/juju/pull/11143#discussion_r373516389 do move the method to `ConstraintsOpsForSpaceNameChange`. I intentionally wrote the logic outside in the `rename.go` file. To comply with your review I have 2 ideas: - move the space logic into the `ConstraintsOpsForSpaceNameChange` or give the | 15:35 |
nammn_de | `ConstraintsOpsForSpaceNameChange` a kind `filter func()` as a second param to apply the space changes. Or did you have something else in mind? | 15:35 |
hml | simple review for someone? https://github.com/juju/juju/pull/11169 | 15:39 |
hml | achilleasa: i’ve udpated 11166 with concensus from rick_h , i believe your concerns are addressed as well. pls let me know | 15:40 |
achilleasa | hml: should I wait for the rename commit before reviewing? | 15:43 |
hml | achilleasa: yes, just to be sure. could you do QA though? only the state-set key= piece will change. just so i have a heads up of issues while i’m at the name change | 15:44 |
achilleasa | will do | 15:46 |
hml | achilleasa: ty | 15:46 |
nammn_de | manadart: for 2 of your review points I am unsure what you mean. I added comments to them | 15:53 |
achilleasa | stickupkid_: RunTransactionObserver won't help as it doesn't tell you which Op failed; however, I think I can work around this by moving error annotations in Build()... should be good enough for the ConsolidateState call I am working on (I hope) | 15:54 |
manadart | Need a review for the instance poller fix for space names: https://github.com/juju/juju/pull/11170 | 16:12 |
rick_h | guild if anyone has time please help ^ for release setup | 16:14 |
achilleasa | manadart: looking | 16:15 |
manadart | achilleasa: Actually doesn't change the instance poller :) I need to do it here, because bootstrap also retrieves instance addresses. | 16:17 |
achilleasa | manadart: I think fixing it in the provider as you did is the best way to go | 16:18 |
stickupkid_ | achilleasa, I'd be happy in making txn more aware of failed txn.Ops | 16:38 |
stickupkid_ | achilleasa, just need to ensure not to leak info | 16:39 |
achilleasa | stickupkid_: I think we would need to tweak mgo to return tha index of the failed operation (maybe wrap it in an error that can be unwrapped) | 16:39 |
stickupkid_ | achilleasa, exactly my thinking | 16:40 |
nammn_de | achilleasa: I am not 100% deep in all possibilities of spaces. Thought you know more and as manadart is out. What possibilities exist? Given the comment from manadart https://github.com/juju/juju/pull/11143#discussion_r373477221 | 16:41 |
achilleasa | stickupkid_: for the time being, the state bits that I am working on run the txn and annotate the error. I can probably do that in the build code and just return the first error out | 16:41 |
achilleasa | nammn_de: looking in 5' | 16:44 |
hml | achilleasa: removing set from utils, makes the problem bigger, as now 5 versions of the charm package now conflict. which raises at least one question of… why are we using 5 versions of charm | 16:47 |
achilleasa | hml: workaround is to remove Tags from utils/set | 16:48 |
achilleasa | we don't seem to be using it in juju | 16:48 |
nammn_de | stickupkid_: is it possible from a test to read the content of a txn.Op? Without needing to apply it? | 16:48 |
achilleasa | hml: fifth time's a charm? :D | 16:48 |
hml | achilleasa: lol | 16:49 |
hml | is it 5 oclock somewhere yet? | 16:49 |
achilleasa | here, in 11m ;-) | 16:49 |
hml | ha! | 16:50 |
stickupkid_ | nammn_de, what do you mean, a txn.Op is a struct, you can do what you want, or am I missing something? | 16:50 |
stickupkid_ | nammn_de, are you wanting to do a "dry run"? | 16:50 |
nammn_de | stickupkid_: yeah, right. I'am writing a test, which returns Ops. Something like a dry run would be great | 16:50 |
nammn_de | txn.Op saves my change in an Update interface. Which I probably need to parse back. Just thought we alreadyt had something in place for that | 16:51 |
nammn_de | like the dry run | 16:51 |
stickupkid_ | so if you're writing a test, then shouldn't it be `reflect.DeepEqual(expected, txn.Op{Update: bson.D{{"$set", bson.D{{"message", message}}}})` | 16:52 |
nammn_de | stickupkid_: thanks! Probably what I had in mind | 16:52 |
hml | achilleasa: i wonder if the problem is not creating a new utils.v2? | 16:56 |
nammn_de | stickupkid_: lol ops[0].Update.(bson.D).Map()["$set"].(state.ConstraintsDoc).Spaces | 17:04 |
achilleasa | hml: left some comments in 11166; do you want me to defer QA till your changes land? | 17:04 |
hml | achilleasa: looking | 17:05 |
hml | achilleasa: why don’t i make these changes and the others before qa | 17:07 |
stickupkid_ | what's the Map? | 17:29 |
stickupkid_ | nammn_de, | 17:29 |
nammn_de | stickupkid_: was just debugging through it and it seems that bson.D has a function to return the saved transaction as a map[string]interface{} | 17:49 |
=== narindergupta is now known as narinderguptamac | ||
achilleasa | stickupkid_: manadart this is my current approach to composing multiple ModelOperations together: https://paste.ubuntu.com/p/dbGNR3wh4w/ | 18:01 |
stickupkid_ | achilleasa, could you not ignore the err message, but log it out? | 18:02 |
stickupkid_ | achilleasa, at least we're not ignore it completely | 18:02 |
stickupkid_ | ? | 18:03 |
achilleasa | stickupkid_: ideally we would need to de-dup (most Done() impls will prob just return the error straight out) and pack in a multi-error | 18:03 |
achilleasa | I will try to come up with something bettern on Monday | 18:03 |
achilleasa | we could also simply return the error out | 18:04 |
achilleasa | but this snippet works as expected if err == nil | 18:05 |
hml | rick_h: do we have any precendent for hook tools returning not found | 20:16 |
rick_h | hml: thinking, like what example? | 20:18 |
hml | rick_h: since we’re going with state-set can set an empty string and will not delete the key. | 20:18 |
hml | rick_h: the follow on to me is that state-get will return not found for keys not found. | 20:19 |
hml | rick_h: o.w. how to distinguish between not set (found) and empty string | 20:19 |
rick_h | hml: hmm, at first I was thinking a non-0 return code (e.g. command failed) | 20:22 |
rick_h | hml: but not sure | 20:22 |
rick_h | hml: sec call | 20:27 |
hml | rick_h: relation-get and relation-set will return a not found | 20:29 |
rick_h | hml: ok, then I guess let's do that | 20:29 |
hml | rick_h: agree, | 20:30 |
rick_h | hml: thanks for checking into that | 20:30 |
skay | how would you add your current juju model to the command prompt? I thought I had a way to do it, but it is not working | 21:43 |
hml | skay: what did you try? i’m not totally sure how myself :-). but i have a maybe not great idea | 21:48 |
skay | I tried PS1="(\$(juju models | grep \* |cut -d ' ' -f 1))$PS1" | 21:48 |
hml | skay: the output of `juju switch` also shows the current model. though you’d have to dump the controller and user pieces | 21:49 |
hml | it’s a one line output | 21:49 |
skay | oh, I thought that was just juju1 | 21:49 |
hml | skay: nope | 21:50 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!