)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":33136,"name":"Rogerio Ferraz","display_name":"Rogerio Ferraz","email":"rferraz.windriver@gmail.com","username":"rferraz"},"change_message_id":"0e4f2af3e50b584250b066e9f9160df27f6d34fd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":11,"id":"2328cd06_4f1ae57b","updated":"2021-11-01 22:40:43.000000000","message":"LGTM","commit_id":"ec949ce385798d647b7433f84e0b1ae13d0c1932"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"b170ac3eb4297babe055e4c01f3f15ce95e026e7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":12,"id":"bf3edfd9_ec5dea7e","updated":"2021-11-24 17:41:03.000000000","message":"I think this code has some fundamental logic bugs.  Haven\u0027t finished looking at it, but I thought I\u0027d point out some initial issues.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"efdc28b7adc549a6f98b50e906e5f1c731293488","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":14,"id":"09246e7b_0e268979","updated":"2021-11-24 21:58:13.000000000","message":"fast-approved since Al had approved previously","commit_id":"699f4c9eb6a01c9c56ac216503734e2f3a3ea54b"}],"sysinv/sysinv/sysinv/sysinv/api/controllers/v1/host_fs.py":[{"author":{"_account_id":33136,"name":"Rogerio Ferraz","display_name":"Rogerio Ferraz","email":"rferraz.windriver@gmail.com","username":"rferraz"},"change_message_id":"0e4f2af3e50b584250b066e9f9160df27f6d34fd","unresolved":true,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# Copyright (c) 2020 Wind River Systems, Inc."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# SPDX-License-Identifier: Apache-2.0"},{"line_number":5,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":11,"id":"d8a1601e_fbbd608e","line":2,"range":{"start_line":2,"start_character":16,"end_line":2,"end_character":20},"updated":"2021-11-01 22:40:43.000000000","message":"2020-2021","commit_id":"ec949ce385798d647b7433f84e0b1ae13d0c1932"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"b170ac3eb4297babe055e4c01f3f15ce95e026e7","unresolved":true,"context_lines":[{"line_number":474,"context_line":""},{"line_number":475,"context_line":"    if host_fs[\u0027name\u0027] \u003d\u003d constants.FILESYSTEM_NAME_IMAGE_CONVERSION:"},{"line_number":476,"context_line":"        try:"},{"line_number":477,"context_line":"            app \u003d utils.is_openstack_installed(pecan.request.dbapi)"},{"line_number":478,"context_line":"            if app.status !\u003d constants.APP_UPLOAD_SUCCESS:"},{"line_number":479,"context_line":"                raise wsme.exc.ClientSideError("},{"line_number":480,"context_line":"                    _(\"Deleting filesystem %s is not allowed when %s is in %s \""}],"source_content_type":"text/x-python","patch_set":12,"id":"98d93ec5_3f106a54","line":477,"range":{"start_line":477,"start_character":24,"end_line":477,"end_character":46},"updated":"2021-11-24 17:41:03.000000000","message":"Should call utils.find_openstack_app().   See comment in utils.py.   Also, the way it\u0027s written now if the application isn\u0027t found we\u0027ll return False here, and then proceed to error out on the next line since False has no status member.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"a5bbd53c44e7039f7bba7c06d89d46c1d51a76a9","unresolved":true,"context_lines":[{"line_number":474,"context_line":""},{"line_number":475,"context_line":"    if host_fs[\u0027name\u0027] \u003d\u003d constants.FILESYSTEM_NAME_IMAGE_CONVERSION:"},{"line_number":476,"context_line":"        try:"},{"line_number":477,"context_line":"            app \u003d utils.is_openstack_installed(pecan.request.dbapi)"},{"line_number":478,"context_line":"            if app.status !\u003d constants.APP_UPLOAD_SUCCESS:"},{"line_number":479,"context_line":"                raise wsme.exc.ClientSideError("},{"line_number":480,"context_line":"                    _(\"Deleting filesystem %s is not allowed when %s is in %s \""}],"source_content_type":"text/x-python","patch_set":12,"id":"fe5ff483_bbd2cbc0","line":477,"range":{"start_line":477,"start_character":24,"end_line":477,"end_character":46},"in_reply_to":"98d93ec5_3f106a54","updated":"2021-11-24 18:32:51.000000000","message":"You are right, I probably just tested that with the app \u0027uploaded\u0027 and \u0027applied\u0027.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"b170ac3eb4297babe055e4c01f3f15ce95e026e7","unresolved":true,"context_lines":[{"line_number":475,"context_line":"    if host_fs[\u0027name\u0027] \u003d\u003d constants.FILESYSTEM_NAME_IMAGE_CONVERSION:"},{"line_number":476,"context_line":"        try:"},{"line_number":477,"context_line":"            app \u003d utils.is_openstack_installed(pecan.request.dbapi)"},{"line_number":478,"context_line":"            if app.status !\u003d constants.APP_UPLOAD_SUCCESS:"},{"line_number":479,"context_line":"                raise wsme.exc.ClientSideError("},{"line_number":480,"context_line":"                    _(\"Deleting filesystem %s is not allowed when %s is in %s \""},{"line_number":481,"context_line":"                      \"state\" % (host_fs[\u0027name\u0027], app.name, app.status))"}],"source_content_type":"text/x-python","patch_set":12,"id":"96382a72_be7d8adb","line":478,"range":{"start_line":478,"start_character":39,"end_line":478,"end_character":57},"updated":"2021-11-24 17:41:03.000000000","message":"Presumably we should also allow deleting the filesystem if the app status is APP_NOT_PRESENT, APP_UPLOAD_IN_PROGRESS, or APP_UPLOAD_FAILURE?","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"a5bbd53c44e7039f7bba7c06d89d46c1d51a76a9","unresolved":true,"context_lines":[{"line_number":475,"context_line":"    if host_fs[\u0027name\u0027] \u003d\u003d constants.FILESYSTEM_NAME_IMAGE_CONVERSION:"},{"line_number":476,"context_line":"        try:"},{"line_number":477,"context_line":"            app \u003d utils.is_openstack_installed(pecan.request.dbapi)"},{"line_number":478,"context_line":"            if app.status !\u003d constants.APP_UPLOAD_SUCCESS:"},{"line_number":479,"context_line":"                raise wsme.exc.ClientSideError("},{"line_number":480,"context_line":"                    _(\"Deleting filesystem %s is not allowed when %s is in %s \""},{"line_number":481,"context_line":"                      \"state\" % (host_fs[\u0027name\u0027], app.name, app.status))"}],"source_content_type":"text/x-python","patch_set":12,"id":"610a6f92_16951e46","line":478,"range":{"start_line":478,"start_character":39,"end_line":478,"end_character":57},"in_reply_to":"96382a72_be7d8adb","updated":"2021-11-24 18:32:51.000000000","message":"This is unrelated to this story/patch.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"}],"sysinv/sysinv/sysinv/sysinv/common/utils.py":[{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"b170ac3eb4297babe055e4c01f3f15ce95e026e7","unresolved":true,"context_lines":[{"line_number":1901,"context_line":""},{"line_number":1902,"context_line":"def is_openstack_installed(dbapi):"},{"line_number":1903,"context_line":"    \"\"\" Checks whether the OpenStack application is applied successfully. \"\"\""},{"line_number":1904,"context_line":"    try:"},{"line_number":1905,"context_line":"        return find_openstack_app(dbapi)"},{"line_number":1906,"context_line":"    except exception.KubeAppNotFound:"},{"line_number":1907,"context_line":"        return False"},{"line_number":1908,"context_line":""},{"line_number":1909,"context_line":""},{"line_number":1910,"context_line":"def is_openstack_applied(dbapi):"}],"source_content_type":"text/x-python","patch_set":12,"id":"a6e817d7_97460bd8","line":1907,"range":{"start_line":1904,"start_character":0,"end_line":1907,"end_character":20},"updated":"2021-11-24 17:41:03.000000000","message":"This is messy and disorganized.  The name of the function implies that it will return a boolean, but if it finds the application then it returns the application object, which is just weird.\n\nIf we want to return the application, then the caller should just use find_openstack_app() and deal with the exception (which it already does).","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"a5bbd53c44e7039f7bba7c06d89d46c1d51a76a9","unresolved":true,"context_lines":[{"line_number":1901,"context_line":""},{"line_number":1902,"context_line":"def is_openstack_installed(dbapi):"},{"line_number":1903,"context_line":"    \"\"\" Checks whether the OpenStack application is applied successfully. \"\"\""},{"line_number":1904,"context_line":"    try:"},{"line_number":1905,"context_line":"        return find_openstack_app(dbapi)"},{"line_number":1906,"context_line":"    except exception.KubeAppNotFound:"},{"line_number":1907,"context_line":"        return False"},{"line_number":1908,"context_line":""},{"line_number":1909,"context_line":""},{"line_number":1910,"context_line":"def is_openstack_applied(dbapi):"}],"source_content_type":"text/x-python","patch_set":12,"id":"c688de64_29a88d06","line":1907,"range":{"start_line":1904,"start_character":0,"end_line":1907,"end_character":20},"in_reply_to":"a6e817d7_97460bd8","updated":"2021-11-24 18:32:51.000000000","message":"Despite the aggressive tone of the comment, I agree. Will change.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"a9521f13681c63c8660747a30af1b950d4152afa","unresolved":true,"context_lines":[{"line_number":2499,"context_line":""},{"line_number":2500,"context_line":""},{"line_number":2501,"context_line":"def find_app_plugin_name(app_name):"},{"line_number":2502,"context_line":"    return \"openstack\" if \"openstack\" in app_name else app_name"},{"line_number":2503,"context_line":""},{"line_number":2504,"context_line":""},{"line_number":2505,"context_line":"def find_kube_app(dbapi, app_name):"}],"source_content_type":"text/x-python","patch_set":12,"id":"64dbfc2d_9084a74f","line":2502,"range":{"start_line":2502,"start_character":26,"end_line":2502,"end_character":49},"updated":"2021-11-24 19:14:19.000000000","message":"this should probably use app_name.endswith(\"openstack\") to match dbapi.kube_app_get_endswith()","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":28486,"name":"Tee Ngo","email":"tee.ngo@windriver.com","username":"teewrs"},"change_message_id":"39f6614c785c6ceaebeaa62020e9861eb220e361","unresolved":true,"context_lines":[{"line_number":2499,"context_line":""},{"line_number":2500,"context_line":""},{"line_number":2501,"context_line":"def find_app_plugin_name(app_name):"},{"line_number":2502,"context_line":"    return \"openstack\" if \"openstack\" in app_name else app_name"},{"line_number":2503,"context_line":""},{"line_number":2504,"context_line":""},{"line_number":2505,"context_line":"def find_kube_app(dbapi, app_name):"}],"source_content_type":"text/x-python","patch_set":12,"id":"5095db01_7ebcb48e","line":2502,"range":{"start_line":2502,"start_character":11,"end_line":2502,"end_character":22},"updated":"2021-11-18 19:00:18.000000000","message":"why is the same string literal used in many places?","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"b8bb5340b5bbcb76674882c74a3acd2dcc2df73c","unresolved":true,"context_lines":[{"line_number":2499,"context_line":""},{"line_number":2500,"context_line":""},{"line_number":2501,"context_line":"def find_app_plugin_name(app_name):"},{"line_number":2502,"context_line":"    return \"openstack\" if \"openstack\" in app_name else app_name"},{"line_number":2503,"context_line":""},{"line_number":2504,"context_line":""},{"line_number":2505,"context_line":"def find_kube_app(dbapi, app_name):"}],"source_content_type":"text/x-python","patch_set":12,"id":"35ac6997_017997a8","line":2502,"range":{"start_line":2502,"start_character":11,"end_line":2502,"end_character":22},"in_reply_to":"5095db01_7ebcb48e","updated":"2021-11-18 19:09:01.000000000","message":"As I replied to Al in a previous comment, I didn\u0027t want to declare it as a constant just because my plan is to get rid of this on the next release. Each app should be able to name its plugin whatever they want instead of having to match the app name (which is clearly a problem for stx-openstack and the downstream releases of it).","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"ee1c1704fdb1eaa7931bc27dde0b715370c2b0ca","unresolved":false,"context_lines":[{"line_number":2499,"context_line":""},{"line_number":2500,"context_line":""},{"line_number":2501,"context_line":"def find_app_plugin_name(app_name):"},{"line_number":2502,"context_line":"    return \"openstack\" if \"openstack\" in app_name else app_name"},{"line_number":2503,"context_line":""},{"line_number":2504,"context_line":""},{"line_number":2505,"context_line":"def find_kube_app(dbapi, app_name):"}],"source_content_type":"text/x-python","patch_set":12,"id":"4333e8cc_ac2a5f63","line":2502,"range":{"start_line":2502,"start_character":26,"end_line":2502,"end_character":49},"in_reply_to":"64dbfc2d_9084a74f","updated":"2021-11-24 20:59:43.000000000","message":"Done","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":28486,"name":"Tee Ngo","email":"tee.ngo@windriver.com","username":"teewrs"},"change_message_id":"39f6614c785c6ceaebeaa62020e9861eb220e361","unresolved":true,"context_lines":[{"line_number":2504,"context_line":""},{"line_number":2505,"context_line":"def find_kube_app(dbapi, app_name):"},{"line_number":2506,"context_line":"    try:"},{"line_number":2507,"context_line":"        if \"openstack\" in app_name:"},{"line_number":2508,"context_line":"            app_name \u003d find_openstack_app(dbapi).name"},{"line_number":2509,"context_line":""},{"line_number":2510,"context_line":"        app \u003d dbapi.kube_app_get(app_name)"}],"source_content_type":"text/x-python","patch_set":12,"id":"f79d971a_950baf94","line":2507,"range":{"start_line":2507,"start_character":11,"end_line":2507,"end_character":22},"updated":"2021-11-18 19:00:18.000000000","message":"ditto","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"a9521f13681c63c8660747a30af1b950d4152afa","unresolved":true,"context_lines":[{"line_number":2504,"context_line":""},{"line_number":2505,"context_line":"def find_kube_app(dbapi, app_name):"},{"line_number":2506,"context_line":"    try:"},{"line_number":2507,"context_line":"        if \"openstack\" in app_name:"},{"line_number":2508,"context_line":"            app_name \u003d find_openstack_app(dbapi).name"},{"line_number":2509,"context_line":""},{"line_number":2510,"context_line":"        app \u003d dbapi.kube_app_get(app_name)"}],"source_content_type":"text/x-python","patch_set":12,"id":"3f4a37b2_cdde26f4","line":2507,"range":{"start_line":2507,"start_character":23,"end_line":2507,"end_character":34},"updated":"2021-11-24 19:14:19.000000000","message":"this should probably use app_name.endswith(\"openstack\") to match dbapi.kube_app_get_endswith()","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"ee1c1704fdb1eaa7931bc27dde0b715370c2b0ca","unresolved":false,"context_lines":[{"line_number":2504,"context_line":""},{"line_number":2505,"context_line":"def find_kube_app(dbapi, app_name):"},{"line_number":2506,"context_line":"    try:"},{"line_number":2507,"context_line":"        if \"openstack\" in app_name:"},{"line_number":2508,"context_line":"            app_name \u003d find_openstack_app(dbapi).name"},{"line_number":2509,"context_line":""},{"line_number":2510,"context_line":"        app \u003d dbapi.kube_app_get(app_name)"}],"source_content_type":"text/x-python","patch_set":12,"id":"6458a22f_4c6821ac","line":2507,"range":{"start_line":2507,"start_character":23,"end_line":2507,"end_character":34},"in_reply_to":"3f4a37b2_cdde26f4","updated":"2021-11-24 20:59:43.000000000","message":"Done","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"}],"sysinv/sysinv/sysinv/sysinv/db/sqlalchemy/api.py":[{"author":{"_account_id":15435,"name":"Al Bailey","email":"albailey1974@gmail.com","username":"albailey"},"change_message_id":"c76613d68a18316f9e875c8b46f394b61cef0e88","unresolved":true,"context_lines":[{"line_number":7894,"context_line":"        return self._kube_app_get(name)"},{"line_number":7895,"context_line":""},{"line_number":7896,"context_line":"    @objects.objectify(objects.kube_app)"},{"line_number":7897,"context_line":"    def kube_app_get_endswith(self, name):"},{"line_number":7898,"context_line":"        query \u003d model_query(models.KubeApp)"},{"line_number":7899,"context_line":"        app_ls \u003d query.filter(models.KubeApp.name.endswith(name)).all()"},{"line_number":7900,"context_line":"        if not app_ls:"}],"source_content_type":"text/x-python","patch_set":11,"id":"37085fa3_f9ca3941","line":7897,"updated":"2021-11-03 17:59:03.000000000","message":"As a note:  we should have these also declared in db/api.py as abstract, however it looks like we did not do this for the other kube_app methods, so I dont think we needs to block the review over this.","commit_id":"ec949ce385798d647b7433f84e0b1ae13d0c1932"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"a9521f13681c63c8660747a30af1b950d4152afa","unresolved":true,"context_lines":[{"line_number":7896,"context_line":"    @objects.objectify(objects.kube_app)"},{"line_number":7897,"context_line":"    def kube_app_get_endswith(self, name):"},{"line_number":7898,"context_line":"        query \u003d model_query(models.KubeApp)"},{"line_number":7899,"context_line":"        app_ls \u003d query.filter(models.KubeApp.name.endswith(name)).all()"},{"line_number":7900,"context_line":"        if not app_ls:"},{"line_number":7901,"context_line":"            raise exception.KubeAppNotFound(name\u003d\"endswith \u0027{}\u0027\".format(name))"},{"line_number":7902,"context_line":"        return app_ls[0]"}],"source_content_type":"text/x-python","patch_set":12,"id":"9200006c_0c2a8aa9","line":7899,"range":{"start_line":7899,"start_character":66,"end_line":7899,"end_character":69},"updated":"2021-11-24 19:14:19.000000000","message":"We should maybe use .one_or_none() here and raise a new exception if there\u0027s more than one application that ends with the same string.\n\nIf we\u0027re okay with assuming that this will never happen, then we could use .first() here and return the result rather than building a list and then returning the first item from it.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"ee1c1704fdb1eaa7931bc27dde0b715370c2b0ca","unresolved":true,"context_lines":[{"line_number":7896,"context_line":"    @objects.objectify(objects.kube_app)"},{"line_number":7897,"context_line":"    def kube_app_get_endswith(self, name):"},{"line_number":7898,"context_line":"        query \u003d model_query(models.KubeApp)"},{"line_number":7899,"context_line":"        app_ls \u003d query.filter(models.KubeApp.name.endswith(name)).all()"},{"line_number":7900,"context_line":"        if not app_ls:"},{"line_number":7901,"context_line":"            raise exception.KubeAppNotFound(name\u003d\"endswith \u0027{}\u0027\".format(name))"},{"line_number":7902,"context_line":"        return app_ls[0]"}],"source_content_type":"text/x-python","patch_set":12,"id":"83ae443a_c2b08e17","line":7899,"range":{"start_line":7899,"start_character":66,"end_line":7899,"end_character":69},"in_reply_to":"9200006c_0c2a8aa9","updated":"2021-11-24 20:59:43.000000000","message":"That gave me a slightly different idea. I\u0027ll get one() and treat the exceptions differently. Please check PS14","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"}],"sysinv/sysinv/sysinv/sysinv/helm/helm.py":[{"author":{"_account_id":15435,"name":"Al Bailey","email":"albailey1974@gmail.com","username":"albailey"},"change_message_id":"c76613d68a18316f9e875c8b46f394b61cef0e88","unresolved":true,"context_lines":[{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    @staticmethod"},{"line_number":834,"context_line":"    def find_app_plugin_name(app_name):"},{"line_number":835,"context_line":"        return \"openstack\" if \"openstack\" in app_name else app_name"},{"line_number":836,"context_line":""},{"line_number":837,"context_line":"    def _find_kube_app_and_app_plugin_name(self, app_name):"},{"line_number":838,"context_line":"        return utils.find_kube_app(self.dbapi, app_name), \\"}],"source_content_type":"text/x-python","patch_set":11,"id":"d23f6128_d35ef21d","line":835,"range":{"start_line":835,"start_character":16,"end_line":835,"end_character":25},"updated":"2021-11-03 17:59:03.000000000","message":"just wondering if this should be a constant\nwe already have \nHELM_NS_OPENSTACK \u003d \u0027openstack\u0027\nSERVICE_TYPE_OPENSTACK \u003d \u0027openstack\u0027\n\nso one of those might make sense here, unless defining a new constant would be more clear.","commit_id":"ec949ce385798d647b7433f84e0b1ae13d0c1932"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"9200fbc4606c2f97ebe9b6dc1fa32ca5f8d98342","unresolved":true,"context_lines":[{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    @staticmethod"},{"line_number":834,"context_line":"    def find_app_plugin_name(app_name):"},{"line_number":835,"context_line":"        return \"openstack\" if \"openstack\" in app_name else app_name"},{"line_number":836,"context_line":""},{"line_number":837,"context_line":"    def _find_kube_app_and_app_plugin_name(self, app_name):"},{"line_number":838,"context_line":"        return utils.find_kube_app(self.dbapi, app_name), \\"}],"source_content_type":"text/x-python","patch_set":11,"id":"a8b8a867_53e3f21a","line":835,"range":{"start_line":835,"start_character":16,"end_line":835,"end_character":25},"in_reply_to":"d23f6128_d35ef21d","updated":"2021-11-03 18:12:09.000000000","message":"I didn\u0027t want to declare it as a constant just because my plan is to get rid of this on the next release. Each app should be able to name its plugin whatever they want instead of having to match the app name (which is clearly a problem for stx-openstack and the downstream releases of it).","commit_id":"ec949ce385798d647b7433f84e0b1ae13d0c1932"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"a9521f13681c63c8660747a30af1b950d4152afa","unresolved":true,"context_lines":[{"line_number":222,"context_line":"        \"\"\"Return an AppLifecycle operator based on app name\"\"\""},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        plugin_name \u003d utils.find_app_plugin_name(app_name)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        if plugin_name in self.app_lifecycle_operators:"},{"line_number":227,"context_line":"            operator \u003d self.app_lifecycle_operators[plugin_name]"},{"line_number":228,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"d51de0de_1145e578","line":225,"updated":"2021-11-24 19:14:19.000000000","message":"nit: probably don\u0027t need this blank line here as it\u0027s logically connected with the next line.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"ee1c1704fdb1eaa7931bc27dde0b715370c2b0ca","unresolved":false,"context_lines":[{"line_number":222,"context_line":"        \"\"\"Return an AppLifecycle operator based on app name\"\"\""},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"        plugin_name \u003d utils.find_app_plugin_name(app_name)"},{"line_number":225,"context_line":""},{"line_number":226,"context_line":"        if plugin_name in self.app_lifecycle_operators:"},{"line_number":227,"context_line":"            operator \u003d self.app_lifecycle_operators[plugin_name]"},{"line_number":228,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":12,"id":"75b04965_92d594b3","line":225,"in_reply_to":"d51de0de_1145e578","updated":"2021-11-24 20:59:43.000000000","message":"Done","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"a9521f13681c63c8660747a30af1b950d4152afa","unresolved":true,"context_lines":[{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        namespaces \u003d []"},{"line_number":340,"context_line":"        if chart_name in self.chart_operators:"},{"line_number":341,"context_line":"            app_plugin_name \u003d utils.find_app_plugin_name(app_name)"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"            namespaces \u003d self.chart_operators[chart_name].get_namespaces_by_app("},{"line_number":344,"context_line":"                app_plugin_name)"}],"source_content_type":"text/x-python","patch_set":12,"id":"6904d317_e83d0af3","line":341,"range":{"start_line":341,"start_character":12,"end_line":341,"end_character":27},"updated":"2021-11-24 19:14:19.000000000","message":"nit: the rest of this file uses \"plugin_name\"","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"ee1c1704fdb1eaa7931bc27dde0b715370c2b0ca","unresolved":true,"context_lines":[{"line_number":338,"context_line":""},{"line_number":339,"context_line":"        namespaces \u003d []"},{"line_number":340,"context_line":"        if chart_name in self.chart_operators:"},{"line_number":341,"context_line":"            app_plugin_name \u003d utils.find_app_plugin_name(app_name)"},{"line_number":342,"context_line":""},{"line_number":343,"context_line":"            namespaces \u003d self.chart_operators[chart_name].get_namespaces_by_app("},{"line_number":344,"context_line":"                app_plugin_name)"}],"source_content_type":"text/x-python","patch_set":12,"id":"c1ff8439_8b52b537","line":341,"range":{"start_line":341,"start_character":12,"end_line":341,"end_character":27},"in_reply_to":"6904d317_e83d0af3","updated":"2021-11-24 20:59:43.000000000","message":"\"plugin_name\" really don\u0027t mean much here since we have app plugins and puppet plugins that aren\u0027t delivered by the app itself, so I rather keep this name.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"a9521f13681c63c8660747a30af1b950d4152afa","unresolved":true,"context_lines":[{"line_number":716,"context_line":""},{"line_number":717,"context_line":"        # Get a manifest operator to provide a single point of"},{"line_number":718,"context_line":"        # manipulation for the chart, chart group and manifest schemas"},{"line_number":719,"context_line":"        manifest_op \u003d self.get_armada_manifest_operator(app.name)"},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"        # Load the manifest into the operator"},{"line_number":722,"context_line":"        armada_manifest \u003d utils.generate_synced_armada_manifest_fqpn("}],"source_content_type":"text/x-python","patch_set":12,"id":"aeed5884_02be552b","line":719,"range":{"start_line":719,"start_character":56,"end_line":719,"end_character":64},"updated":"2021-11-24 19:14:19.000000000","message":"why wouldn\u0027t we continue to use \"app_name\" here?","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"d40883c43d5fe50a667313cb66b948dcd56d8875","unresolved":true,"context_lines":[{"line_number":716,"context_line":""},{"line_number":717,"context_line":"        # Get a manifest operator to provide a single point of"},{"line_number":718,"context_line":"        # manipulation for the chart, chart group and manifest schemas"},{"line_number":719,"context_line":"        manifest_op \u003d self.get_armada_manifest_operator(app.name)"},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"        # Load the manifest into the operator"},{"line_number":722,"context_line":"        armada_manifest \u003d utils.generate_synced_armada_manifest_fqpn("}],"source_content_type":"text/x-python","patch_set":12,"id":"8c4096a0_5401d26b","line":719,"range":{"start_line":719,"start_character":56,"end_line":719,"end_character":64},"in_reply_to":"9893d451_131996d4","updated":"2021-11-24 21:57:31.000000000","message":"\"app_name\" is a function argument, so will be available everywhere inside generate_helm_application_overrides().  It would be a language violation for it to be garbage-collected early.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"ee1c1704fdb1eaa7931bc27dde0b715370c2b0ca","unresolved":true,"context_lines":[{"line_number":716,"context_line":""},{"line_number":717,"context_line":"        # Get a manifest operator to provide a single point of"},{"line_number":718,"context_line":"        # manipulation for the chart, chart group and manifest schemas"},{"line_number":719,"context_line":"        manifest_op \u003d self.get_armada_manifest_operator(app.name)"},{"line_number":720,"context_line":""},{"line_number":721,"context_line":"        # Load the manifest into the operator"},{"line_number":722,"context_line":"        armada_manifest \u003d utils.generate_synced_armada_manifest_fqpn("}],"source_content_type":"text/x-python","patch_set":12,"id":"9893d451_131996d4","line":719,"range":{"start_line":719,"start_character":56,"end_line":719,"end_character":64},"in_reply_to":"aeed5884_02be552b","updated":"2021-11-24 20:59:43.000000000","message":"app will be resused, so it can\u0027t be garbage collected. app_name might be freed to GC after 715. It\u0027s kinda over optimization, but I do it than don\u0027t.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"a9521f13681c63c8660747a30af1b950d4152afa","unresolved":true,"context_lines":[{"line_number":831,"context_line":"        manifest_op.save_delete_manifest()"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    @staticmethod"},{"line_number":834,"context_line":"    def find_app_plugin_name(app_name):"},{"line_number":835,"context_line":"        return \"openstack\" if \"openstack\" in app_name else app_name"},{"line_number":836,"context_line":""},{"line_number":837,"context_line":"    def _find_kube_app_and_app_plugin_name(self, app_name):"},{"line_number":838,"context_line":"        return utils.find_kube_app(self.dbapi, app_name), \\"}],"source_content_type":"text/x-python","patch_set":12,"id":"b363005d_515a604f","line":835,"range":{"start_line":834,"start_character":4,"end_line":835,"end_character":67},"updated":"2021-11-24 19:14:19.000000000","message":"dupe of utils.find_app_plugin_name()","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"ee1c1704fdb1eaa7931bc27dde0b715370c2b0ca","unresolved":true,"context_lines":[{"line_number":831,"context_line":"        manifest_op.save_delete_manifest()"},{"line_number":832,"context_line":""},{"line_number":833,"context_line":"    @staticmethod"},{"line_number":834,"context_line":"    def find_app_plugin_name(app_name):"},{"line_number":835,"context_line":"        return \"openstack\" if \"openstack\" in app_name else app_name"},{"line_number":836,"context_line":""},{"line_number":837,"context_line":"    def _find_kube_app_and_app_plugin_name(self, app_name):"},{"line_number":838,"context_line":"        return utils.find_kube_app(self.dbapi, app_name), \\"}],"source_content_type":"text/x-python","patch_set":12,"id":"9451cdbf_3017d9ff","line":835,"range":{"start_line":834,"start_character":4,"end_line":835,"end_character":67},"in_reply_to":"b363005d_515a604f","updated":"2021-11-24 20:59:43.000000000","message":"Good catch.","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"}],"sysinv/sysinv/sysinv/sysinv/puppet/ceph.py":[{"author":{"_account_id":33136,"name":"Rogerio Ferraz","display_name":"Rogerio Ferraz","email":"rferraz.windriver@gmail.com","username":"rferraz"},"change_message_id":"0e4f2af3e50b584250b066e9f9160df27f6d34fd","unresolved":true,"context_lines":[{"line_number":1,"context_line":"#"},{"line_number":2,"context_line":"# Copyright (c) 2017-2018 Wind River Systems, Inc."},{"line_number":3,"context_line":"#"},{"line_number":4,"context_line":"# SPDX-License-Identifier: Apache-2.0"},{"line_number":5,"context_line":"#"}],"source_content_type":"text/x-python","patch_set":11,"id":"56691038_a4b8c31d","line":2,"range":{"start_line":2,"start_character":21,"end_line":2,"end_character":25},"updated":"2021-11-01 22:40:43.000000000","message":"2021","commit_id":"ec949ce385798d647b7433f84e0b1ae13d0c1932"},{"author":{"_account_id":15435,"name":"Al Bailey","email":"albailey1974@gmail.com","username":"albailey"},"change_message_id":"b83cc3763d20ea367aedbb3252dc77d972bad2e8","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                config[\u0027platform::ceph::params::cephfs_filesystems\u0027] \u003d cephfs_filesystems"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        if utils.is_openstack_applied(self.dbapi):"},{"line_number":145,"context_line":"            openstack_app \u003d utils.find_openstack_app(self.dbapi)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"            if utils.is_chart_enabled("},{"line_number":148,"context_line":"                    self.dbapi,"}],"source_content_type":"text/x-python","patch_set":12,"id":"b549f25b_a88a7bc7","line":145,"updated":"2021-11-13 17:32:14.000000000","message":"Doesnt the line before this (is_openstack_applied) now been updated to return \u0027False\u0027 or the \u0027app\u0027, so isn\u0027t querying the app a second time redundant?","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":15435,"name":"Al Bailey","email":"albailey1974@gmail.com","username":"albailey"},"change_message_id":"e79441401905c8c945e96c052c07d94799c27a57","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                config[\u0027platform::ceph::params::cephfs_filesystems\u0027] \u003d cephfs_filesystems"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        if utils.is_openstack_applied(self.dbapi):"},{"line_number":145,"context_line":"            openstack_app \u003d utils.find_openstack_app(self.dbapi)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"            if utils.is_chart_enabled("},{"line_number":148,"context_line":"                    self.dbapi,"}],"source_content_type":"text/x-python","patch_set":12,"id":"a57b6333_84394f6b","line":145,"in_reply_to":"75d40157_8afc41f8","updated":"2021-11-16 18:19:19.000000000","message":"You are fine.  I thought is_openstack_applied was returning the entire object rather than just one of its attributes","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"},{"author":{"_account_id":32349,"name":"Thiago Paiva Brito","email":"thiago.brito@windriver.com","username":"outbrito"},"change_message_id":"bb2d1560b665ffe960622d0a9d73530e4e6d88cf","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                config[\u0027platform::ceph::params::cephfs_filesystems\u0027] \u003d cephfs_filesystems"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        if utils.is_openstack_applied(self.dbapi):"},{"line_number":145,"context_line":"            openstack_app \u003d utils.find_openstack_app(self.dbapi)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"            if utils.is_chart_enabled("},{"line_number":148,"context_line":"                    self.dbapi,"}],"source_content_type":"text/x-python","patch_set":12,"id":"75d40157_8afc41f8","line":145,"in_reply_to":"b549f25b_a88a7bc7","updated":"2021-11-16 12:31:54.000000000","message":"One can\u0027t do attributions inside conditional checking in python, and if you look closely on https://review.opendev.org/c/starlingx/config/+/814670/12/sysinv/sysinv/sysinv/sysinv/common/utils.py#1913, here we have to check \u0027app.active\u0027 too. I can use \u0027is_openstack_installed\u0027 and check \u0027active\u0027 manually instead, but that would spread behavior across several places on behalf of a round trip to the database (that I believe sqlalchemy caches, but I might be wrong). Do you think it worths the cost (code complexity vs. round trip to db)?","commit_id":"da288bb2b3a4548f4746085150993c9b0075e470"}]}
