[00:41] <timClicks> addyess, techalchemy: agreed - there's lots of useful refactoring in there
[00:51] <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
[01:12] <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:13] <wallyworld> there may be an old wiki page somewhere
[01:15] <timClicks> *websockets
[01:15] <timClicks> wallyworld: okay that's fine.. I'll put that down as another todo
[01:17] <wallyworld> one of many
[02:36] <wallyworld> hpidcock: adding a new operation tag for the actions work https://github.com/juju/names/pull/102
[02:37] <hpidcock> wallyworld: looking now
[02:37] <wallyworld> ta
[03:28] <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:29] <wallyworld> ta. i hope this works
[03:34] <wallyworld> and it did, dep ensure is happy now
[03:47] <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:48] <tlm> got time for a HO ?
[03:49] <wallyworld> tlm: it's not possible to use expectanytimes?
[03:49] <wallyworld> and /or expect after etc?
[03:50] <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:59] <tlm> wallyworld: might be user error, if you got 10 minutes would love to rubber duck it
[03:59] <wallyworld> ok
[04:36] <timClicks> signing heading off for the week
[04:36] <tlm> cya
[04:40] <hpidcock> oops, how did you go tlm?
[04:43] <tlm> good think we found a better path
[04:44] <hpidcock> the gift that keeps on giving
[06:50] <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:55] <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
[10:02] <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:04] <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:05] <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:06] <nammn_de> manadart: great,  sure no worries! Gimme a ping, when you find the chance for the spec talk
[10:41] <stickupkid_> achilleasa, you free ?
[10:41] <achilleasa> sure
[10:41] <achilleasa> daily?
[10:41] <stickupkid_> yeah
[11:09] <stickupkid_> damn it, I missed a PR
[11:09] <stickupkid_> :(
[11:14] <stickupkid_> it's the one where I re-wrote the firewall migration import as well
[11:15] <manadart> stickupkid_: Got a sec for HO?
[11:16] <stickupkid_> sure
[11:43] <achilleasa> manadart: quick HO?
[11:46] <manadart> achilleasa: OMW
[12:26] <manadart> Fix for the machine address space ID upgrade step: https://github.com/juju/juju/pull/11167
[12:35] <stickupkid_> well that back port went smoother now I added the missing PR
[12:43] <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:44]  * stickupkid_ pats self on back
[12:45] <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:46] <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:47] <achilleasa> we could of course throw in an adaptor if the core doesn't care about Done and only implements Build
[12:49] <manadart> achilleasa: Happy to take a look if you get a draft up.
[12:50] <achilleasa> manadart: not quite there yet but will try to get something up for comments
[12:55] <manadart> nammn_de: You want to chat before standup?
[12:55] <nammn_de> manadart: sure!
[13:23] <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:24] <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:25] <stickupkid_> achilleasa, can you not change how txn works?
[13:25]  * achilleasa runs away
[13:26] <rick_h> lol
[13:26] <achilleasa> will provide more info in standup. Maybe I am missing something
[13:34] <stickupkid_> achilleasa, RunTransactionObserver might be your friend
[14:11] <stickupkid_> https://paste.ubuntu.com/p/7V85FZTbrW/
[14:11] <stickupkid_> it did pass integration tests :)
[14:21] <rick_h> guild if anyone wants to have fun with the community check out https://github.com/juju/gomaasapi/pull/85
[14:25] <nammn_de> manadart: I took a quick look and qa on your pr. Added something I saw
[14:30] <hml> achilleasa:  RE:  11166, i’m on the fence for both points.  We can chat it out?
[14:31] <achilleasa> meet you in daily in 1m
[14:31] <hml> achilleasa:  omw
[14:39] <hml> rick_h: want to join our conversation. ^^
[14:44] <nammn_de> manadart: can a machine have an address in that space, but not a constraint?
[14:49] <manadart> Yes. But if it is not constrained, we still allow renaming.
[14:50] <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:52] <rick_h> hml:  sorry, was rebooting/etc still going?
[14:53] <rick_h> wheee waving from Description: Ubuntu Focal Fossa (development branch)
[14:56] <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:57] <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.
[15:11] <hml> rick_h:  we had to stop. can catch up now?
[15:11] <rick_h> hml:  omw
[15:35] <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:39] <hml> simple review for someone? https://github.com/juju/juju/pull/11169
[15:40] <hml> achilleasa:  i’ve udpated 11166 with concensus from rick_h , i believe your concerns are addressed as well.  pls let me know
[15:43] <achilleasa> hml: should I wait for the rename commit before reviewing?
[15:44] <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:46] <achilleasa> will do
[15:46] <hml> achilleasa:  ty
[15:53] <nammn_de> manadart: for 2 of your review points I am unsure what you mean. I added comments to them
[15:54] <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)
[16:12] <manadart> Need a review for the instance poller fix for space names: https://github.com/juju/juju/pull/11170
[16:14] <rick_h> guild if anyone has time please help ^ for release setup
[16:15] <achilleasa> manadart: looking
[16:17] <manadart> achilleasa: Actually doesn't change the instance poller :) I need to do it here, because bootstrap also retrieves instance addresses.
[16:18] <achilleasa> manadart: I think fixing it in the provider as you did is the best way to go
[16:38] <stickupkid_> achilleasa, I'd be happy in making txn more aware of failed txn.Ops
[16:39] <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:40] <stickupkid_> achilleasa, exactly my thinking
[16:41] <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:44] <achilleasa> nammn_de: looking in 5'
[16:47] <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:48] <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:49] <hml> achilleasa:  lol
[16:49] <hml> is it 5 oclock somewhere yet?
[16:49] <achilleasa> here, in 11m ;-)
[16:50] <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:51] <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:52] <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:56] <hml> achilleasa:  i wonder if the problem is not creating a new utils.v2?
[17:04] <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:05] <hml> achilleasa:  looking
[17:07] <hml> achilleasa:  why don’t i make these changes and the others before qa
[17:29] <stickupkid_> what's the Map?
[17:29] <stickupkid_> nammn_de,
[17:49] <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{}
[18:01] <achilleasa> stickupkid_: manadart this is my current approach to composing multiple ModelOperations together: https://paste.ubuntu.com/p/dbGNR3wh4w/
[18:02] <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:03] <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:04] <achilleasa> we could also simply return the error out
[18:05] <achilleasa> but this snippet works as expected if err == nil
[20:16] <hml> rick_h: do we have any precendent for hook tools returning not found
[20:18] <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:19] <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:22] <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:27] <rick_h> hml:  sec call
[20:29] <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:30] <hml> rick_h: agree,
[20:30] <rick_h> hml:  thanks for checking into that
[21:43] <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:48] <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:49] <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:50] <hml> skay:  nope