| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-11 | |||
| 09:16:38 | openstackgerrit | Balazs Gibizer proposed openstack/nova-specs master: Remove force flag from live-migrate and evacuate https://review.openstack.org/609330 | |
| 09:30:25 | openstackgerrit | Merged openstack/python-novaclient master: doc: Start using openstackdoctheme's extlink extension https://review.openstack.org/608829 | |
| 10:01:52 | openstackgerrit | Ghanshyam Mann proposed openstack/nova master: Merge used_limits extension response into limit view builder https://review.openstack.org/606031 | |
| 10:46:25 | openstack | Launchpad bug 1743532 in python-novaclient "nova list doesn't work above api max_limit" [Undecided,Fix released] - Assigned to Jacek Tomasiak (skazi) | |
| 10:46:25 | s10 | Hello. Should be bug https://bugs.launchpad.net/python-novaclient/+bug/1743532 be reopened because fix was reverted in https://review.openstack.org/#/c/572539 ? | |
| 11:13:52 | sean-k-mooney | s10 from the message it looks like you could have passed --limie=-1 before so the change was not needed and intoduced a bug | |
| 11:17:07 | lyarwood | ~. | |
| 11:32:46 | openstackgerrit | Josephine Seifert proposed openstack/nova-specs master: Spec for the Nova part of Image Encryption https://review.openstack.org/608696 | |
| 12:10:48 | s10 | sean-k-mooney: most of novaclient users don't expect, that they should pass limit=-1 to get all nova instances. For example, behaviour of the minderclient is expected - it gets all cinder volume. But for nova we should pass limit=-1, and nobody does it. | |
| 12:11:05 | s10 | sean-k-mooney: examples are https://github.com/openstack/ceilometer/blob/master/ceilometer/nova_client.py#L146 and https://github.com/openstack/watcher/blob/master/watcher/common/nova_helper.py#L76 | |
| 12:11:37 | sean-k-mooney | s10: sure but the api limit is there to prevent excess load on that api and the operator does not expect that to be bypassed | |
| 12:11:51 | s10 | sean-k-mooney: ceilometer and watcher expects to get all nova instances here. If someone else uses novaclient python library, they also will face this issue | |
| 12:12:19 | sean-k-mooney | s10: ceilometer and watcher can easilly jsut pass -1 however | |
| 12:12:55 | s10 | sean-k-mooney: so basically we should open two bug in ceilometer and watcher and mark 1743532 as a feature, not a bug... | |
| 12:13:42 | s10 | sean-k-mooney: and should we open a bug against cinderclient? because it returns full list of volumes and don't honor api max_limit | |
| 12:14:29 | sean-k-mooney | on the cinder side proably unless its doing the paging internally. | |
| 12:14:55 | sean-k-mooney | so this is really a question of what are the expected semantics of the api and the client | |
| 12:15:35 | sean-k-mooney | i understand that users expect nova list to return all instance if invoked form the commandline | |
| 12:16:12 | sean-k-mooney | that is somwhat resonable but that could be supproted by defaulting to --limit=-1 | |
| 12:18:19 | sean-k-mooney | s10: to be clear i personally also expect nova list to return all instances but the previous fix was reverted because it actully intoduced a bug so if we want alter teh behavior again it shoudl likely be done as an RFE | |
| 12:22:21 | sean-k-mooney | s10: what might be the best thing to do is to reopen the old bug and flag it in the next nova meeting | |
| 12:28:50 | gmann | mriedem: added my comment on https://review.openstack.org/#/c/608474/ | |
| 12:29:16 | gmann | this is going to impact other APi also like PUT flavor, POST server, resize server etc. | |
| 12:30:50 | gmann | those i found till now but there might be other which does get flavor by id | |
| 12:36:31 | mriedem | gmann: i saw, thanks | |
| 12:36:34 | mriedem | left some replies | |
| 12:37:49 | sean-k-mooney | jaypipes: do the questions i just asked on https://review.openstack.org/#/c/609591/ make sense? | |
| 12:57:11 | jaypipes | sean-k-mooney: he mentions in the commit message that oslo.policy now handles that. | |
| 13:00:08 | jaypipes | sean-k-mooney: I think I see you point. commented. | |
| 13:01:10 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Reject forced move with nested source allocation https://review.openstack.org/605785 | |
| 13:03:16 | sean-k-mooney | yes i saw that. i had two point really. one i think they missed the removal of _warning_for_deprecated_user_based_rules and second ya i was not sure how usefual init was at this point | |
| 13:05:14 | sean-k-mooney | but as i also said in the review. i normally dont reviwe any of the policy stuff so im very open to being wrong on either point | |
| 13:09:01 | jaosorior | sean-k-mooney: hey! thanks for your review on my policy patch. | |
| 13:09:28 | sean-k-mooney | jaosorior: no worries did it make sense? | |
| 13:09:32 | jaosorior | sean-k-mooney: I'm now quite confused though. something should be caling the init() function, as that's what initializes the Enfocer object, which actually does the policy evaluation. | |
| 13:09:38 | jaosorior | * valling | |
| 13:09:43 | jaosorior | * calling | |
| 13:09:47 | jaosorior | damn, can't type today :D | |
| 13:10:51 | sean-k-mooney | well i was not actully suggesting removing init but i was about to comment that maybe you can change the module level global into a property that would do the init when its first used? | |
| 13:11:23 | jaosorior | that could be an option. But now I really wanna know where this gets initialized in the first place :D | |
| 13:12:13 | sean-k-mooney | jaosorior: haha well as i pointed out in the review thre are like 4 direct calles to init in the file so whatever gets called first i guess | |
| 13:12:24 | sean-k-mooney | arnt global values fun :P | |
| 13:14:14 | jaypipes | jaosorior: in oslo.policy, unless I'm mistaken... | |
| 13:14:29 | sean-k-mooney | jaosorior: if set_rules is the first call to init it passes use_conf=false in all other cases init is called with no argument so it used the default | |
| 13:14:56 | jaypipes | jaosorior: though, to be fair, the easiest way to determine if sean-k-mooney's right is just remove all calls to nova.policy.init() in the entire codebase and see if any tests break :) | |
| 13:15:14 | jaosorior | jaypipes: oslo.policy defines the Enforcer object that's being initialized there. It expects someone to actually initialize the object. Which is what happens in the init function. | |
| 13:16:09 | jaosorior | sean-k-mooney: I don't find calls to set_rules | |
| 13:16:33 | jaosorior | sean-k-mooney: must be authorize then | |
| 13:25:15 | Luzi | hey Nova, we've written the Spec for Image Encryption for Nova and would appreciate reviews :) https://review.openstack.org/#/c/608696/ | |
| 13:26:44 | openstackgerrit | Juan Antonio Osorio Robles proposed openstack/nova master: Remove saved_file_rules check from policy https://review.openstack.org/609591 | |
| 13:28:12 | openstackgerrit | Juan Antonio Osorio Robles proposed openstack/nova master: Remove dead code from policy module https://review.openstack.org/609591 | |
| 13:28:42 | jaosorior | sean-k-mooney: so... to keep it simple and not mess it up, I just left the init function. But removed the dead code from that module. Now it looks quite similar to what we have in barbican. | |
| 13:29:22 | jaosorior | ah, damn, we needed the get_enforcer function | |
| 13:31:17 | jaosorior | that is used by oslo.policy's generator (to generate sample policies and docs) | |
| 13:31:23 | openstackgerrit | Juan Antonio Osorio Robles proposed openstack/nova master: Remove dead code from policy module https://review.openstack.org/609591 | |
| 13:33:14 | sean-k-mooney | jaosorior: ok im going to stop tying to leave comments. once its working agin this all look better to me :) i left a comment on patchset 2 but i dont know you if you really want to refactor more then you already have | |
| 13:33:20 | openstackgerrit | Juan Antonio Osorio Robles proposed openstack/nova master: Remove dead code from policy module https://review.openstack.org/609591 | |
| 13:34:06 | jaosorior | sean-k-mooney: checking it out | |
| 13:36:36 | jaosorior | sean-k-mooney: I'll do the middle ground just to be safe | |
| 13:37:36 | sean-k-mooney | jaosorior: ya no worries removing _warning_for_deprecated_user_based_rules was the main thing i wanted you to do and you have so im happy with your change assuming zuul agrees | |
| 13:37:57 | jaosorior | sean-k-mooney: I'll push another patch in a bit | |
| 13:37:59 | jaosorior | running unit tests | |
| 13:38:11 | jaosorior | jaypipes, sean-k-mooney: Thanks for the reviews! | |
| 13:39:35 | openstackgerrit | Juan Antonio Osorio Robles proposed openstack/nova master: Remove dead code from policy module https://review.openstack.org/609591 | |
| 13:43:13 | sean-k-mooney | jaypipes: since your about care to take a look at https://review.openstack.org/#/c/584999/ when you have a chance. i know your busy so if you dont get to it its fine | |
| 13:53:35 | jaypipes | Luzi: done | |
| 13:54:04 | openstackgerrit | Jim Rollenhagen proposed openstack/nova-specs master: Use conductor groups to partition nova-compute services for Ironic https://review.openstack.org/609709 | |
| 13:54:16 | jroll | dansmith: TheJulia: ^ I think you were both interested in that | |
| 13:54:41 | dansmith | jroll: hmm, doesn't sound like me | |
| 13:54:58 | jroll | heh | |
| 13:55:07 | jroll | I could be wrong, of course :P | |
| 13:55:14 | dansmith | heh | |
| 13:59:04 | TheJulia | jroll: <3 | |
| 14:00:31 | jroll | :) | |
| 14:01:10 | jangutter | quick question for the room, what's the policy for reflowing text in .rst docs? I'm editing one where it looks like the column width was set at 60. | |
| 14:03:43 | efried | jangutter: You're going to piss off *somebody* | |
| 14:04:24 | efried | If you do it in the same change with other stuff, folks will complain that it's unrelated. If you do it in a separate patch, folks will complain that it's trivial and unnecessary. | |
| 14:04:42 | efried | But if I had to pick, I would do the latter. | |
| 14:05:24 | jaypipes | sean-k-mooney: done | |
| 14:16:50 | jangutter | efried: how to make friends and meet people using whitespace.... | |
| 14:17:22 | mriedem | do the latter | |
| 14:17:30 | mriedem | don't mix formatting with content changes | |
| 14:17:42 | mriedem | b/c it becomes a pain in the ass to review | |
| 14:17:59 | mriedem | GOD I'M MAD JUST THINKING ABOUT THIS?!?!?!? | |
| 14:18:02 | aspiers | efried: haha | |
| 14:18:10 | aspiers | mriedem: +1 | |
| 14:18:13 | artom | I think if you manage to make an outline of Batman's silhouette by varying the text width everyone will +2 immediately. | |
| 14:19:04 | aspiers | or you could embed subliminal messages in the whitespace via http://www.darkside.com.au/snow/ | |
| 14:22:09 | jangutter | I was more thinking of writing my life story with an acrostic, but those suggestions are good too. | |
| 14:23:23 | aspiers | X-D | |
| 14:43:08 | openstackgerrit | Jan Gutter proposed openstack/os-vif master: Reflow docs to 79 columns https://review.openstack.org/609726 | |
| 14:43:59 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Run negative server moving tests with nested RPs https://review.openstack.org/604125 | |
| 14:44:00 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Handle allocations consuming only from the child RPs https://review.openstack.org/608298 | |
| 14:45:00 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Handle allocations consuming only from the child RPs https://review.openstack.org/608298 | |
| 14:57:16 | jangutter | I think the only time formatting and whitespace changes are legit allowed to be mixed is in the IOCCC | |
| 14:57:25 | sean-k-mooney | jangutter: for the record i also hate that we enforce a 79 charater colum limit | |
| 14:58:06 | jangutter | sean-k-mooney: you prefer wider or narrower? | |
| 14:58:39 | sean-k-mooney | well one wider but too the limit was ment to be 80 charaters but someone decided to enforce 79 instead | |
| 14:59:14 | jangutter | sean-k-mooney: could be worse, could be Fortran. | |
| 15:00:03 | sean-k-mooney | frotrans standard is 132 i would be fine with that | |