| hpidcock | forward port from 2.7 https://github.com/juju/juju/pull/10910 | 00:14 |
|---|---|---|
| pmatulis | on 2.6.10 when i do 'juju show-cloud maas' i believe this should apply to the cloud info on the controller correct? | 01:33 |
| pmatulis | yet the first line of the output is: | 01:34 |
| pmatulis | 'defined: local' | 01:34 |
| anastasiamac | on 2.6.10, shoe-cloud command only showed local clouds | 01:36 |
| anastasiamac | the ability to see controller clouds was added in 2.7 | 01:37 |
| anastasiamac | show* | 01:37 |
| pmatulis | anastasiamac, k, the --local option is confusing me | 01:39 |
| pmatulis | in the help i mean | 01:40 |
| anastasiamac | pmatulis: that's why on 2.7 we tried to deprecate it | 01:40 |
| pmatulis | maybe update the help then. it's quite confusing | 01:40 |
| pmatulis | no biggie though | 01:41 |
| anastasiamac | m not sure whether we r planning another 2.6 release... it may only b critical issues that will b addressed | 01:41 |
| nammn_de | manadart: as we were talking about this yesterday https://github.com/juju/juju/pull/10907 What do you think about adding the information to the modelcache? IMO this should be a safe operation, though I could just use the string "admin/controller" and remove the rest of the code. | 12:00 |
| manadart | nammn_de: Do you mean adding "IsController" the the cached Model type? | 12:09 |
| nammn_de | manadart: yes, because initially I was not sure whether we can alwaysd be sure that "admin/controller" will be the controller model. | 12:09 |
| nammn_de | So I thought there is nothing to lose to add that information | 12:09 |
| nammn_de | but maybe it will never change and the change itself is not needed. Not even in the future 🤠| 12:10 |
| manadart | nammn_de: For your purposes, I think using admin/controller is the shortest path, but I have no problem with syncing the rest of the multi-watcher model fields into the cache type - we'll probably need that eventually. | 12:12 |
| nammn_de | manadart: thanks! And for my pr above? The current state works, which already adds the isController information. But I could revert the change and "just" add the admin/controller check | 12:13 |
| nammn_de | Happy for a short pr review. I will fix the tests depending on the review output by then | 12:13 |
| nammn_de | *s/by then// | 12:13 |
| manadart | nammn_de: I say revert the local store changes. If we can get away without adding complexity there... | 12:14 |
| nammn_de | manadart: will do :) | 12:14 |
| nammn_de | manadart: change is in https://github.com/juju/juju/pull/10907 small one | 12:22 |
| nammn_de | manadart thanks! I just amended and added the change you mentioned. "luckily" the modelupgrade command, if no controllername is given, takes the controllername from the cache. Thats why I did not realise it during testing. | 12:42 |
| nammn_de | manadart: is there a easy way to add a user to the controller in the tests? | 15:15 |
| nammn_de | and then add them to the accountdetails? | 15:15 |
| manadart | nammn_de: Do you mean unit tests for 10907? | 15:28 |
| nammn_de | manadart: yes, I tried to create a user, add it to the model and change the current user to the one im testing with. Everything is working beside that he says that my current user does not have enough permission. Thats what I did | 15:30 |
| nammn_de | https://github.com/juju/juju/pull/10907/files#diff-eb405c3d636e6c7c21e2d460f92ae522R193 | 15:30 |
| nammn_de | tbh, I just want to run one of those 5 tests to see whether it runs through | 15:30 |
| nammn_de | because else it fails because of missing permissions. Which is not bad, because this is independent of my test. But the best test would be everything else is just silently mocked out and it can run through. I dont need the full suite for my change | 15:31 |
| manadart | nammn_de: I think you need to change the suite's authoriser, which looks like it only auth's the admin user by default. | 15:44 |
| nammn_de | manadart: tried, did not work :/ | 15:58 |
| nammn_de | manadart I finally got it to run :D. manadart rick_h one of you want to give a quick review? https://github.com/juju/juju/pull/10907/files | 17:33 |
| rick_h | manadart: nammn_de we're making sure to now hard code "controller" as the expected name anywhere right? | 19:07 |
| === oliveiradan is now known as doliveira | ||
| === doliveira is now known as oliveiradan_ | ||
| jam | axino: https://jenkins.juju.canonical.com/job/BuildJuju-amd64/2700/ | 23:53 |
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!