/srv/irclogs.ubuntu.com/2020/01/31/#juju.txt

timClicksaddyess, techalchemy: agreed - there's lots of useful refactoring in there00:41
techalchemytimClicks, most significatly adding compatibility is important, I'm installing a fork so i can use python 3 which is a bit annoying00:51
timClickswhelp00:51
timClicksis juju's over-the-wire communication mechanism, e.g. server-side sockets, documented anywhere publicly?01:12
wallyworldnot that i now of01:12
wallyworldthere may be an old wiki page somewhere01:13
timClicks*websockets01:15
timClickswallyworld: okay that's fine.. I'll put that down as another todo01:15
wallyworldone of many01:17
wallyworldhpidcock: adding a new operation tag for the actions work https://github.com/juju/names/pull/10202:36
hpidcockwallyworld: looking now02:37
wallyworldta02:37
wallyworldhpidcock: i hate go dep, bring on go mod https://github.com/juju/names/pull/10303:28
hpidcockone of us one of us one of us!03:28
wallyworldta. i hope this works03:29
wallyworldand it did, dep ensure is happy now03:34
tlmhpidcock: I think we may need to change this watcher design a little, the go routine has broken these tests because they wants such strict ordering03:47
tlmgot time for a HO ?03:48
wallyworldtlm: it's not possible to use expectanytimes?03:49
wallyworldand /or expect after etc?03:49
wallyworldcan jump in standup if you want03:50
tlmjust looking at this now. Can  you give me 20, will try and get a better example of the problem I have  hit03:50
wallyworldok03:50
tlmwallyworld: might be user error, if you got 10 minutes would love to rubber duck it03:59
wallyworldok03:59
timClickssigning heading off for the week04:36
tlmcya04:36
hpidcockoops, how did you go tlm?04:40
tlmgood think we found a better path04:43
hpidcockthe gift that keeps on giving04:44
wallyworldtlm: 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 monday06:50
tlmwallyworld: yeah thats fine06:55
tlmPR will be in my monday06:55
tlmjust a bit of boilerplate to mock all this in06:55
=== parlos is now known as parlos_afk
=== parlos_afk is now known as parlos
=== parlos is now known as parlos_afk
nammn_demanadart: added the changes and phase to skip the model if its not controllermodel for settingsdelta.10:02
nammn_dein 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_dehttps://github.com/juju/juju/pull/1114310:02
manadartnammn_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_demanadart: i also updated the qa steps in the pr to match the change10:04
manadartnammn_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_demanadart: great,  sure no worries! Gimme a ping, when you find the chance for the spec talk10:06
=== parlos_afk is now known as parlos
stickupkid_achilleasa, you free ?10:41
achilleasasure10:41
achilleasadaily?10:41
stickupkid_yeah10:41
stickupkid_damn it, I missed a PR11:09
stickupkid_:(11:09
stickupkid_it's the one where I re-wrote the firewall migration import as well11:14
=== parlos is now known as parlos_afk
manadartstickupkid_: Got a sec for HO?11:15
stickupkid_sure11:16
=== parlos_afk is now known as parlos
=== parlos is now known as parlos_afk
achilleasamanadart: quick HO?11:43
manadartachilleasa: OMW11:46
=== parlos_afk is now known as parlos
manadartFix for the machine address space ID upgrade step: https://github.com/juju/juju/pull/1116712:26
stickupkid_well that back port went smoother now I added the missing PR12: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 back12:44
achilleasamanadart: 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 operation12:45
achilleasathere are parts that have side-effects (e.g. update doc fields once the txn completes)12:46
achilleasas/doc fields/model fields/12:46
achilleasawe could of course throw in an adaptor if the core doesn't care about Done and only implements Build12:47
manadartachilleasa: Happy to take a look if you get a draft up.12:49
achilleasamanadart: not quite there yet but will try to get something up for comments12:50
manadartnammn_de: You want to chat before standup?12:55
nammn_demanadart: sure!12:55
=== parlos is now known as parlos_afk
=== parlos_afk is now known as parlos
achilleasamanadart: 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 ops13:23
achilleasacomposing 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 method13:24
stickupkid_achilleasa, can you not change how txn works?13:25
=== parlos is now known as parlos_afk
* achilleasa runs away13:25
rick_hlol13:26
achilleasawill provide more info in standup. Maybe I am missing something13:26
=== parlos_afk is now known as parlos
stickupkid_achilleasa, RunTransactionObserver might be your friend13:34
stickupkid_https://paste.ubuntu.com/p/7V85FZTbrW/14:11
stickupkid_it did pass integration tests :)14:11
rick_hguild if anyone wants to have fun with the community check out https://github.com/juju/gomaasapi/pull/8514:21
nammn_demanadart: I took a quick look and qa on your pr. Added something I saw14:25
hmlachilleasa:  RE:  11166, i’m on the fence for both points.  We can chat it out?14:30
achilleasameet you in daily in 1m14:31
hmlachilleasa:  omw14:31
hmlrick_h: want to join our conversation. ^^14:39
nammn_demanadart: can a machine have an address in that space, but not a constraint?14:44
manadartYes. But if it is not constrained, we still allow renaming.14:49
nammn_demanadart: makes sense, thanks14:50
manadartnammn_de: Added another review to you patch.14:50
manadartnammn_de: Also, a suggested simplification for you op/factory: https://pastebin.canonical.com/p/4vgXNZYQVR/14:50
rick_hhml:  sorry, was rebooting/etc still going?14:52
rick_hwheee waving from Description: Ubuntu Focal Fossa (development branch)14:53
manadartnammn_de: Thanks for the review. Can you or stickupkid_ approve it?14:56
rick_hlooks like folks are done. hml let me know if you want to catch up14:56
stickupkid_manadart, i didn't do q&a... nammn_de can you approve once you've done it14:57
nammn_dedone14:57
manadartTa.14:57
hmlrick_h:  we had to stop. can catch up now?15:11
rick_hhml:  omw15:11
nammn_demanadart: 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 the15: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
hmlsimple review for someone? https://github.com/juju/juju/pull/1116915:39
hmlachilleasa:  i’ve udpated 11166 with concensus from rick_h , i believe your concerns are addressed as well.  pls let me know15:40
achilleasahml: should I wait for the rename commit before reviewing?15:43
hmlachilleasa:  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 change15:44
achilleasawill do15:46
hmlachilleasa:  ty15:46
nammn_demanadart: for 2 of your review points I am unsure what you mean. I added comments to them15:53
achilleasastickupkid_: 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
manadartNeed a review for the instance poller fix for space names: https://github.com/juju/juju/pull/1117016:12
rick_hguild if anyone has time please help ^ for release setup16:14
achilleasamanadart: looking16:15
manadartachilleasa: Actually doesn't change the instance poller :) I need to do it here, because bootstrap also retrieves instance addresses.16:17
achilleasamanadart: I think fixing it in the provider as you did is the best way to go16:18
stickupkid_achilleasa, I'd be happy in making txn more aware of failed txn.Ops16:38
stickupkid_achilleasa, just need to ensure not to leak info16:39
achilleasastickupkid_: 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 thinking16:40
nammn_deachilleasa: 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_r37347722116:41
achilleasastickupkid_: 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 out16:41
achilleasanammn_de: looking in 5'16:44
hmlachilleasa:  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 charm16:47
achilleasahml: workaround is to remove Tags from utils/set16:48
achilleasawe don't seem to be using it in juju16:48
nammn_destickupkid_:  is it possible from a test to read the content of a txn.Op? Without needing to apply it?16:48
achilleasahml: fifth time's a charm? :D16:48
hmlachilleasa:  lol16:49
hmlis it 5 oclock somewhere yet?16:49
achilleasahere, in 11m ;-)16:49
hmlha!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_destickupkid_: yeah, right. I'am writing a test, which returns Ops. Something like a dry run would be great16:50
nammn_detxn.Op saves my change in an Update interface. Which I probably need to parse back. Just thought we alreadyt had something in place for that16:51
nammn_delike the dry run16: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_destickupkid_: thanks! Probably what I had in mind16:52
hmlachilleasa:  i wonder if the problem is not creating a new utils.v2?16:56
nammn_destickupkid_: lol  ops[0].Update.(bson.D).Map()["$set"].(state.ConstraintsDoc).Spaces17:04
achilleasahml: left some comments in 11166; do you want me to defer QA till your changes land?17:04
hmlachilleasa:  looking17:05
hmlachilleasa:  why don’t i make these changes and the others before qa17:07
stickupkid_what's the Map?17:29
stickupkid_nammn_de,17:29
nammn_destickupkid_: 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
achilleasastickupkid_: 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 completely18:02
stickupkid_?18:03
achilleasastickupkid_: ideally we would need to de-dup (most Done() impls will prob just return the error straight out) and pack in a multi-error18:03
achilleasaI will try to come up with something bettern on Monday18:03
achilleasawe could also simply return the error out18:04
achilleasabut this snippet works as expected if err == nil18:05
hmlrick_h: do we have any precendent for hook tools returning not found20:16
rick_hhml:  thinking, like what example?20:18
hmlrick_h: since we’re going with state-set can set an empty string and will not delete the key.20:18
hmlrick_h: the follow on to me is that state-get will return not found for keys not found.20:19
hmlrick_h: o.w. how to distinguish between not set (found) and empty string20:19
rick_hhml:  hmm, at first I was thinking a non-0 return code (e.g. command failed)20:22
rick_hhml:  but not sure20:22
rick_hhml:  sec call20:27
hmlrick_h: relation-get and relation-set will return a not found20:29
rick_hhml:  ok, then I guess let's do that20:29
hmlrick_h: agree,20:30
rick_hhml:  thanks for checking into that20:30
skayhow would you add your current juju model to the command prompt? I thought I had a way to do it, but it is not working21:43
hmlskay:  what did you try?  i’m not totally sure how myself :-). but i have a maybe not great idea21:48
skayI tried PS1="(\$(juju models | grep \* |cut -d ' ' -f 1))$PS1"21:48
hmlskay: the output of `juju switch` also shows the current model.  though you’d have to dump the controller and user pieces21:49
hmlit’s a one line output21:49
skayoh, I thought that was just juju121:49
hmlskay:  nope21:50

Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!