Tuesday, 2018-05-15

*** threestrands has quit IRC01:00
*** yamamoto has joined #openstack-oslo01:18
*** yamamoto has quit IRC01:25
*** namnh has joined #openstack-oslo01:35
*** dave-mccowan has joined #openstack-oslo01:41
*** yamamoto has joined #openstack-oslo01:41
*** yamamoto has quit IRC01:46
*** yamamoto has joined #openstack-oslo02:02
*** yamamoto has quit IRC02:06
*** yamamoto has joined #openstack-oslo02:23
*** yamamoto has quit IRC02:29
*** yamamoto has joined #openstack-oslo02:45
*** rcernin has quit IRC02:46
*** yamamoto has quit IRC02:50
*** rcernin has joined #openstack-oslo02:50
*** yamamoto has joined #openstack-oslo03:06
*** yamamoto has quit IRC03:10
*** nicolasbock has quit IRC03:11
*** yamamoto has joined #openstack-oslo03:27
*** yamamoto has quit IRC03:33
*** links has joined #openstack-oslo03:37
*** dave-mccowan has quit IRC03:38
openstackgerritVu Cong Tuan proposed openstack/taskflow master: Update Kazoo to correct URL  https://review.openstack.org/56718103:48
*** yamamoto has joined #openstack-oslo03:50
*** yamamoto has quit IRC03:54
*** gyankum has joined #openstack-oslo03:59
*** yamamoto has joined #openstack-oslo04:12
*** yamamoto has quit IRC04:17
*** namnh has quit IRC04:18
*** namnh has joined #openstack-oslo04:18
*** lpetrut has joined #openstack-oslo04:25
*** pcaruana has joined #openstack-oslo04:31
*** yamamoto has joined #openstack-oslo04:33
*** yamamoto has quit IRC04:33
*** yamamoto has joined #openstack-oslo04:35
*** linhnm has joined #openstack-oslo04:38
*** chhagarw has joined #openstack-oslo04:54
*** linhnm has quit IRC05:04
*** mikal has joined #openstack-oslo05:12
*** mikal_ has quit IRC05:15
*** linhnm has joined #openstack-oslo05:26
*** lpetrut has quit IRC05:50
*** linhnm has quit IRC05:56
*** snapiri has joined #openstack-oslo05:58
*** jbadiapa has quit IRC06:00
*** lpetrut has joined #openstack-oslo06:12
*** linhnm has joined #openstack-oslo06:22
*** lpetrut has quit IRC06:38
*** namnh has quit IRC07:01
*** namnh has joined #openstack-oslo07:01
*** rcernin has quit IRC07:13
*** tesseract has joined #openstack-oslo07:13
*** yamamoto_ has joined #openstack-oslo07:15
*** pcaruana has quit IRC07:17
*** pcaruana has joined #openstack-oslo07:17
*** pcaruana has quit IRC07:18
*** yamamoto has quit IRC07:18
*** lpetrut has joined #openstack-oslo07:22
*** pcaruana has joined #openstack-oslo07:23
*** lpetrut has quit IRC07:33
*** linhnm has quit IRC07:36
*** AlexeyAbashkin has joined #openstack-oslo07:36
*** kashyap has joined #openstack-oslo07:39
kashyapHi folks, wonder if anyone here know when was the oslo.config 'types' ("from oslo_config import types") was introduced?07:40
*** lucas-afk is now known as lucasagomes08:04
*** shardy has joined #openstack-oslo08:09
*** finucannot is now known as stephenfin08:21
*** e0ne has joined #openstack-oslo08:50
*** edmondsw has joined #openstack-oslo08:52
*** edmondsw has quit IRC08:56
*** linhnm has joined #openstack-oslo09:11
*** jbadiapa has joined #openstack-oslo09:15
*** e0ne has quit IRC09:29
*** e0ne has joined #openstack-oslo09:33
*** lpetrut has joined #openstack-oslo09:35
*** bhagyashri_s has joined #openstack-oslo09:37
*** e0ne has quit IRC09:38
*** pooja-jadhav has joined #openstack-oslo09:38
*** sambetts|afk is now known as sambetts09:40
*** bhagyashris has quit IRC09:40
*** pooja_jadhav has quit IRC09:41
*** linhnm has quit IRC09:42
*** e0ne has joined #openstack-oslo09:59
*** jaosorior has quit IRC10:00
*** jaosorior has joined #openstack-oslo10:01
*** namnh has quit IRC10:15
*** edmondsw has joined #openstack-oslo10:40
*** nicolasbock has joined #openstack-oslo10:40
*** tesseract is now known as info10:41
*** info is now known as tesseract10:41
*** edmondsw has quit IRC10:44
*** pbourke has quit IRC11:14
*** pbourke has joined #openstack-oslo11:14
*** edmondsw has joined #openstack-oslo11:21
openstackgerritMoisés Guimarães de Medeiros proposed openstack/oslo.config master: Create INI file ConfigurationSourceDriver.  https://review.openstack.org/56274611:26
*** ansmith has quit IRC11:26
*** raildo has joined #openstack-oslo12:01
*** pooja-jadhav is now known as pooja_jadhav12:17
*** gyankum has quit IRC12:25
*** ansmith has joined #openstack-oslo12:50
*** nicolasbock has quit IRC12:55
*** nicolasbock has joined #openstack-oslo13:01
*** kgiusti has joined #openstack-oslo13:02
*** dave-mccowan has joined #openstack-oslo13:02
*** rpittau has quit IRC13:02
*** eck` is now known as eck`gone13:08
openstackgerritRaildo Mascena proposed openstack/oslo-specs master: config: Protecting Plaintext Secrets  https://review.openstack.org/47430413:10
*** eck`gone is now known as eck`13:11
*** kashyap has left #openstack-oslo13:25
*** gyankum has joined #openstack-oslo13:28
*** janzian has joined #openstack-oslo13:32
*** pblaho has joined #openstack-oslo13:56
*** bobh has joined #openstack-oslo14:13
*** takedakn has joined #openstack-oslo14:19
openstackgerritMerged openstack/oslo-specs master: Trivial: update url to new url  https://review.openstack.org/56810814:37
*** spilla has joined #openstack-oslo14:47
lbragstado/ curious if anyone happens to have feedback on https://review.openstack.org/#/c/530509/14:52
*** takedakn has quit IRC14:58
*** takedakn has joined #openstack-oslo14:59
*** takedakn has quit IRC15:01
openstackgerritDoug Hellmann proposed openstack/oslo-specs master: fix pip install command  https://review.openstack.org/56861815:18
openstackgerritDoug Hellmann proposed openstack/oslo-specs master: add rocky specs to toctree  https://review.openstack.org/56861915:18
openstackgerritDoug Hellmann proposed openstack/oslo-specs master: remove redundant index pages  https://review.openstack.org/56862015:18
*** eck` is now known as eck`gone15:20
*** eck`gone is now known as eck`15:20
*** links has quit IRC15:21
dhellmannlbragstad : I promised you a review of that a few days ago :-(15:21
dhellmannI'll do it after this meeting15:21
dhellmannmoguimar : do those tests pass for you locally?15:27
moguimardhellmann, spilla, raildo: I'm planning to submit a talk to EuroPython about our work on this drivers, do you guys have any sugestion for a 30 min talk scope?15:28
moguimarwhen I run tox it runs the py35, py27 and flake8 only15:28
dhellmannmoguimar : do you want to talk about oslo.config in general? or just the drivers?15:28
moguimarhow can I test those locally?15:28
dhellmannI see 2 failures, lower-constraints and requirements-check15:28
dhellmannthe lower-constraints job you can test with "tox -e lower-constraints"15:29
dhellmannI'll propose an update to your patch to fix the requirements-check job15:29
moguimarthanks dhellmann15:30
moguimarI was thinking on a talk focused on the security aspect of not having sensitive data in plaintext15:30
openstackgerritDoug Hellmann proposed openstack/oslo.config master: Base class for a configuration driver  https://review.openstack.org/56002715:34
openstackgerritDoug Hellmann proposed openstack/oslo.config master: ConfigurationSource base class  https://review.openstack.org/55938915:34
openstackgerritDoug Hellmann proposed openstack/oslo.config master: Create INI file ConfigurationSourceDriver.  https://review.openstack.org/56274615:34
openstackgerritDoug Hellmann proposed openstack/oslo.config master: fix lower-constraints  https://review.openstack.org/56862215:34
dhellmannok, ^^ should fix the lower-constraints and I put it at the bottom of the stack so we can approve it before the other changes15:34
moguimarnice dhellmann, thanks15:37
lbragstaddhellmann: no worries :)15:39
*** snapiri has quit IRC15:43
dhellmannlbragstad : is there something (spec? bug?) that explains the different levels of scope?15:51
lbragstaddhellmann: there are - let me grab you a link15:52
dhellmannlbragstad : thanks, and if you have a few minutes now I have a few more background questions about this stuff15:53
dhellmannI can leave them as review comments, too, if you don't have time15:53
lbragstaddhellmann: this might help give you a high level view of the problem http://specs.openstack.org/openstack/keystone-specs/specs/keystone/queens/system-scope.html#problem-description15:53
lbragstadyou don't have to read the whole specification15:54
lbragstad(i attempted to condense it down into just that first paragraph)15:54
lbragstadwe do have the keystone meeting in about 5 minutes, but i'm available afterwords15:55
dhellmanncan you give me an example of a valid value for the system argument to that constructor?15:55
dhellmannlbragstad : ok15:55
lbragstaddhellmann: a valid value would be a string15:55
lbragstadand today we support a special string, which is 'all'15:56
dhellmannok, I think I need to read that spec because system='all' conveys no meaning at all to me right now15:56
dhellmannwhich means I don't know enough, but also "system" is not a sufficiently descriptive name for that argument15:57
lbragstadyeah - that was a downside of the implementation15:57
dhellmannalso we need an enum or const with that 'all' string15:57
lbragstadgood call15:57
dhellmanndoes some automatic part of the implementation dictate the naming convention?15:58
lbragstadsorry, which naming convention?15:59
lbragstadthe header?15:59
dhellmannwhy did you choose the name "system" for that argument to the constructor?15:59
dhellmannyeah, does something translate the header into "system" automatically?16:00
dhellmannoh, it looks like there's a mapping in here16:00
dhellmannso we could make that name better16:00
lbragstadyeah - it's supposed to map to a "system" scoped token16:00
lbragstadsimilar to how context handles project-scoped tokens16:01
dhellmannI'm talking about line 5216:01
dhellmanndo your meeting, I'm going to get lunch, we can talk more after both :-)16:01
lbragstadsounds like a plan16:01
*** lucasagomes is now known as lucas-afk16:09
*** tesseract has quit IRC16:21
*** jaosorior has quit IRC16:31
*** pcaruana has quit IRC16:33
*** e0ne has quit IRC16:35
openstackgerritNam Nguyen Hoai proposed openstack/oslo.config master: Handle config mapping changes  https://review.openstack.org/52631416:56
lbragstaddhellmann: done with our meeting17:04
dhellmanngood timing, I just finished lunch :-)17:04
lbragstadawesome17:05
dhellmannso I read the spec17:05
dhellmannlet me see if I can summarize it accurately17:05
dhellmannour existing role scopes all relate somehow to resources owned by projects17:05
dhellmannthis new scope relates to things that are outside of projects17:05
dhellmannso "system" in this sense means the underlying cloud system? (the example given is hypervisors)17:06
* lbragstad nods17:06
dhellmannand so I might have permission to look at hypervisors but not routers or other network gear, for example17:06
dhellmanndoes that cover it?17:06
lbragstadyep - pretty much17:06
dhellmannok17:06
lbragstadthe main crux of the issue is that openstack evolved APIs without evolving something to separate them17:07
dhellmannI think the variable should have a better name, but I now don't know what that is :-)17:07
lbragstads/APIs/system-level APIs/17:07
lbragstadyeah - it could have a better name, but i went with the existing convention (e.g. project_id)17:07
dhellmannyeah17:07
lbragstadideally - it might make sense to have something like project_scope, or system_scope17:08
dhellmannsystem_name isn't that much better17:08
dhellmannsystem_scope feels not quite right17:08
dhellmannis that value "scoping" the system? or is it a system name?17:09
dhellmannI'm still unclear on what the valid values might be17:09
dhellmannare they arbitrary? or defined somewhere?17:09
lbragstadso - it can be both17:09
dhellmannmaybe I have permissions to do things to hypervisors in one data center but not another?17:09
lbragstadright now - keystone supports getting a system-scoped token, which means you have authorization to operate on the entire deployment system17:09
dhellmannso the system value might be the data center name?17:09
lbragstadmore so a region + service pairing17:09
lbragstadbut we don't support that today17:10
lbragstadwhen we originally designed this, we thought that might be a powerful addition in the future17:10
dhellmannI see, so there's a plan for more levels but for now it's "all" or nothing17:10
lbragstade.g. i could give Alice the admin role on the compute service and Bob the auditor role on the storage service, and you could have the admin role on everything17:11
lbragstadyeah - right now it's all or nothing17:11
dhellmannok17:11
dhellmannfor the other things like project and domain we have both a name and an id, will we need that here?17:12
lbragstadno17:12
lbragstadthat should be specific to project or domain scoped tokens17:12
*** shardy has quit IRC17:12
dhellmannok, the http header name is HTTP_OPENSTACK_SYSTEM_SCOPE so maybe we can at least call the argument system_scope so it's obvious those are tied together17:12
lbragstad++17:13
dhellmannoh, I meant would we need both a name and an id for this scope17:13
lbragstadi think that is reasonable17:13
dhellmanndoes it need to be 2 variables, iow17:13
lbragstadoh...17:13
lbragstadno i don't think so?17:13
dhellmannlike, are these things someone is going to define in a database somewhere?17:13
dhellmannor are we going to have them hard-coded somehow?17:13
lbragstadthe only hardcoded value that carries special meaning would be 'all'17:13
lbragstadfuture work to scope to a specific service would likely incorporate a service_id17:14
dhellmannwill the others be site-specific conventions?17:14
dhellmannah17:14
dhellmannso a different 2nd variable17:14
lbragstadyeah17:14
lbragstadi guess so17:15
dhellmannwell, maybe not17:15
dhellmannmaybe it would system_scope='compute' ?17:15
lbragstadthat's kinda where we are in the discussion of it17:15
dhellmannor would it be system_scope='compute-in-iad-data-center'?17:15
lbragstadbecause regions play a part in that, too17:15
dhellmannyeah17:15
lbragstadexactly17:15
dhellmannok17:15
* lbragstad has a crazy idea17:15
*** chhagarw has quit IRC17:15
dhellmannit really feels like what we need is a second type for some of these things17:15
dhellmannlike instead of having 2 args, project_name and project_id, we should have had a ProjectID class that had those parameters17:16
dhellmannand maybe one or the other is optional or whatever17:16
dhellmannand then we'd have a SystemScopeDefinition or something and *that* could eventually grow other parameters to deal with the other cases17:16
lbragstadso - what if the "system" was composed of service entities (that keystone already has to know about) and the value of system_scope is always a node in that tree?17:17
dhellmannbut that's all off the top of my head and I'm sure it makes instantiating the context way harder17:17
dhellmannthat makes a lot of sense to me17:17
dhellmannhow does "all" fit into that?17:17
lbragstadi hate to bring up hierachical projects, but it would kinda be like that17:17
lbragstad'all' would be the root of the tree17:17
lbragstadevery service, every region, etc...17:18
lbragstadthen you could have services without regions that sit at the top of the tree17:18
lbragstad(e.g. keystone and swift make good example of services that serve multiple regions)17:19
lbragstadthen you can organize your regions, and services within those regions17:19
lbragstadso if you have compute services in ord, iad, and dfw17:19
lbragstadi could give someone the admin role on compute in iad, and they could get tokens scoped to that "target"17:20
dhellmannthat sounds logical17:20
lbragstadallowing them to do things there, but isolating their permissions to that region17:20
lbragstads/region/region+service/17:20
dhellmanncould you give them the same access in 2 disconnected regions?17:20
lbragstadyou could, but it would likely be two separate role assignments?17:21
lbragstade.g. i can give Alice admin on compute+iad and admin on compute+dfw17:21
dhellmannsure, that makes sense17:21
lbragstadbut she would be required to get different tokens17:21
lbragstad(much like getting different tokens for different projects you have authorization on)17:21
lbragstadeven though you may have the same role assignment on each17:22
dhellmannso does that turn "system_scope" into "service_id" instead?17:22
lbragstadyeah - it totally could be17:22
dhellmannor some variation of that, I guess -- I don't know what it's called in the keystone API17:22
dhellmannok, I'd rather get some consensus about that then and pick the name properly to start out17:22
dhellmannthat's going to be a little confusing with all of the service_* parameters, though17:23
dhellmannso maybe system_scope is still a good name, and we just say the value needs to be a service_id17:23
lbragstadso - system_scope could be `system_scope='all'` or `system_scope=$COMPUTE_IAD_SERVICE_ID`17:23
dhellmannright17:24
lbragstadnone of the service id stuff exists yet, but we're just trying to make sure we can build for that or add that later17:24
dhellmannsystem_scope is fine as a name in that case, and we just need to document what is expected as a value17:24
lbragstadok - that's reasonable17:24
dhellmannsure, that's fine. I'd just hate for us to realize in 3 months that this should all have been 4 different variables describing something other than what we're describing today17:25
dhellmannI mean, that can still happen, but I'd like to at least pretend we think we know what we're doing ;-)17:25
lbragstadright - i completely agree17:25
*** spilla has quit IRC17:25
*** AlexeyAbashkin has quit IRC17:25
dhellmannok, so I think my comments about naming still apply, and I had asked for some light docs about the parameter17:26
lbragstadthat was a big reason why we just decided to target 'all' initially and leave things open to build on in the future (once we figure out the semantics of how all that should work)17:26
dhellmannotherwise the patch looks OK17:26
lbragstadcool17:26
lbragstadi can get those addressed today17:26
dhellmannI completely support that approach. My goal in talking about it was to see what you were thinking *today* because I haven't really been focused on this at all17:26
dhellmannthanks for spending the time to bring me up to speed17:26
lbragstadanytime, thanks for the review and fresh perspective17:27
*** e0ne has joined #openstack-oslo17:39
dhellmannlbragstad: another question just popped into my mind:17:42
lbragstadsure17:42
dhellmannif the system scope is tied to the id of the service, do we need to pass it explicitly?17:42
dhellmannisn't the check just "what roles do they have for my service id? or 'all'?"17:43
dhellmannwhere "my" is from the perspective of the running service, which presumably knows its id?17:43
dhellmannit's not like project, where a user might log in to different projects, right?17:44
lbragstadright17:44
dhellmannso why does it need to be part of the context?17:44
dhellmannand why do we need an http header?17:44
dhellmannsorry, I'm probably just missing some fundamental assumption17:44
lbragstadthe http header gets populated by keystonemiddleware when it receives a token17:44
dhellmannhmm, ok17:45
lbragstadso - taking a step back17:45
dhellmannso I might get a token in one scope, and need a token in a different scope?17:45
lbragstadyeah - so the work in oslo.context tries to expose all scope information to oslo.policy (in a way)17:46
lbragstadit's very closely related to http://specs.openstack.org/openstack/oslo-specs/specs/queens/include-scope-in-policy.html17:46
dhellmannif I get a token scoped to compute-iad but then talk to compute-ord or something?17:46
dhellmannI guess I'm not sure why we make the user specify that when they login, vs. deriving what they can get at the point we need to do the check17:47
dhellmannmaybe it makes the policy check implementation easier17:47
lbragstadwhen we support that case, each service would need to understand it's own service id to be able to say "no, you may have the right role, but that doesn't match my service id"17:47
dhellmannthat part makes sense17:47
dhellmannthe bit that doesn't is why the user has to specify that scope up front in order to login to keystone17:47
lbragstadkinda of like trying to delete instances in a project you don't have the admin role on17:48
dhellmannor to get the token, I should say17:48
lbragstadsure17:48
dhellmannbecause if the token identifies me, and I can have many roles, shouldn't we be caring about those roles? not the scope?17:48
lbragstadtraditionally, we've always required users to only have one "scope" per token17:48
dhellmannI suppose that makes using the token on the receiving end simpler17:49
dhellmannat least inside the middlware and service17:49
lbragstaddo you have all your roles on the same project?17:49
dhellmannalthough as a user I might end up needing several tokens to do different things17:49
lbragstadvery true17:49
dhellmannI guess if we look at the service_id as a parallel to the project, then saying "get the token for the service_id you're trying to talk to" makes sense17:50
dhellmannbut if I need to talk to compute-iad and networking-iad then I need 2 tokens?17:50
dhellmannunless I have an all-iad token, I suppose17:50
lbragstadyes17:50
lbragstador if you have a role assignment on 'all'17:50
lbragstadthen you can do everything anywhere17:50
dhellmannwould I ever combine project and service ids? like projectA and compute-iad in one call then projectB and compute-iad in another?17:51
lbragstadideally, you wouldn't... but i also wouldn't be surprised if there are APIs somewhere in openstack where that would make sense17:51
dhellmannok17:52
lbragstadone of the interesting ones if force create in nova17:52
lbragstadwhere you have an API that allows users to force instances creation on a specific hypervisor17:52
lbragstadis that a system-level api or a project level api?17:52
dhellmannit feels a bit like we're creating another set of hierarchy and could collapse a bunch of the arguments to the context to avoid duplication of code, but maybe we need to keep them all separated17:52
dhellmannit sounds like a bad idea, however we classify it :-)17:53
lbragstadright - it's an odd thing17:53
dhellmannbut, ok, I see that as a case that calls for keeping project and service separate17:53
*** lpetrut has quit IRC17:54
*** sambetts is now known as sambetts|afk17:54
lbragstadit's kind of treads the line between usability and limiting damage control17:54
dhellmannheh, yeah17:54
lbragstadwhich has traditionally been the argument against allowing tokens scoped to multiple projects or targets17:55
dhellmannI think I get enough of this to see that the current proposal is a good step forward17:55
*** gyankum has quit IRC17:55
*** dave-mccowan has quit IRC17:55
dhellmannyeah, that's a good point17:55
openstackgerritLance Bragstad proposed openstack/oslo.context master: Implement system-scope  https://review.openstack.org/53050917:56
*** dave-mcc_ has joined #openstack-oslo17:56
*** links has joined #openstack-oslo18:00
*** spilla has joined #openstack-oslo18:02
eanderssonAre there any guidelines for adding schema changes to projects?18:05
*** e0ne has quit IRC18:20
dhellmanneandersson : database schema? or jsonschema?18:21
eanderssonSorry, database schema.18:21
dhellmannI thought that was probably what you meant, but wanted to make sure18:21
dhellmannI don't see anything in the oslo.db docs18:21
dhellmannhttps://docs.openstack.org/oslo.db/latest/18:21
dhellmannis your project using sqlalchemy-migrate or alembic?18:21
dhellmanneandersson : I think the best I'm going to be able to do is refer you to an implementation in an existing project18:23
dhellmannwhich is disappointing, because I thought we had docs on this18:24
dhellmannmaybe I just can't find them18:24
dhellmannzzzeek : maybe you know? ^^18:24
* zzzeek knows all18:24
dhellmannI'm not finding them in the nova contributor guide, either18:24
zzzeekeandersson: not official docs no, you need to look through the migrations given for indiviual projects and see how they are doing them18:25
zzzeekeandersson: best way to find out is to propose one and put it up for gerrit review18:25
eanderssonYea - that is kinda what I figured. I couldn't find any documentation, but was hoping that I just couldn't find it. :p18:25
* zzzeek concludes this moment's episode of zzzeek knows all18:25
* zzzeek does not know why his fedora 28 DL just crapped out18:26
zzzeeknor what is causing this eventlet issue18:26
eanderssonWish I had more time to write documentation :D18:29
*** lpetrut has joined #openstack-oslo18:34
*** bobh has quit IRC18:34
*** e0ne has joined #openstack-oslo18:35
*** mriedem has joined #openstack-oslo18:37
mriedemthe oslo.policy sphinx extension seems to either have a bug in it or i'm doing something wrong, based on building docs for this policy https://review.openstack.org/#/c/524425/11/nova/api/openstack/placement/policies/base.py - just two rules, but the rendered doc has one line uncommented: http://logs.openstack.org/25/524425/11/check/build-openstack-sphinx-docs/39855e8/html/configuration/sample-placement-policy.html18:39
mriedemit seems to be duplicating "placement": "role:admin"18:39
mriedemi guess it would be sphinxpolicygen in this case18:42
*** yamamoto_ has quit IRC18:46
bnemeclbragstad: ^might be relevant to your interests18:49
lbragstadmriedem: checking your placement patch quick18:50
bnemecI think this is the bug: https://github.com/openstack/oslo.policy/blob/master/oslo_policy/generator.py#L14018:50
bnemecMissing # on that line of the format string.18:50
bnemecHmm, the deprecated_rule one is missing comment characters too.18:51
lbragstadi think that is supposed to be uncommented?18:51
bnemecYeah, I'm a little confused why that DEPRECATED has a comment and the other doesn't.18:53
mriedemyeah looks like the spot i was just looking at, not sure why it would be uncommented though (intentionally(18:54
lbragstadi remember working with mgagne on some aliasing stuff, i can't remember it if was directly related to this18:54
lbragstadit certainly was related to deprecation things though18:54
lbragstadwe ultimately had to modify how the sample was rendered18:55
mriedemi seems like L140 shouldn't even exist18:55
mriedembecause the name:check_str are already in text18:55
lbragstadoh18:55
mriedemwell, see the TODO i have in my code https://review.openstack.org/#/c/524425/11/nova/api/openstack/placement/policies/base.py@28 - i probably should be using deprecated_rule here18:55
mriedembut i was having some issues getting that to work18:55
lbragstadi was just looking at that18:55
lbragstadwe have examples of it here - https://docs.openstack.org/oslo.policy/latest/reference/api/oslo_policy.policy.html#oslo_policy.policy.DeprecatedRule18:56
mriedemi know and i tried following that once,18:58
mriedembut it made the undeprecated alias look like it was actually deprecated,18:58
mriedemi'll see if i can reproduce what i was seeing18:58
mgagnethis isn't the case that we worked on. The case we worked on was about deprecating/replacing a rule name, not a check_str. And make the deprecated rule name an alias of the new rule name so you only have to update the check_str at one place.18:58
mgagneso app still parsing the old rule name (like Horizon) can still work until they are updated to read the new rule name.18:59
*** ansmith has quit IRC18:59
lbragstadmriedem: try this? http://paste.openstack.org/raw/721040/19:00
lbragstadmgagne: ah - yes.. you're right19:00
mriedemi think i might see the problem19:00
mriedemmy problem was i had both deprecated_for_removal and deprecated_rule set for my alias, but the sample gen handles those as mutually exclusive19:01
lbragstadyeah...19:02
lbragstadthere is some logic in the RuleDefault constructor for handling that, but i might have missed something19:02
mriedemyeah now i get this19:03
mriedem# DEPRECATED19:03
mriedem# "placement":"role:admin" has been deprecated since 18.0.0 in favor19:03
mriedem# of "admin_api":"role:admin".19:03
mriedem"placement": "rule:admin_api"19:03
mriedemi'm not sure why it's uncommented though19:03
lbragstadare you generating a sample?19:03
mgagneso it's an alias of admin_api19:04
*** lpetrut has quit IRC19:04
mgagneso one still using the "placement" rule can still rely on it until it's removed19:04
mriedemmgagne: right that's the point of the alias,19:04
mriedemi'm just wondering why the sample has the deprecated rule uncommented19:04
*** e0ne has quit IRC19:04
mriedemhere https://github.com/openstack/oslo.policy/blob/master/oslo_policy/generator.py#L16019:04
mgagneas an op, I want it to be uncommented so it actually works as an alias19:05
mriedemok but then admin_api (the rule) doesn't show up on its own in the sample - but i guess it is documented in the context of the deprecated rule, so maybe that's good enough?19:06
mgagnebut... I suppose that19:06
mgagneyou are now aliasing to something that doesn't exist19:06
mriedemright...19:06
mgagneI see your point19:06
mriedembecause admin_api isn't listed in the sample19:06
*** yamamoto has joined #openstack-oslo19:06
*** e0ne has joined #openstack-oslo19:07
mgagneI'm not sure how to fix it, maybe commenting it is the right thing to do19:07
mriedemthat's why i was using deprecated_for_removal before,19:07
mriedembut then i get this weird output http://logs.openstack.org/25/524425/11/check/build-openstack-sphinx-docs/39855e8/html/configuration/sample-placement-policy.html19:07
mriedemwere "placement": "role:admin" is uncommented, and #"admin_api": "role:admin" is commented19:07
mriedembut at least it's listed19:07
mriedem*where19:07
bnemecMaybe we need to include the original text in the one at line 159?19:08
mriedemdeprecated_reason isn't used in here either https://github.com/openstack/oslo.policy/blob/master/oslo_policy/generator.py#L14619:08
bnemecI think the deprecation text is overwriting the new policy text as it is.19:09
*** yamamoto has quit IRC19:12
mriedembnemec: so like this? http://paste.openstack.org/show/721041/19:12
mriedemthat would comment out the deprecated rule but also document the alias rule19:13
bnemecmriedem: Yeah, something like that.  Although I might put the original text first so the new rule comes before the deprecated rule.19:14
bnemecI'm not sure I'm familiar enough with how policy works to comment on how best to handle commenting/uncommenting the alias though.19:16
*** yamamoto has joined #openstack-oslo19:17
openstackgerritBen Nemec proposed openstack/oslo.policy master: Include both new and deprecated rules in generated sample  https://review.openstack.org/56867619:19
*** links has quit IRC19:20
bnemec^leaves it uncommented.  If we want to comment it we just need to add the appropriate #.19:20
mriedemi'll test that out locally quick19:20
bnemecYou can see the change in output in the unit test though.19:20
*** jaosorior has joined #openstack-oslo19:21
*** yamamoto has quit IRC19:22
*** janzian has quit IRC19:24
mriedemyeah this is what i get now19:25
mriedem# Default rule for most placement APIs.19:25
mriedem#"admin_api": "role:admin"19:25
mriedem# DEPRECATED19:25
mriedem# "placement":"role:admin" has been deprecated since 18.0.0 in favor19:25
mriedem# of "admin_api":"role:admin".19:25
mriedem"placement": "rule:admin_api"19:25
mriedemwhich is better imo19:25
bnemecCool.19:28
bnemecThe alias should work fine, right?  Even if the new rule isn't explicitly set, it still exists and has a default.19:28
bnemecExcept it's a little concerning that users would need to add the alias manually when upgrading. :-/19:29
bnemecThat's kind of the opposite of oslo.config deprecation where the deprecated alias is handled in code so existing configs continue to work during the deprecation period.19:30
mriedem"would need to add the alias manually when upgrading" - if the alias rule is policy in code, then it's not added manually, right?19:30
mriedemthe default would be in code19:31
mriedemso if your check string for the deprecated rule didn't match the default on the alias, then yeah you'd have to add it manually19:31
mriedembut isn't that true with config options also?19:31
bnemecIf it's already being handled in code, then yeah.19:33
bnemecI guess I got the impression it was uncommented in the sample file because it needed to be.19:33
bnemecBut if the code will handle the alias by default then we should comment out the alias in the sample file.19:34
*** yamamoto has joined #openstack-oslo19:39
*** yamamoto has quit IRC19:44
mriedemthat's what i was thinking - it seems weird to have the old rule uncommented in the file, but idk19:46
*** lpetrut has joined #openstack-oslo19:47
mriedemanyway, i think i got what i needed and have changed my code to use deprecated_rule, thanks bnemec lbragstad mgagne19:48
lbragstadmriedem: cool - let me know if you hit any other weird cases19:50
*** yamamoto has joined #openstack-oslo19:59
*** e0ne has quit IRC20:02
*** yamamoto has quit IRC20:04
openstackgerritBen Nemec proposed openstack/oslo.policy master: Include deprecated_reason when deprecated_rule is set  https://review.openstack.org/56868720:20
*** yamamoto has joined #openstack-oslo20:22
*** yamamoto has quit IRC20:26
*** e0ne has joined #openstack-oslo20:35
*** mriedem has left #openstack-oslo20:39
*** e0ne has quit IRC20:40
*** yamamoto has joined #openstack-oslo20:43
*** janzian has joined #openstack-oslo20:45
*** yamamoto has quit IRC20:47
*** raildo has quit IRC20:59
*** kgiusti has quit IRC21:00
*** yamamoto has joined #openstack-oslo21:04
*** yamamoto has quit IRC21:08
*** mriedem has joined #openstack-oslo21:21
mriedemhmm, so when i changed to use deprecated_rule, i'm getting errors now when using the deprecated "placement" policy because it's not registered, but its alias is21:21
mriedemhttp://logs.openstack.org/25/524425/12/check/openstack-tox-py27/82bf7ed/testr_results.html.gz21:22
mriedemi thought the point of the alias was so we could continue using the old thing and it wouldn't blow up?21:22
*** yamamoto has joined #openstack-oslo21:24
*** yamamoto has quit IRC21:29
mgagnemriedem: an alias to external consumer app which reads the file manually like Horizon. I don't think the app (placement) will actually register an alias.21:32
*** ansmith has joined #openstack-oslo21:37
mriedemok, i've changed back and just marked the "placement" rule as deprecated_for_removal21:40
mriedemwith the reason being, use rule:admin_api21:40
*** yamamoto has joined #openstack-oslo21:45
*** yamamoto has quit IRC21:50
*** edmondsw has quit IRC21:52
*** edmondsw has joined #openstack-oslo21:53
*** lpetrut has quit IRC21:54
*** mriedem is now known as mriedem_afk21:55
*** edmondsw has quit IRC21:57
*** rcernin has joined #openstack-oslo22:00
*** yamamoto has joined #openstack-oslo22:06
*** yamamoto has quit IRC22:10
*** yamamoto has joined #openstack-oslo22:27
*** yamamoto has quit IRC22:31
*** yamamoto has joined #openstack-oslo22:47
*** yamamoto has quit IRC22:52
*** yamamoto has joined #openstack-oslo23:08
*** yamamoto has quit IRC23:12
*** edmondsw has joined #openstack-oslo23:29
*** yamamoto has joined #openstack-oslo23:30
*** edmondsw has quit IRC23:34
*** yamamoto has quit IRC23:35
*** yamamoto has joined #openstack-oslo23:52
*** yamamoto has quit IRC23:56

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