)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f9500e1ab29db3f00e2a1a162140cd4ad52686e5","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"api: Keep track of action controllers"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We need to be able to resolve the original, unversioned methods."},{"line_number":10,"context_line":"Register these things slightly differently. It would likely be better to"},{"line_number":11,"context_line":"fold these action controllers into the main controllers, but that\u0027s a"},{"line_number":12,"context_line":"lot of code motion that I don\u0027t really want to do right now."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7790a7e4_ebede8cd","line":9,"updated":"2024-04-19 10:24:58.000000000","message":"Note for reviewers: we need this because I need access to more attributes of these sub controllers than just the `wsgi_actions` attribute. Specifically, I currently need access to the `versioned_methods` attribute so that I\u0027m able to look up the original function rather than the `version_select` wrapper function we use. You can see that lookup taking place later in the series [here](https://review.opendev.org/c/openstack/nova/+/915735/3/nova/tests/unit/api/openstack/compute/test_schemas.py#72).","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"092b9073eb532003b0acfdd6b20bca6851b5ae61","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"api: Keep track of action controllers"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We need to be able to resolve the original, unversioned methods."},{"line_number":10,"context_line":"Register these things slightly differently. It would likely be better to"},{"line_number":11,"context_line":"fold these action controllers into the main controllers, but that\u0027s a"},{"line_number":12,"context_line":"lot of code motion that I don\u0027t really want to do right now."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"ceb8c7a3_5019abf3","line":9,"in_reply_to":"7790a7e4_ebede8cd","updated":"2024-05-02 22:22:00.000000000","message":"this is what I was searhcing for, where you are going to use those subcontroller ref and what info. but seeing the test you mentioned I got it now. thanks for the note","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"092b9073eb532003b0acfdd6b20bca6851b5ae61","unresolved":true,"context_lines":[{"line_number":7,"context_line":"api: Keep track of action controllers"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"We need to be able to resolve the original, unversioned methods."},{"line_number":10,"context_line":"Register these things slightly differently. It would likely be better to"},{"line_number":11,"context_line":"fold these action controllers into the main controllers, but that\u0027s a"},{"line_number":12,"context_line":"lot of code motion that I don\u0027t really want to do right now."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: Iee37500e6b2dbacf0c1514bfc52ef2dfe8ceb94f"},{"line_number":15,"context_line":"Signed-off-by: Stephen Finucane \u003cstephenfin@redhat.com\u003e"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"751a5256_e017947b","line":12,"range":{"start_line":10,"start_character":44,"end_line":12,"end_character":60},"updated":"2024-05-02 22:22:00.000000000","message":"I am not sure that will give us any benefits than just having all things in single module to ref or fetch data from. Even if we merge these action controllers into the main controller, we still need the \u0027complex\u0027, \u0027hard to understand/maintain/test\u0027 magic of mapping those 1. actions 2. versioned methods in wsgi layer. I remember that I thought/discussed about it during extensions merge work and due to same (wsgi complexity remains same) reason, we did not merge those.\n\nThe only thing that could make Nova API simple, is if we remove the action APIs and provide them in a direct RESTful method way. but it changes the API so hard to do that (or hopefully some super-microversion can do it :))","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"}],"/PATCHSET_LEVEL":[{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"297b970ec0672b2b64691726041290dbefea50bd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3276b69e_04969df4","updated":"2024-05-02 22:51:02.000000000","message":"No issues from me, I\u0027ll add the +W, thanks for looking at this gmann 😊","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":4690,"name":"melanie witt","display_name":"melwitt","email":"melwittt@gmail.com","username":"melwitt"},"change_message_id":"fd61fd3e783f3ede651e3f235606ab1b9b71c646","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ae8b9225_437abac9","updated":"2024-04-30 20:48:41.000000000","message":"Unfortunately I can\u0027t really figure out what\u0027s going on here despite the commit message and reviewer note 😆 so I\u0027ve asked gmann if he can look","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"643999b05357d1810380439de192e2efb7f1707c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"f7fcee4a_0a0a1147","updated":"2024-04-15 10:53:27.000000000","message":"recheck unrelated grenade timeout","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"cc0c977083cf217f83432985010a74e6093df2d9","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"d025058f_1c24a8a8","in_reply_to":"9f25e9a0_f3d6f3ff","updated":"2024-05-01 16:14:53.000000000","message":"stephen please put this into a blog post our our contibutors docs in some form.\n\nthis too useful of an explaination to live only in a comment","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"e454cd62cf351c93b5ca1a7128453a249ab5aabc","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"9f25e9a0_f3d6f3ff","in_reply_to":"ae8b9225_437abac9","updated":"2024-05-01 12:53:16.000000000","message":"Fair 😄 Let me try again, one more time. First a bit of background. `APIRouterV21` provides mappings of URLs (or rather, URL paths) to methods of `Resource` objects. A `Resource` object is a wrapper around some `Controller` objects: a main controller and zero or more sub controllers. If you look at `nova/api/openstack/compute/routes.py` you\u0027ll see a whole load of `functools.partial` calls where we create the `Resource` objects via a calls to `_create_controller`:\n\n```\nflavor_controller \u003d functools.partial(_create_controller,\n    flavors.FlavorsController,\n    [\n        flavor_access.FlavorActionController\n    ]\n)\n```\n\nHere, `flavors.FlavorsController` is the \"main\" controller and `flavor_access.FlavorActionController` is a \"sub\" (or \"action\"/\"child\") controller. The sub controller extends the main controller by adding new APIs or actions and to the best of my knowledge it is legacy from the days where we had API extensions. \n\nNow with that knowledge, run this script locally.\n\n```\nfrom nova.api.openstack import compute\nfrom oslo_config import cfg\nfrom nova.tests import fixtures\n\nCONF \u003d cfg.CONF\nfixtures.ConfFixture(CONF).setUp()\nfixtures.RPCFixture(\u0027nova.test\u0027).setUp()\n\nrouter \u003d compute.APIRouterV21()\n\ncount \u003d 0\nfor route in router.map.matchlist:\n    if \u0027controller\u0027 not in route.defaults:\n        continue\n    controller \u003d route.defaults[\u0027controller\u0027]\n    if controller.wsgi_actions \u003d\u003d {}:\n        continue\n    for action, method in controller.wsgi_actions.items():\n        print(f\u0027  action: {action}, method: {method}\u0027)\n        if \u0027version_select\u0027 in str(method):\n            count +\u003d 1\n\nprint(f\u0027useless versioned method count: {count}\u0027)\n```\n\nAll that this is doing is configuring enough config-related fixtures to allow us to create an `APIRouterV21` instance so that we can iterate through the path-controller mappings it has. If you run this, you\u0027ll see a whole load of output finishing in this:\n\n```\nuseless versioned method count: 288\n```\n\nThis is a count of the number of actions that resolve to `version_select` methods. These are not the \"real\" method and are instead wrappers around the real methods (potentially plural, depending on amount of microversioned revisions) that allow us to handle API versioning. This wrapper methods are useless to us because we need to get attributes of the real methods - namely, the schema-related attributes we start storing [later in this series](https://review.opendev.org/c/openstack/nova/+/915735/3/nova/api/validation/__init__.py#112). The way to find the \"real\" method is to look at the `versioned_methods` attribute of a `Controller`, which contains a mapping of method name to real methods. If you change the above for loop you can see this:\n\n```\ncount \u003d 0\nfor route in router.map.matchlist:\n    if \u0027controller\u0027 not in route.defaults:\n        continue\n    controller \u003d route.defaults[\u0027controller\u0027]\n    if controller.wsgi_actions \u003d\u003d {}:\n        continue\n    for action, method in controller.wsgi_actions.items():\n        method_name \u003d controller.controller.wsgi_actions.get(action)\n        if method_name:\n            versioned_methods \u003d getattr(\n                controller.controller, \u0027versioned_methods\u0027, {}\n            ).get(method_name)\n            if versioned_methods:\n                method \u003d versioned_methods[0].func\n        print(f\u0027  action: {action}, method: {method}\u0027)\n        if \u0027version_select\u0027 in str(method):\n            count +\u003d 1\n\nprint(f\u0027useless versioned method count: {count}\u0027)\n```\n\nNow if you run this, you\u0027ll get a reduced count:\n\n```\nuseless versioned method count: 224\n```\n\nHowever, it\u0027s still not `0`. This is because we\u0027re not able to resolve to all \"real\" methods using only the \"main\" controller alone. To do that, we need the `versioned_methods` attribute of the sub controllers also, or to use the case of the controllers we gave at the start, the `versioned_methods` attribute of both the `FlavorsController` controller *and* the `FlavorActionController` controller. However, we currently have no reference to `FlavorActionController` (or any other sub controller) so we can\u0027t do this.\n\nThis patch corrects this by storing the reference to `FlavorActionController` (and any other sub controller) under `Resource.sub_controllers`, thus giving us a mechanism to retrieve `versioned_methods` attributes (and any other attribute we might need later) from these sub-controller. With this patch applied, we can change the for loop further:\n\n```\ncount \u003d 0\nfor route in router.map.matchlist:\n    if \u0027controller\u0027 not in route.defaults:\n        continue\n    controller \u003d route.defaults[\u0027controller\u0027]\n    if controller.wsgi_actions \u003d\u003d {}:\n        continue\n    for action, method in controller.wsgi_actions.items():\n        main_controller \u003d controller.controller\n        method_name \u003d main_controller.wsgi_actions.get(action)\n        if method_name:\n            versioned_methods \u003d getattr(\n                main_controller, \u0027versioned_methods\u0027, {}\n            ).get(method_name)\n            if versioned_methods:\n                method \u003d versioned_methods[0].func\n        for sub_controller in controller.sub_controllers:\n            method_name \u003d sub_controller.wsgi_actions.get(action)\n            if method_name:\n                versioned_methods \u003d getattr(\n                    sub_controller, \u0027versioned_methods\u0027, {}\n                ).get(method_name)\n                if versioned_methods:\n                    method \u003d versioned_methods[0].func\n        print(f\u0027  action: {action}, method: {method}\u0027)\n        if \u0027version_select\u0027 in str(method):\n            count +\u003d 1\n\nprint(f\u0027useless versioned method count: {count}\u0027)\n```\n\nNow, finally, we\u0027ll get an output of `0` when we run this:\n\n```\nuseless versioned method count: 0\n```\n\nHopefully that makes more sense?","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"becf8c5f0d76b24aca2af4f7b823e439326dca53","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ebb5da70_d151635f","in_reply_to":"d025058f_1c24a8a8","updated":"2024-05-01 17:02:03.000000000","message":"https://that.guru/blog/routers-in-openstack/","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"092b9073eb532003b0acfdd6b20bca6851b5ae61","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6f8a20b4_655fef58","in_reply_to":"ebb5da70_d151635f","updated":"2024-05-02 22:22:00.000000000","message":"Thanks, stephen for a nice explanation in the article. \n\nYes, it is not easy to map all these action/versioning complexities we have in wsgi layer. This change looks good to me, we did not store the sub-controller earlier but no reason not to do if we can use those in this automation.","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"}],"nova/api/openstack/wsgi.py":[{"author":{"_account_id":8556,"name":"Ghanshyam Maan","display_name":"Ghanshyam Maan","email":"gmaan.os14@gmail.com","username":"ghanshyam"},"change_message_id":"092b9073eb532003b0acfdd6b20bca6851b5ae61","unresolved":true,"context_lines":[{"line_number":415,"context_line":""},{"line_number":416,"context_line":"    def register_subcontroller_actions(self, sub_controller):"},{"line_number":417,"context_line":"        \"\"\"Registers sub-controller actions with this resource.\"\"\""},{"line_number":418,"context_line":"        self.sub_controllers.append(sub_controller)"},{"line_number":419,"context_line":"        actions \u003d getattr(sub_controller, \u0027wsgi_actions\u0027, {})"},{"line_number":420,"context_line":"        for key, method_name in actions.items():"},{"line_number":421,"context_line":"            self.wsgi_actions[key] \u003d getattr(sub_controller, method_name)"}],"source_content_type":"text/x-python","patch_set":1,"id":"0b60a8da_fa6ceb44","line":418,"range":{"start_line":418,"start_character":0,"end_line":418,"end_character":51},"updated":"2024-05-02 22:22:00.000000000","message":"Yes, we never stored the sub controller objects as we did not need those except mapping of action-name and func (wsgi_actions).\n\nI think there is no harm of storing the ref of sub controller also. I mean there is no reason to treat them differently than the main controller when we do such automation.\n\nI am +2 but will wait for melwitt to check this if she has something otherwise I am good to +W also.","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"57e65a07863bf96881e245a09e2e8db3a278f0d6","unresolved":true,"context_lines":[{"line_number":416,"context_line":"    def register_subcontroller_actions(self, sub_controller):"},{"line_number":417,"context_line":"        \"\"\"Registers sub-controller actions with this resource.\"\"\""},{"line_number":418,"context_line":"        self.sub_controllers.append(sub_controller)"},{"line_number":419,"context_line":"        actions \u003d getattr(sub_controller, \u0027wsgi_actions\u0027, {})"},{"line_number":420,"context_line":"        for key, method_name in actions.items():"},{"line_number":421,"context_line":"            self.wsgi_actions[key] \u003d getattr(sub_controller, method_name)"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"    def get_action_args(self, request_environment):"},{"line_number":424,"context_line":"        \"\"\"Parse dictionary created by routes library.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"aacaa556_27e812a6","line":421,"range":{"start_line":419,"start_character":7,"end_line":421,"end_character":73},"updated":"2024-04-19 10:03:44.000000000","message":"c is there any reason not to just call register_actions here","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"f9500e1ab29db3f00e2a1a162140cd4ad52686e5","unresolved":true,"context_lines":[{"line_number":416,"context_line":"    def register_subcontroller_actions(self, sub_controller):"},{"line_number":417,"context_line":"        \"\"\"Registers sub-controller actions with this resource.\"\"\""},{"line_number":418,"context_line":"        self.sub_controllers.append(sub_controller)"},{"line_number":419,"context_line":"        actions \u003d getattr(sub_controller, \u0027wsgi_actions\u0027, {})"},{"line_number":420,"context_line":"        for key, method_name in actions.items():"},{"line_number":421,"context_line":"            self.wsgi_actions[key] \u003d getattr(sub_controller, method_name)"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"    def get_action_args(self, request_environment):"},{"line_number":424,"context_line":"        \"\"\"Parse dictionary created by routes library.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"ea4d9a68_c5c79d7b","line":421,"range":{"start_line":419,"start_character":7,"end_line":421,"end_character":73},"in_reply_to":"aacaa556_27e812a6","updated":"2024-04-19 10:24:58.000000000","message":"I think I had some reason for doing this but I clearly didn\u0027t write it down. There\u0027s a good chance it was purely a purity thing. I\u0027m happy to de-dupe this in a follow-up?","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2017f89981bbd7ef521410946394dca3e45369b8","unresolved":false,"context_lines":[{"line_number":416,"context_line":"    def register_subcontroller_actions(self, sub_controller):"},{"line_number":417,"context_line":"        \"\"\"Registers sub-controller actions with this resource.\"\"\""},{"line_number":418,"context_line":"        self.sub_controllers.append(sub_controller)"},{"line_number":419,"context_line":"        actions \u003d getattr(sub_controller, \u0027wsgi_actions\u0027, {})"},{"line_number":420,"context_line":"        for key, method_name in actions.items():"},{"line_number":421,"context_line":"            self.wsgi_actions[key] \u003d getattr(sub_controller, method_name)"},{"line_number":422,"context_line":""},{"line_number":423,"context_line":"    def get_action_args(self, request_environment):"},{"line_number":424,"context_line":"        \"\"\"Parse dictionary created by routes library.\"\"\""}],"source_content_type":"text/x-python","patch_set":1,"id":"cdcc3cf3_ff04ac3c","line":421,"range":{"start_line":419,"start_character":7,"end_line":421,"end_character":73},"in_reply_to":"ea4d9a68_c5c79d7b","updated":"2024-04-22 10:14:56.000000000","message":"Acknowledged","commit_id":"233fe1865feab34d7d75332267613e640d8f552f"}]}