| Posted | Nick | Remark | |
|---|---|---|---|
| #openstack-nova - 2018-10-11 | |||
| 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 | |
| 15:01:39 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Consider nested allocations during allocation cleanup https://review.openstack.org/606050 | |
| 15:01:39 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Reject forced move with nested source allocation https://review.openstack.org/605785 | |
| 15:01:40 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Run negative server moving tests with nested RPs https://review.openstack.org/604125 | |
| 15:01:40 | openstackgerrit | Balazs Gibizer proposed openstack/nova master: Handle allocations consuming only from the child RPs https://review.openstack.org/608298 | |
| 15:08:17 | openstackgerrit | Ivaylo Mitev proposed openstack/nova master: VMware: OVA and StrOpt images as VM templates https://review.openstack.org/609736 | |
| 15:30:42 | artom | So, I'm trying to write a new func tests that boots a server with a volume using the Cinder fixture, and I'm hitting 401 errors: http://paste.openstack.org/show/731904/ | |
| 15:31:09 | artom | As far as I can tell I'm going the same things as other existing tests | |
| 15:32:30 | openstackgerrit | Matt Riedemann proposed openstack/python-novaclient master: Add support for microversion 2.67: BDMv2 volume_type https://review.openstack.org/609743 | |
| 15:47:05 | mriedem | dansmith: huh, something must have regressed devstack's setup for nova-cpu.conf because it has db setup in it now... http://logs.openstack.org/87/604687/6/check/tempest-full/5e2a539/controller/logs/etc/nova/nova-cpu_conf.txt.gz | |
| 15:47:29 | dansmith | mriedem: hmm, I just installed a fresh devstack the other day and it didn't | |
| 15:47:46 | mriedem | i noticed this on a devstack i just created this morning | |
| 15:47:58 | dansmith | and pointing at cell0 | |
| 15:48:51 | mriedem | right - not that nova-compute should be trying to hit a db either way, but those shouldn't be in the cpu config | |
| 15:49:00 | dansmith | maybe melwitt's console patch? | |
| 15:49:05 | dansmith | definitely | |
| 15:49:25 | mriedem | https://github.com/openstack-dev/devstack/blob/master/lib/nova#L918 ? | |
| 15:49:49 | mriedem | cell1 conf looks good http://logs.openstack.org/87/604687/6/check/tempest-full/5e2a539/controller/logs/etc/nova/nova_cell1_conf.txt.gz | |
| 15:50:27 | dansmith | that line is old | |
| 15:50:33 | mriedem | yeah, | |
| 15:50:39 | mriedem | it's also wrong in nova-cpu.conf on queens http://logs.openstack.org/55/606155/1/check/tempest-full/dcd0005/controller/logs/etc/nova/nova-cpu_conf.txt.gz | |
| 15:50:45 | mriedem | so it's not mel's console config change | |
| 15:50:49 | mriedem | maybe just always busted? | |
| 15:51:03 | dansmith | mm, I dunno | |
| 15:51:06 | mriedem | looks like that function is copying nova-cpu.conf from $NOVA_CONF which is nova.conf | |
| 15:51:09 | mriedem | which is the controller config | |
| 15:51:46 | mriedem | anywho, not a huge deal since we don't have local conductor | |
| 15:51:50 | mriedem | i was just confused | |
| 15:51:58 | mriedem | b/c i was able to sync the cell db using nova-cpu.conf | |
| 15:52:00 | mriedem | which shouldn't work | |
| 15:52:36 | dansmith | definitely wrong though.. I can't imagine that's been broken like that for so long | |