)]}'
{"kuryr_kubernetes/controller/drivers/utils.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"07bf9089534a9a57d34270f576cdf2432aba2b97","unresolved":true,"context_lines":[{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrloadbalancer_crds(loadbalancer_crd\u003dNone):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"75122776_e8a0d8cf","line":345,"range":{"start_line":345,"start_character":31,"end_line":345,"end_character":48},"updated":"2021-06-30 14:43:57.000000000","message":"this should be namespace, right? either you provide a namespace (and only return the CRDs on that namespace) or you don\u0027t provide the namespace and return all the CRDs on all namespaces","commit_id":"350b8fe806453ef85fecf2eb33dd866357d88d91"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"7f43ea01cf524931bc96ff88651c0b1e0a6c0c83","unresolved":true,"context_lines":[{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrloadbalancer_crds(loadbalancer_crd\u003dNone):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":1,"id":"0d5e1095_40ad5510","line":345,"range":{"start_line":345,"start_character":31,"end_line":345,"end_character":48},"in_reply_to":"75122776_e8a0d8cf","updated":"2021-06-30 14:47:27.000000000","message":"\u003e this should be namespace, right? either you provide a namespace (and only return the CRDs on that namespace) or you don\u0027t provide the namespace and return all the CRDs on all namespaces\n\nOkay got it!","commit_id":"350b8fe806453ef85fecf2eb33dd866357d88d91"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f41239c4fdcab930c6c7a7d2db212fa53610056f","unresolved":true,"context_lines":[{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrloadbalancer_crds(namespace\u003dNone):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"b166996c_924dbaf8","line":345,"range":{"start_line":345,"start_character":4,"end_line":345,"end_character":30},"updated":"2021-07-01 06:45:59.000000000","message":"this function is exactly the same as the one defined in line 325. Perhaps a base function can be created that both calls to avoid code repetition","commit_id":"40ff64db5b4806382e0f8909af1ff3ffaa493aa2"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"b685daee3b181d6307cc7077f9c62cde47228f27","unresolved":true,"context_lines":[{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrloadbalancer_crds(namespace\u003dNone):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"6b2d8903_09cf4b94","line":345,"range":{"start_line":345,"start_character":4,"end_line":345,"end_character":30},"in_reply_to":"28ee656d_72fe522b","updated":"2021-07-02 15:06:20.000000000","message":"\u003e \u003e this function is exactly the same as the one defined in line 325. Perhaps a base function can be created that both calls to avoid code repetition\n\u003e \n\u003e Let me find a way around it, so the URL would be generic sort of\n\nI will focus on this once I am done with the reconciliation handling","commit_id":"40ff64db5b4806382e0f8909af1ff3ffaa493aa2"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"c65feb7b0b457832e8ff2276ee5f11c722d6cba2","unresolved":true,"context_lines":[{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrloadbalancer_crds(namespace\u003dNone):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"28ee656d_72fe522b","line":345,"range":{"start_line":345,"start_character":4,"end_line":345,"end_character":30},"in_reply_to":"b166996c_924dbaf8","updated":"2021-07-01 07:05:43.000000000","message":"\u003e this function is exactly the same as the one defined in line 325. Perhaps a base function can be created that both calls to avoid code repetition\n\nLet me find a way around it, so the URL would be generic sort of","commit_id":"40ff64db5b4806382e0f8909af1ff3ffaa493aa2"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"eacd0a3d7eeeeb219f1f18dfffc55fd1f6a561d6","unresolved":true,"context_lines":[{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"},{"line_number":349,"context_line":"        if namespace:"},{"line_number":350,"context_line":"            klb_path \u003d \u0027{}/{}/kuryrloadbalancers\u0027.format("},{"line_number":351,"context_line":"                constants.K8S_API_CRD_KURYRLOADBALANCERS, namespace)"},{"line_number":352,"context_line":"        else:"},{"line_number":353,"context_line":"            klb_path \u003d constants.K8S_API_CRD_KURYRLOADBALANCERS"},{"line_number":354,"context_line":"        klbs \u003d kubernetes.get(klb_path)"},{"line_number":355,"context_line":"        LOG.debug(\"Returning KuryrLoadBalancers %s\", klbs)"},{"line_number":356,"context_line":"    except k_exc.K8sResourceNotFound:"},{"line_number":357,"context_line":"        LOG.exception(\"KuryrLoadBalancer CRD not found\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"7ca10066_6e996d14","line":354,"range":{"start_line":349,"start_character":0,"end_line":354,"end_character":39},"updated":"2021-07-01 18:17:31.000000000","message":"Shouldn\u0027t we retrieve all CRDs regardless of Namespace?","commit_id":"ef6be4576b9f0b78bb905086e06dfdd67e49ec48"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b0ae53d0600628fbc6818c0b44f6160475f2b429","unresolved":true,"context_lines":[{"line_number":352,"context_line":"        else:"},{"line_number":353,"context_line":"            klb_path \u003d constants.K8S_API_CRD_KURYRLOADBALANCERS"},{"line_number":354,"context_line":"        klbs \u003d kubernetes.get(klb_path)"},{"line_number":355,"context_line":"        LOG.debug(\"Returning KuryrLoadBalancers %s\", klbs)"},{"line_number":356,"context_line":"    except k_exc.K8sResourceNotFound:"},{"line_number":357,"context_line":"        LOG.exception(\"KuryrLoadBalancer CRD not found\")"},{"line_number":358,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":7,"id":"af4ae042_bcc34982","line":355,"range":{"start_line":355,"start_character":0,"end_line":355,"end_character":58},"updated":"2021-07-02 16:28:13.000000000","message":"This would spam the logs, it has to go once the code is ready.","commit_id":"bec38211099399eb5349fa96f28a23f68ce667ab"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b0ae53d0600628fbc6818c0b44f6160475f2b429","unresolved":true,"context_lines":[{"line_number":356,"context_line":"    except k_exc.K8sResourceNotFound:"},{"line_number":357,"context_line":"        LOG.exception(\"KuryrLoadBalancer CRD not found\")"},{"line_number":358,"context_line":"        return []"},{"line_number":359,"context_line":"    except k_exc.K8sClientException:"},{"line_number":360,"context_line":"        LOG.exception(\"Exception during fetch KuryrLoadBalancers. Retrying.\")"},{"line_number":361,"context_line":"        raise k_exc.ResourceNotReady(klb_path)"},{"line_number":362,"context_line":"    return klbs.get(\u0027items\u0027, [])"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":""}],"source_content_type":"text/x-python","patch_set":7,"id":"d76b3717_d67778c2","line":361,"range":{"start_line":359,"start_character":0,"end_line":361,"end_character":46},"updated":"2021-07-02 16:28:13.000000000","message":"This is mixing responsibilities, a higher layer should translate the exception.","commit_id":"bec38211099399eb5349fa96f28a23f68ce667ab"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"16509c7b367e34b4143301d1010e8b02519898bc","unresolved":true,"context_lines":[{"line_number":341,"context_line":"        raise k_exc.ResourceNotReady(knp_path)"},{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrloadbalancer_crds(namespace\u003dNone):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"},{"line_number":349,"context_line":"        if namespace:"},{"line_number":350,"context_line":"            klb_path \u003d \u0027{}/{}/kuryrloadbalancers\u0027.format("},{"line_number":351,"context_line":"                constants.K8S_API_CRD_KURYRLOADBALANCERS, namespace)"},{"line_number":352,"context_line":"        else:"},{"line_number":353,"context_line":"            klb_path \u003d constants.K8S_API_CRD_KURYRLOADBALANCERS"},{"line_number":354,"context_line":"        klbs \u003d kubernetes.get(klb_path)"},{"line_number":355,"context_line":"        LOG.debug(\"Returning KuryrLoadBalancers %s\", klbs)"},{"line_number":356,"context_line":"    except k_exc.K8sResourceNotFound:"},{"line_number":357,"context_line":"        LOG.exception(\"KuryrLoadBalancer CRD not found\")"},{"line_number":358,"context_line":"        return []"},{"line_number":359,"context_line":"    except k_exc.K8sClientException:"},{"line_number":360,"context_line":"        LOG.exception(\"Exception during fetch KuryrLoadBalancers. Retrying.\")"},{"line_number":361,"context_line":"        raise k_exc.ResourceNotReady(klb_path)"},{"line_number":362,"context_line":"    return klbs.get(\u0027items\u0027, [])"},{"line_number":363,"context_line":""},{"line_number":364,"context_line":""},{"line_number":365,"context_line":"def get_networkpolicies(namespace\u003dNone):"}],"source_content_type":"text/x-python","patch_set":10,"id":"851bd213_4673a647","line":362,"range":{"start_line":344,"start_character":0,"end_line":362,"end_character":32},"updated":"2021-07-08 06:41:14.000000000","message":"this still needs to be \"merged\" with the previous function to avoid code duplication","commit_id":"062ade6efd134efe18f44786c0a78e011b8d19bc"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"91b1f9a58fa67507fedc303fbab94aa0c63c3100","unresolved":true,"context_lines":[{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrk8s_resource(name, resource):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"a4500fd5_f3bc5478","line":345,"updated":"2021-07-12 05:03:17.000000000","message":"I have created this utility function to also replace the one defined at line 325. The name and the resource needs to be passed from the calling function.","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"d16420bdf4d85701a869cf7257b999facdc7cc40","unresolved":true,"context_lines":[{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrk8s_resource(name, resource):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"32f06e0b_80c67202","line":345,"in_reply_to":"98592f1d_117a9d47","updated":"2021-07-12 08:31:03.000000000","message":"\u003e perhaps I did not explain myself properly here. My intention was to have the 2 functions you had before (get_kuryrnetworkpolicy_crds, get_kuryrloadbalancer_crds), but internally they will call something similar as what you have here (e.g., _get_crd_resource).\n\nOkay, If I read you correctly, I would need to have both functions previously and still have them call this  _get_crd_resource function individually? I think that is correct?","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"aec8e0c6dc6b8b8bbddc50c20af9d269d0afa227","unresolved":true,"context_lines":[{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrk8s_resource(name, resource):"},{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":12,"id":"98592f1d_117a9d47","line":345,"in_reply_to":"a4500fd5_f3bc5478","updated":"2021-07-12 07:32:35.000000000","message":"perhaps I did not explain myself properly here. My intention was to have the 2 functions you had before (get_kuryrnetworkpolicy_crds, get_kuryrloadbalancer_crds), but internally they will call something similar as what you have here (e.g., _get_crd_resource).","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"7fd486e6a4066077d91380069a5eb46fc7adc7f9","unresolved":true,"context_lines":[{"line_number":346,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"},{"line_number":349,"context_line":"        resource_path \u003d resource"},{"line_number":350,"context_line":"        klbs \u003d kubernetes.get(resource_path)"},{"line_number":351,"context_line":"        LOG.debug(\"Returning %s\", name)"},{"line_number":352,"context_line":"    except k_exc.K8sResourceNotFound:"}],"source_content_type":"text/x-python","patch_set":12,"id":"9c83ac6f_093c74e5","line":349,"range":{"start_line":349,"start_character":8,"end_line":349,"end_character":21},"updated":"2021-07-12 08:59:04.000000000","message":"Why does the resource need to be reassigned?\nBetter to directly change the name of resource parameter to resource_path instead.","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"aec8e0c6dc6b8b8bbddc50c20af9d269d0afa227","unresolved":true,"context_lines":[{"line_number":347,"context_line":""},{"line_number":348,"context_line":"    try:"},{"line_number":349,"context_line":"        resource_path \u003d resource"},{"line_number":350,"context_line":"        klbs \u003d kubernetes.get(resource_path)"},{"line_number":351,"context_line":"        LOG.debug(\"Returning %s\", name)"},{"line_number":352,"context_line":"    except k_exc.K8sResourceNotFound:"},{"line_number":353,"context_line":"        LOG.exception(\u0027{} CRD not found\u0027.format(name))"}],"source_content_type":"text/x-python","patch_set":12,"id":"c0147a1a_b2925052","line":350,"range":{"start_line":350,"start_character":8,"end_line":350,"end_character":13},"updated":"2021-07-12 07:32:35.000000000","message":"klbs is not the right name for the var name if this is for different type of resources. Also missing the namespace piece","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"7fd486e6a4066077d91380069a5eb46fc7adc7f9","unresolved":true,"context_lines":[{"line_number":355,"context_line":"    except k_exc.K8sClientException:"},{"line_number":356,"context_line":"        LOG.exception(\"Exception during fetch %s.\", name)"},{"line_number":357,"context_line":"        raise k_exc.ResourceNotReady(resource_path)"},{"line_number":358,"context_line":"    return klbs.get(\u0027items\u0027, [])"},{"line_number":359,"context_line":""},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"def get_networkpolicies(namespace\u003dNone):"}],"source_content_type":"text/x-python","patch_set":12,"id":"b4a5687a_66c95b05","line":358,"updated":"2021-07-12 08:59:04.000000000","message":"might be safer to define klbs \u003d {} before the try/except block or move this return to line 351","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"534d4b9c96a36b2cb892f79222c5e3558d1087df","unresolved":true,"context_lines":[{"line_number":349,"context_line":"                    constants.K8S_API_CRD_KURYRLOADBALANCERS, namespace)"},{"line_number":350,"context_line":"    else:"},{"line_number":351,"context_line":"        klb_path \u003d constants.K8S_API_CRD_KURYRLOADBALANCERS"},{"line_number":352,"context_line":"    return get_kuryrk8s_resource(name, klb_path)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"def get_kuryrk8s_resource(name, resource_path):"}],"source_content_type":"text/x-python","patch_set":13,"id":"56916150_147571ea","line":352,"range":{"start_line":352,"start_character":11,"end_line":352,"end_character":32},"updated":"2021-07-13 08:37:12.000000000","message":"If we are good with the implementation then I can modify line 325.","commit_id":"1abd688e49ba079214703485c245e277a0511510"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5b3a5a627ff19925c03ca03b19e299548865dc94","unresolved":true,"context_lines":[{"line_number":343,"context_line":""},{"line_number":344,"context_line":""},{"line_number":345,"context_line":"def get_kuryrloadbalancer_crds(namespace\u003dNone):"},{"line_number":346,"context_line":"    name \u003d \u0027KuryrLoadBalancers\u0027"},{"line_number":347,"context_line":"    if namespace:"},{"line_number":348,"context_line":"        klb_path \u003d \u0027{}/{}/kuryrloadbalancers\u0027.format("},{"line_number":349,"context_line":"                    constants.K8S_API_CRD_KURYRLOADBALANCERS, namespace)"}],"source_content_type":"text/x-python","patch_set":14,"id":"49f69db8_3268fbb2","line":346,"range":{"start_line":346,"start_character":4,"end_line":346,"end_character":8},"updated":"2021-07-13 09:53:58.000000000","message":"the path already holds the information about which resource if being fetched, so I guess this is not needed","commit_id":"ec85a19a058c68f099b7ef2ed33d3097381e629f"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5b3a5a627ff19925c03ca03b19e299548865dc94","unresolved":true,"context_lines":[{"line_number":352,"context_line":"    return get_kuryrk8s_resource(name, klb_path)"},{"line_number":353,"context_line":""},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"def get_kuryrk8s_resource(name, resource_path):"},{"line_number":356,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":357,"context_line":"    rsrcs \u003d {}"},{"line_number":358,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":14,"id":"27aa45ce_dd199020","line":355,"range":{"start_line":355,"start_character":4,"end_line":355,"end_character":25},"updated":"2021-07-13 09:53:58.000000000","message":"get_k8s_resource might fit better as the method can also collect any k8s native resource","commit_id":"ec85a19a058c68f099b7ef2ed33d3097381e629f"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5b3a5a627ff19925c03ca03b19e299548865dc94","unresolved":true,"context_lines":[{"line_number":354,"context_line":""},{"line_number":355,"context_line":"def get_kuryrk8s_resource(name, resource_path):"},{"line_number":356,"context_line":"    kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":357,"context_line":"    rsrcs \u003d {}"},{"line_number":358,"context_line":"    try:"},{"line_number":359,"context_line":"        rsrcs \u003d kubernetes.get(resource_path)"},{"line_number":360,"context_line":"        LOG.debug(\"Returning {} {}\".format(name, resource_path))"}],"source_content_type":"text/x-python","patch_set":14,"id":"975849f7_ffe97fa9","line":357,"range":{"start_line":357,"start_character":4,"end_line":357,"end_character":9},"updated":"2021-07-13 09:53:58.000000000","message":"what would this variable name mean?\nmaybe rephrase to k8s_resource?","commit_id":"ec85a19a058c68f099b7ef2ed33d3097381e629f"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5b3a5a627ff19925c03ca03b19e299548865dc94","unresolved":true,"context_lines":[{"line_number":357,"context_line":"    rsrcs \u003d {}"},{"line_number":358,"context_line":"    try:"},{"line_number":359,"context_line":"        rsrcs \u003d kubernetes.get(resource_path)"},{"line_number":360,"context_line":"        LOG.debug(\"Returning {} {}\".format(name, resource_path))"},{"line_number":361,"context_line":"    except k_exc.K8sResourceNotFound:"},{"line_number":362,"context_line":"        LOG.exception(\u0027{} CRD not found\u0027.format(name))"},{"line_number":363,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":14,"id":"656382f0_f4037834","line":360,"range":{"start_line":360,"start_character":8,"end_line":360,"end_character":64},"updated":"2021-07-13 09:53:58.000000000","message":"No need to add this log as it\u0027s already logged inside the get method\nhttps://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/k8s_client.py#L115","commit_id":"ec85a19a058c68f099b7ef2ed33d3097381e629f"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"a85bd81404c452c1f5e50aed3e78af5f8b1a5ae8","unresolved":true,"context_lines":[{"line_number":357,"context_line":"    rsrcs \u003d {}"},{"line_number":358,"context_line":"    try:"},{"line_number":359,"context_line":"        rsrcs \u003d kubernetes.get(resource_path)"},{"line_number":360,"context_line":"        LOG.debug(\"Returning {} {}\".format(name, resource_path))"},{"line_number":361,"context_line":"    except k_exc.K8sResourceNotFound:"},{"line_number":362,"context_line":"        LOG.exception(\u0027{} CRD not found\u0027.format(name))"},{"line_number":363,"context_line":"        return []"}],"source_content_type":"text/x-python","patch_set":14,"id":"79565a30_0e47f522","line":360,"range":{"start_line":360,"start_character":8,"end_line":360,"end_character":64},"in_reply_to":"656382f0_f4037834","updated":"2021-07-13 11:30:13.000000000","message":"\u003e No need to add this log as it\u0027s already logged inside the get method\n\u003e https://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/k8s_client.py#L115\n\nGot it!","commit_id":"ec85a19a058c68f099b7ef2ed33d3097381e629f"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"33d82192b8c3ccbd530db1c1791eff98b94ebfb5","unresolved":true,"context_lines":[{"line_number":338,"context_line":"        return []"},{"line_number":339,"context_line":"    except k_exc.K8sClientException:"},{"line_number":340,"context_line":"        LOG.exception(\"Exception during fetch KuryrNetworkPolicies. Retrying.\")"},{"line_number":341,"context_line":"        raise k_exc.ResourceNotReady(knp_path)"},{"line_number":342,"context_line":"    return knps.get(\u0027items\u0027, [])"},{"line_number":343,"context_line":""},{"line_number":344,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"19e4a883_1752a693","side":"PARENT","line":341,"range":{"start_line":341,"start_character":0,"end_line":341,"end_character":46},"updated":"2021-08-03 13:14:50.000000000","message":"Looks like this modifies the behavior of unrelated code, that ResourceNotReady most likely was there on purpose.","commit_id":"862deaa455abfce55a8e50993cbfd8b1e108b720"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"33d82192b8c3ccbd530db1c1791eff98b94ebfb5","unresolved":true,"context_lines":[{"line_number":349,"context_line":"        LOG.exception(\u0027Kubernetes CRD not found\u0027)"},{"line_number":350,"context_line":"        return []"},{"line_number":351,"context_line":"    except k_exc.K8sClientException:"},{"line_number":352,"context_line":"        LOG.exception(\"Exception during Kubernetes recource\")"},{"line_number":353,"context_line":"    return k8s_resource.get(\u0027items\u0027, [])"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"81151763_af71bfd9","line":352,"range":{"start_line":352,"start_character":23,"end_line":352,"end_character":59},"updated":"2021-08-03 13:14:50.000000000","message":"\"Exception while fetching Kubernetes resource\"","commit_id":"7f2c925a51794558c00501bfb8b39a13f158c545"}],"kuryr_kubernetes/controller/handlers/loadbalancer.py":[{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"07bf9089534a9a57d34270f576cdf2432aba2b97","unresolved":true,"context_lines":[{"line_number":130,"context_line":"        klbs \u003d []"},{"line_number":131,"context_line":"        try:"},{"line_number":132,"context_line":"            klbs \u003d driver_utils.get_kuryrloadbalancer_crds()"},{"line_number":133,"context_line":"            klbs_status \u003d []"},{"line_number":134,"context_line":"            for klb in klbs:"},{"line_number":135,"context_line":"                klbs_status.append(klb[\u0027status\u0027])"},{"line_number":136,"context_line":"        except k_exc.K8sClientException:"},{"line_number":137,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":138,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":1,"id":"0f212e45_afda2003","line":135,"range":{"start_line":133,"start_character":0,"end_line":135,"end_character":49},"updated":"2021-06-30 14:43:57.000000000","message":"this should not be inside this try/except","commit_id":"350b8fe806453ef85fecf2eb33dd866357d88d91"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"50c73441349ee5c2a943b1def7bdbd3270ad1bf3","unresolved":true,"context_lines":[{"line_number":141,"context_line":"            lbs_id.append(klb_status[\u0027loadbalancer\u0027][\u0027id\u0027])"},{"line_number":142,"context_line":"        lbaas \u003d clients.get_loadbalancer_client()"},{"line_number":143,"context_line":"        loadbalancers \u003d lbaas.load_balancers()"},{"line_number":144,"context_line":"        for loadbalancer in loadbalancers:"},{"line_number":145,"context_line":"            if loadbalancer[\u0027id\u0027] not in lbs_id:"},{"line_number":146,"context_line":"                LOG.debug(\"Reconciling loadbalancers in OpenStack with CRDs\")"},{"line_number":147,"context_line":"                self._reconcile_lbaas(lbs_id)"},{"line_number":148,"context_line":"            else:"},{"line_number":149,"context_line":"                LOG.debug(\"Skipping reconciliations of loadbalancers\""},{"line_number":150,"context_line":"                          \"in OpenStack as they are in sync with Kubernetes\")"}],"source_content_type":"text/x-python","patch_set":1,"id":"46aafa0c_74223dcd","line":147,"range":{"start_line":144,"start_character":0,"end_line":147,"end_character":45},"updated":"2021-06-30 15:09:01.000000000","message":"actually, this should be the other way around... you need to check if the loadbalancer on the CRD exist on the list of loadbalanacers on the OpenStack side","commit_id":"350b8fe806453ef85fecf2eb33dd866357d88d91"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f41239c4fdcab930c6c7a7d2db212fa53610056f","unresolved":true,"context_lines":[{"line_number":126,"context_line":"                              \u0027reconciliation. It will be tried in 10s\u0027)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def _trigger_loadbalancer_reconciliation(self):"},{"line_number":129,"context_line":"        import pdb; pdb.set_trace()"},{"line_number":130,"context_line":"        LOG.debug(\"Reconciling the loadbalancer CRDs\")"},{"line_number":131,"context_line":"        klbs \u003d []"},{"line_number":132,"context_line":"        try:"}],"source_content_type":"text/x-python","patch_set":2,"id":"64c779b7_fa33e489","line":129,"range":{"start_line":129,"start_character":8,"end_line":129,"end_character":35},"updated":"2021-07-01 06:45:59.000000000","message":"remember removing this before submitting the patch sets","commit_id":"40ff64db5b4806382e0f8909af1ff3ffaa493aa2"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f41239c4fdcab930c6c7a7d2db212fa53610056f","unresolved":true,"context_lines":[{"line_number":135,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":136,"context_line":"            return"},{"line_number":137,"context_line":"        klbs_status \u003d []"},{"line_number":138,"context_line":"        for klb in klbs:"},{"line_number":139,"context_line":"            klbs_status.append(klb[\u0027status\u0027])"},{"line_number":140,"context_line":"        lbs_id \u003d []"},{"line_number":141,"context_line":"        for klb_status in klbs_status:"},{"line_number":142,"context_line":"            lbs_id.append(klb_status[\u0027loadbalancer\u0027][\u0027id\u0027])"},{"line_number":143,"context_line":"        lbaas \u003d clients.get_loadbalancer_client()"},{"line_number":144,"context_line":"        loadbalancers \u003d lbaas.load_balancers()"},{"line_number":145,"context_line":"        loadbalancers_id \u003d [] "}],"source_content_type":"text/x-python","patch_set":2,"id":"33ca183d_aa619aeb","line":142,"range":{"start_line":138,"start_character":0,"end_line":142,"end_character":59},"updated":"2021-07-01 06:45:59.000000000","message":"this can be simplified with list comprehension","commit_id":"40ff64db5b4806382e0f8909af1ff3ffaa493aa2"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f41239c4fdcab930c6c7a7d2db212fa53610056f","unresolved":true,"context_lines":[{"line_number":145,"context_line":"        loadbalancers_id \u003d [] "},{"line_number":146,"context_line":"        for loadbalancer in loadbalancers:"},{"line_number":147,"context_line":"            loadbalancers_id.append(loadbalancer[\u0027id\u0027].split(\" \"))"},{"line_number":148,"context_line":"            if lbs_id not in loadbalancers_id:"},{"line_number":149,"context_line":"                LOG.debug(\"Reconciling loadbalancers in OpenStack with CRDs\")"},{"line_number":150,"context_line":"                self._reconcile_lbaas(lbs_id)"},{"line_number":151,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":2,"id":"626be659_8836562d","line":148,"range":{"start_line":148,"start_character":12,"end_line":148,"end_character":46},"updated":"2021-07-01 06:45:59.000000000","message":"this is wrong, lbs_id is another list. You need to iterate on lbs_id, and check if each id in lbs_id is present in the list of ids on the openstack side (loadbalancers_id)","commit_id":"40ff64db5b4806382e0f8909af1ff3ffaa493aa2"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"d585c0fae57a42bdce34589fef2b9c57e19c7c4a","unresolved":true,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _reconcile_loadbalancers(self):"},{"line_number":120,"context_line":"        while True:"},{"line_number":121,"context_line":"            eventlet.sleep(60)"},{"line_number":122,"context_line":"            try:"},{"line_number":123,"context_line":"                self._trigger_loadbalancer_reconciliation()"},{"line_number":124,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":4,"id":"bc519322_b5e13f1b","line":121,"range":{"start_line":121,"start_character":27,"end_line":121,"end_character":29},"updated":"2021-07-01 18:16:05.000000000","message":"The value of retry might be too small, we don\u0027t want frequent openstack calls. Should this be used CRD_RECONCILIATION_FREQUENCY?","commit_id":"ef6be4576b9f0b78bb905086e06dfdd67e49ec48"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"d585c0fae57a42bdce34589fef2b9c57e19c7c4a","unresolved":true,"context_lines":[{"line_number":133,"context_line":"        except k_exc.K8sClientException:"},{"line_number":134,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":"        lbs_id \u003d [klb[\u0027status\u0027][\u0027loadbalancer\u0027][\u0027id\u0027] for klb in klbs]"},{"line_number":137,"context_line":"        lbaas \u003d clients.get_loadbalancer_client()"},{"line_number":138,"context_line":"        lbaas_spec \u003d {}"},{"line_number":139,"context_line":"        self._drv_lbaas.add_tags(\u0027loadbalancer\u0027, lbaas_spec)"}],"source_content_type":"text/x-python","patch_set":4,"id":"79d02312_21f2e93a","line":136,"range":{"start_line":136,"start_character":18,"end_line":136,"end_character":53},"updated":"2021-07-01 18:16:05.000000000","message":"It\u0027s not safe to directly access the keys of the dict (crd might not a lb yet), better to use .get()","commit_id":"ef6be4576b9f0b78bb905086e06dfdd67e49ec48"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"f95f346d5ac227b996b15c2f53d67230b4b6ed6b","unresolved":true,"context_lines":[{"line_number":140,"context_line":"        loadbalancers \u003d lbaas.load_balancers(**lbaas_spec)"},{"line_number":141,"context_line":"        loadbalancers_id \u003d [loadbalancer[\u0027id\u0027]"},{"line_number":142,"context_line":"                            for loadbalancer in loadbalancers]"},{"line_number":143,"context_line":"        if loadbalancers_id not in lbs_id:"},{"line_number":144,"context_line":"            LOG.debug(\"Reconciling loadbalancers in OpenStack with CRDs\")"},{"line_number":145,"context_line":"            self._reconcile_lbaas(lbs_id)"},{"line_number":146,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":4,"id":"44b7467b_75d260f2","line":143,"range":{"start_line":143,"start_character":0,"end_line":143,"end_character":42},"updated":"2021-07-01 13:09:49.000000000","message":"this should be the other way around... looking for each of the loadbalancer ids in lbs_id that are not in loadbalanacers_id","commit_id":"ef6be4576b9f0b78bb905086e06dfdd67e49ec48"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"d585c0fae57a42bdce34589fef2b9c57e19c7c4a","unresolved":true,"context_lines":[{"line_number":145,"context_line":"            self._reconcile_lbaas(lbs_id)"},{"line_number":146,"context_line":"        else:"},{"line_number":147,"context_line":"            LOG.debug(\"Skipping reconciliations of loadbalancers\""},{"line_number":148,"context_line":"                      \"in OpenStack as they are in sync with Kubernetes\")"},{"line_number":149,"context_line":"            return"},{"line_number":150,"context_line":""},{"line_number":151,"context_line":"    def _reconcile_lbaas(self, lbs_id):"}],"source_content_type":"text/x-python","patch_set":4,"id":"793f0264_b489a8ad","line":148,"range":{"start_line":148,"start_character":61,"end_line":148,"end_character":72},"updated":"2021-07-01 18:16:05.000000000","message":"Kubernetes Services","commit_id":"ef6be4576b9f0b78bb905086e06dfdd67e49ec48"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b2413c2b1782078311a5ef338e0bd638bc9e30fd","unresolved":true,"context_lines":[{"line_number":31,"context_line":"CONF \u003d config.CONF"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"OCTAVIA_DEFAULT_PROVIDERS \u003d [\u0027octavia\u0027, \u0027amphora\u0027]"},{"line_number":34,"context_line":"CRD_RECONCILIATION_FREQUENCY \u003d 10  # seconds"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"class KuryrLoadBalancerHandler(k8s_base.ResourceEventHandler):"}],"source_content_type":"text/x-python","patch_set":5,"id":"d22e047e_580ea847","line":34,"range":{"start_line":34,"start_character":0,"end_line":34,"end_character":44},"updated":"2021-07-02 06:53:49.000000000","message":"this should be way bigger, perhaps 10 mins","commit_id":"c943b973e562603cb15893f62ffb84bc76a79a61"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b2413c2b1782078311a5ef338e0bd638bc9e30fd","unresolved":true,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"    def _reconcile_loadbalancers(self):"},{"line_number":120,"context_line":"        while True:"},{"line_number":121,"context_line":"            eventlet.sleep(60)"},{"line_number":122,"context_line":"            try:"},{"line_number":123,"context_line":"                self._trigger_loadbalancer_reconciliation()"},{"line_number":124,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":5,"id":"43f983bb_63216f8b","line":121,"range":{"start_line":121,"start_character":12,"end_line":121,"end_character":30},"updated":"2021-07-02 06:53:49.000000000","message":"this should use the global var you defined instead","commit_id":"c943b973e562603cb15893f62ffb84bc76a79a61"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b2413c2b1782078311a5ef338e0bd638bc9e30fd","unresolved":true,"context_lines":[{"line_number":134,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":135,"context_line":"            return"},{"line_number":136,"context_line":"        # get the loadbalancers id in the CRD status"},{"line_number":137,"context_line":"        loadbalancer_crds_id \u003d [loadbalancer_crd[\u0027status\u0027][\u0027loadbalancer\u0027]"},{"line_number":138,"context_line":"                                [\u0027id\u0027] for loadbalancer_crd"},{"line_number":139,"context_line":"                                in loadbalancer_crds]"},{"line_number":140,"context_line":"        lbaas \u003d clients.get_loadbalancer_client()"},{"line_number":141,"context_line":"        lbaas_spec \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"3cd3f291_b3f369fc","line":138,"range":{"start_line":137,"start_character":32,"end_line":138,"end_character":38},"updated":"2021-07-02 06:53:49.000000000","message":"this should use .get(...) calls instead to avoid exceptions if status if empty","commit_id":"c943b973e562603cb15893f62ffb84bc76a79a61"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b2413c2b1782078311a5ef338e0bd638bc9e30fd","unresolved":true,"context_lines":[{"line_number":150,"context_line":"                             lb_id not in loadbalancers_id]"},{"line_number":151,"context_line":"        LOG.debug(\"Acquired the following loadbalancer crds missing in \""},{"line_number":152,"context_line":"                  \"OpenStack: %r\", crds_to_reconcile)"},{"line_number":153,"context_line":"        if crds_to_reconcile is not [] or None:"},{"line_number":154,"context_line":"            for crd_id in crds_to_reconcile:"},{"line_number":155,"context_line":"                LOG.debug(\"Reconciling loadbalancer: %r\", crd_id)"},{"line_number":156,"context_line":"                self._reconcile_lbaas(crd_id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"2bf9808b_e6a94d16","line":153,"range":{"start_line":153,"start_character":8,"end_line":153,"end_character":47},"updated":"2021-07-02 06:53:49.000000000","message":"this is the same as \"if crds_to_reconcile\"\n\nIn fact you don\u0027t even need the if. The loop for will take care of it","commit_id":"c943b973e562603cb15893f62ffb84bc76a79a61"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"7c5eea80470e517e5c460350e7c081f64dad2688","unresolved":true,"context_lines":[{"line_number":151,"context_line":"        LOG.debug(\"Acquired the following loadbalancer crds missing in \""},{"line_number":152,"context_line":"                  \"OpenStack: %r\", crds_to_reconcile)"},{"line_number":153,"context_line":"        if crds_to_reconcile is not [] or None:"},{"line_number":154,"context_line":"            for crd_id in crds_to_reconcile:"},{"line_number":155,"context_line":"                LOG.debug(\"Reconciling loadbalancer: %r\", crd_id)"},{"line_number":156,"context_line":"                self._reconcile_lbaas(crd_id)"},{"line_number":157,"context_line":"        elif crds_to_reconcile is None or []:"}],"source_content_type":"text/x-python","patch_set":5,"id":"1fd61089_86500bc2","line":154,"updated":"2021-07-02 10:00:58.000000000","message":"This loop can be avoided if you use the loop at line 149.","commit_id":"c943b973e562603cb15893f62ffb84bc76a79a61"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b2413c2b1782078311a5ef338e0bd638bc9e30fd","unresolved":true,"context_lines":[{"line_number":154,"context_line":"            for crd_id in crds_to_reconcile:"},{"line_number":155,"context_line":"                LOG.debug(\"Reconciling loadbalancer: %r\", crd_id)"},{"line_number":156,"context_line":"                self._reconcile_lbaas(crd_id)"},{"line_number":157,"context_line":"        elif crds_to_reconcile is None or []:"},{"line_number":158,"context_line":"            LOG.debug(\"Skipping empty loadbalancer reconciliation: %r\","},{"line_number":159,"context_line":"                      [])"},{"line_number":160,"context_line":"            return"},{"line_number":161,"context_line":"        else:"},{"line_number":162,"context_line":"            LOG.debug(\"Unable to read loadbalancer_crds\")"},{"line_number":163,"context_line":"            return"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def _reconcile_lbaas(self, crd_id):"},{"line_number":166,"context_line":"        if crd_id:"}],"source_content_type":"text/x-python","patch_set":5,"id":"147d1b27_6e63f583","line":163,"range":{"start_line":157,"start_character":1,"end_line":163,"end_character":18},"updated":"2021-07-02 06:53:49.000000000","message":"this looks wrong. Idea is that either there is CRDS to reconcile (if on line 153), or there aren\u0027t. In the case there is not, a simple LOG.debug message saying something like KuryrLoadBalancer CRDs already in sync should be enough","commit_id":"c943b973e562603cb15893f62ffb84bc76a79a61"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"b2413c2b1782078311a5ef338e0bd638bc9e30fd","unresolved":true,"context_lines":[{"line_number":163,"context_line":"            return"},{"line_number":164,"context_line":""},{"line_number":165,"context_line":"    def _reconcile_lbaas(self, crd_id):"},{"line_number":166,"context_line":"        if crd_id:"},{"line_number":167,"context_line":"            pass"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"    def _should_ignore(self, loadbalancer_crd):"},{"line_number":170,"context_line":"        return (not(self._has_endpoints(loadbalancer_crd) or"}],"source_content_type":"text/x-python","patch_set":5,"id":"879a6c0c_90893d64","line":167,"range":{"start_line":166,"start_character":0,"end_line":167,"end_character":16},"updated":"2021-07-02 06:53:49.000000000","message":"waiting to see the implementation of this function! :)","commit_id":"c943b973e562603cb15893f62ffb84bc76a79a61"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"bccbe00a3c688d83a581543d20a124642b5880bb","unresolved":true,"context_lines":[{"line_number":149,"context_line":"        # OpenStack"},{"line_number":150,"context_line":"        crds_to_reconcile \u003d [lb_id for lb_id in loadbalancer_crds_id if"},{"line_number":151,"context_line":"                             lb_id not in loadbalancers_id]"},{"line_number":152,"context_line":"        if crds_to_reconcile:"},{"line_number":153,"context_line":"             LOG.debug(\"Reconciling the following KuryrLoadBalancer CRDs: %r\","},{"line_number":154,"context_line":"                       crds_to_reconcile)"},{"line_number":155,"context_line":"             self._reconcile_lbaas(crds_to_reconcile)"}],"source_content_type":"text/x-python","patch_set":6,"id":"15b1b5bb_d260d251","line":152,"range":{"start_line":152,"start_character":7,"end_line":152,"end_character":29},"updated":"2021-07-02 13:12:17.000000000","message":"I am thinking of looping with each crd_id here. Reason being if,crds_to_reconcile exists, it needs to loop through each crd_id before logging a message that all CRDs are now in  sync? What do you think?","commit_id":"2feff31671eb5886a5530c655807814975425fc1"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b0ae53d0600628fbc6818c0b44f6160475f2b429","unresolved":true,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import eventlet"},{"line_number":17,"context_line":"import time"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"from oslo_log import log as logging"}],"source_content_type":"text/x-python","patch_set":7,"id":"72632834_b53e82aa","line":16,"range":{"start_line":16,"start_character":7,"end_line":16,"end_character":15},"updated":"2021-07-02 16:28:13.000000000","message":"eventlet is a third-party lib, not a built-in like time. It should go into another section.","commit_id":"bec38211099399eb5349fa96f28a23f68ce667ab"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b0ae53d0600628fbc6818c0b44f6160475f2b429","unresolved":true,"context_lines":[{"line_number":123,"context_line":"                self._trigger_loadbalancer_reconciliation()"},{"line_number":124,"context_line":"            except Exception:"},{"line_number":125,"context_line":"                LOG.exception(\u0027Error while running loadbalancers \u0027"},{"line_number":126,"context_line":"                              \u0027reconciliation. It will be tried in 60s\u0027)"},{"line_number":127,"context_line":""},{"line_number":128,"context_line":"    def _trigger_loadbalancer_reconciliation(self):"},{"line_number":129,"context_line":"        LOG.debug(\"Reconciling the loadbalancer CRDs\")"}],"source_content_type":"text/x-python","patch_set":7,"id":"de63c9be_944c0454","line":126,"range":{"start_line":126,"start_character":67,"end_line":126,"end_character":70},"updated":"2021-07-02 16:28:13.000000000","message":"Not 600?","commit_id":"bec38211099399eb5349fa96f28a23f68ce667ab"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"b0ae53d0600628fbc6818c0b44f6160475f2b429","unresolved":true,"context_lines":[{"line_number":129,"context_line":"        LOG.debug(\"Reconciling the loadbalancer CRDs\")"},{"line_number":130,"context_line":"        loadbalancer_crds \u003d []"},{"line_number":131,"context_line":"        try:"},{"line_number":132,"context_line":"            loadbalancer_crds \u003d driver_utils.get_kuryrloadbalancer_crds()"},{"line_number":133,"context_line":"        except k_exc.K8sClientException:"},{"line_number":134,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":135,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":7,"id":"319fd5c6_dbedbd1a","line":132,"range":{"start_line":132,"start_character":45,"end_line":132,"end_character":71},"updated":"2021-07-02 16:28:13.000000000","message":"If ResourceNotReady gets raised from inside this method it\u0027ll kill the thread.","commit_id":"bec38211099399eb5349fa96f28a23f68ce667ab"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"399e624828b20c9571446fc14bd892c4afc129c5","unresolved":true,"context_lines":[{"line_number":148,"context_line":"                            for loadbalancer in loadbalancers]"},{"line_number":149,"context_line":"        # for each loadbalancer id in the CRD status, check if exists in"},{"line_number":150,"context_line":"        # OpenStack"},{"line_number":151,"context_line":"        crds_to_reconcile \u003d [lb_id for lb_id in loadbalancer_crds_id if"},{"line_number":152,"context_line":"                             lb_id not in loadbalancers_id]"},{"line_number":153,"context_line":"        if crds_to_reconcile:"},{"line_number":154,"context_line":"            LOG.debug(\"Reconciling the following KuryrLoadBalancer CRDs: %r\","},{"line_number":155,"context_line":"                      crds_to_reconcile)"}],"source_content_type":"text/x-python","patch_set":7,"id":"377c6848_0854a19a","line":152,"range":{"start_line":151,"start_character":0,"end_line":152,"end_character":59},"updated":"2021-07-02 15:11:26.000000000","message":"this is not what you want (I think), you want to get the crd name for the crd ids that do not have a corresponding loadbalancer (openstack) id. So, this is not the correct way of going through the list of tuples you created at line 137.","commit_id":"bec38211099399eb5349fa96f28a23f68ce667ab"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"4c4cd47ecdaf1690f1d4fcef0f4914829d4ef078","unresolved":true,"context_lines":[{"line_number":148,"context_line":"                            for loadbalancer in loadbalancers]"},{"line_number":149,"context_line":"        # for each loadbalancer id in the CRD status, check if exists in"},{"line_number":150,"context_line":"        # OpenStack"},{"line_number":151,"context_line":"        crds_to_reconcile \u003d [lb_id for lb_id in loadbalancer_crds_id if"},{"line_number":152,"context_line":"                             lb_id not in loadbalancers_id]"},{"line_number":153,"context_line":"        if crds_to_reconcile:"},{"line_number":154,"context_line":"            LOG.debug(\"Reconciling the following KuryrLoadBalancer CRDs: %r\","},{"line_number":155,"context_line":"                      crds_to_reconcile)"}],"source_content_type":"text/x-python","patch_set":7,"id":"1778f544_c5b7868d","line":152,"range":{"start_line":151,"start_character":0,"end_line":152,"end_character":59},"in_reply_to":"377c6848_0854a19a","updated":"2021-07-02 15:22:39.000000000","message":"\u003e this is not what you want (I think), you want to get the crd name for the crd ids that do not have a corresponding loadbalancer (openstack) id. So, this is not the correct way of going through the list of tuples you created at line 137.\n\nSure I would modify it. Right now it is giving me the tuple for missing CRD in OpenStack, so I will extract only the name and pass that in the next patchset.","commit_id":"bec38211099399eb5349fa96f28a23f68ce667ab"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"c45b32cbd4105d18e538238d5b0cc4accefb96cb","unresolved":true,"context_lines":[{"line_number":150,"context_line":"                            for loadbalancer in loadbalancers]"},{"line_number":151,"context_line":"        # for each loadbalancer id in the CRD status, check if exists in"},{"line_number":152,"context_line":"        # OpenStack"},{"line_number":153,"context_line":"        crds_name_to_reconcile \u003d [lb_id[1] for lb_id in loadbalancer_crds_id if"},{"line_number":154,"context_line":"                                  lb_id[0] not in loadbalancers_id]"},{"line_number":155,"context_line":"        crds_to_reconcile_selflink \u003d [lb_id[2] for lb_id in"},{"line_number":156,"context_line":"                                      loadbalancer_crds_id if"},{"line_number":157,"context_line":"                                      lb_id[0] not in loadbalancers_id]"},{"line_number":158,"context_line":"        LOG.debug(\"Reconciling the following KuryrLoadBalancer Crds: %r\","},{"line_number":159,"context_line":"                  crds_name_to_reconcile)"},{"line_number":160,"context_line":"        self._reconcile_lbaas(crds_to_reconcile_selflink)"}],"source_content_type":"text/x-python","patch_set":9,"id":"0f731800_c5cfea53","line":157,"range":{"start_line":153,"start_character":0,"end_line":157,"end_character":71},"updated":"2021-07-05 08:14:52.000000000","message":"this could be merged into one dict or something similar to avoid going over the loop twice","commit_id":"91f4587bfac2dd5285ab79917c973e4ef6d1e215"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"040368a9daf7123616cbac1b04fc801614d4bcb5","unresolved":true,"context_lines":[{"line_number":150,"context_line":"                            for loadbalancer in loadbalancers]"},{"line_number":151,"context_line":"        # for each loadbalancer id in the CRD status, check if exists in"},{"line_number":152,"context_line":"        # OpenStack"},{"line_number":153,"context_line":"        crds_name_to_reconcile \u003d [lb_id[1] for lb_id in loadbalancer_crds_id if"},{"line_number":154,"context_line":"                                  lb_id[0] not in loadbalancers_id]"},{"line_number":155,"context_line":"        crds_to_reconcile_selflink \u003d [lb_id[2] for lb_id in"},{"line_number":156,"context_line":"                                      loadbalancer_crds_id if"},{"line_number":157,"context_line":"                                      lb_id[0] not in loadbalancers_id]"},{"line_number":158,"context_line":"        LOG.debug(\"Reconciling the following KuryrLoadBalancer Crds: %r\","},{"line_number":159,"context_line":"                  crds_name_to_reconcile)"},{"line_number":160,"context_line":"        self._reconcile_lbaas(crds_to_reconcile_selflink)"}],"source_content_type":"text/x-python","patch_set":9,"id":"c652c132_74ecf8f2","line":157,"range":{"start_line":153,"start_character":0,"end_line":157,"end_character":71},"in_reply_to":"0f731800_c5cfea53","updated":"2021-07-05 08:35:31.000000000","message":"I don\u0027t think I even need the name for any major purpose. I should remove it in the next patch?","commit_id":"91f4587bfac2dd5285ab79917c973e4ef6d1e215"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"9d6231c5c8d63213e1d592f02f54ac569205b0b7","unresolved":true,"context_lines":[{"line_number":150,"context_line":"                            for loadbalancer in loadbalancers]"},{"line_number":151,"context_line":"        # for each loadbalancer id in the CRD status, check if exists in"},{"line_number":152,"context_line":"        # OpenStack"},{"line_number":153,"context_line":"        crds_name_to_reconcile \u003d [lb_id[1] for lb_id in loadbalancer_crds_id if"},{"line_number":154,"context_line":"                                  lb_id[0] not in loadbalancers_id]"},{"line_number":155,"context_line":"        crds_to_reconcile_selflink \u003d [lb_id[2] for lb_id in"},{"line_number":156,"context_line":"                                      loadbalancer_crds_id if"},{"line_number":157,"context_line":"                                      lb_id[0] not in loadbalancers_id]"},{"line_number":158,"context_line":"        LOG.debug(\"Reconciling the following KuryrLoadBalancer Crds: %r\","},{"line_number":159,"context_line":"                  crds_name_to_reconcile)"},{"line_number":160,"context_line":"        self._reconcile_lbaas(crds_to_reconcile_selflink)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7ffeb7b2_042a220a","line":157,"range":{"start_line":153,"start_character":0,"end_line":157,"end_character":71},"in_reply_to":"c652c132_74ecf8f2","updated":"2021-07-07 09:15:10.000000000","message":"+1, the list of CRDs name seems not needed.","commit_id":"91f4587bfac2dd5285ab79917c973e4ef6d1e215"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"c45b32cbd4105d18e538238d5b0cc4accefb96cb","unresolved":true,"context_lines":[{"line_number":158,"context_line":"        LOG.debug(\"Reconciling the following KuryrLoadBalancer Crds: %r\","},{"line_number":159,"context_line":"                  crds_name_to_reconcile)"},{"line_number":160,"context_line":"        self._reconcile_lbaas(crds_to_reconcile_selflink)"},{"line_number":161,"context_line":"        if not crds_name_to_reconcile:"},{"line_number":162,"context_line":"            LOG.debug(\"KuryrLoadBalancer CRDs already in sync\")"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"    def _reconcile_lbaas(self, crds_to_reconcile_selflink):"},{"line_number":165,"context_line":"        kubernetes \u003d clients.get_kubernetes_client()"}],"source_content_type":"text/x-python","patch_set":9,"id":"092f4238_f8bb973a","line":162,"range":{"start_line":161,"start_character":1,"end_line":162,"end_character":63},"updated":"2021-07-05 08:14:52.000000000","message":"this should be call befre calling the _reconcile_lbaas. There is no need to reconcile if the list is empty","commit_id":"91f4587bfac2dd5285ab79917c973e4ef6d1e215"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"16509c7b367e34b4143301d1010e8b02519898bc","unresolved":true,"context_lines":[{"line_number":136,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":137,"context_line":"            return"},{"line_number":138,"context_line":"        # get the loadbalancers id in the CRD status"},{"line_number":139,"context_line":"        loadbalancer_crds_id \u003d [(loadbalancer_crd.get(\u0027status\u0027, {}).get("},{"line_number":140,"context_line":"                                \u0027loadbalancer\u0027, {}).get(\u0027id\u0027, {}),"},{"line_number":141,"context_line":"                                loadbalancer_crd.get(\u0027metadata\u0027, {}).get("},{"line_number":142,"context_line":"                                \u0027name\u0027), utils.get_res_link(loadbalancer_crd)"}],"source_content_type":"text/x-python","patch_set":10,"id":"96a6ad66_c27f6b5c","line":139,"range":{"start_line":139,"start_character":8,"end_line":139,"end_character":29},"updated":"2021-07-08 06:41:14.000000000","message":"crd_loadbalancer_ids?","commit_id":"062ade6efd134efe18f44786c0a78e011b8d19bc"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"16509c7b367e34b4143301d1010e8b02519898bc","unresolved":true,"context_lines":[{"line_number":138,"context_line":"        # get the loadbalancers id in the CRD status"},{"line_number":139,"context_line":"        loadbalancer_crds_id \u003d [(loadbalancer_crd.get(\u0027status\u0027, {}).get("},{"line_number":140,"context_line":"                                \u0027loadbalancer\u0027, {}).get(\u0027id\u0027, {}),"},{"line_number":141,"context_line":"                                loadbalancer_crd.get(\u0027metadata\u0027, {}).get("},{"line_number":142,"context_line":"                                \u0027name\u0027), utils.get_res_link(loadbalancer_crd)"},{"line_number":143,"context_line":"                                ) for loadbalancer_crd in"},{"line_number":144,"context_line":"                                loadbalancer_crds]"},{"line_number":145,"context_line":"        lbaas \u003d clients.get_loadbalancer_client()"}],"source_content_type":"text/x-python","patch_set":10,"id":"c1f96072_eb128c7d","line":142,"range":{"start_line":141,"start_character":32,"end_line":142,"end_character":77},"updated":"2021-07-08 06:41:14.000000000","message":"seems you are not using the name, so it can be removed","commit_id":"062ade6efd134efe18f44786c0a78e011b8d19bc"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"16509c7b367e34b4143301d1010e8b02519898bc","unresolved":true,"context_lines":[{"line_number":154,"context_line":"        crds_to_reconcile_selflink \u003d [lb_id[2] for lb_id in"},{"line_number":155,"context_line":"                                      loadbalancer_crds_id if"},{"line_number":156,"context_line":"                                      lb_id[0] not in loadbalancers_id]"},{"line_number":157,"context_line":"        if not crds_to_reconcile_selflink:"},{"line_number":158,"context_line":"            LOG.debug(\"KuryrLoadBalancer CRDs already in sync\")"},{"line_number":159,"context_line":"        LOG.debug(\"Reconciling the following KuryrLoadBalancer Crds: %r\","},{"line_number":160,"context_line":"                  crds_to_reconcile_selflink)"},{"line_number":161,"context_line":"        self._reconcile_lbaas(crds_to_reconcile_selflink)"}],"source_content_type":"text/x-python","patch_set":10,"id":"8f4eb164_17e86ee0","line":158,"range":{"start_line":157,"start_character":0,"end_line":158,"end_character":63},"updated":"2021-07-08 06:41:14.000000000","message":"this should have a return to not process the rest","commit_id":"062ade6efd134efe18f44786c0a78e011b8d19bc"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"16509c7b367e34b4143301d1010e8b02519898bc","unresolved":true,"context_lines":[{"line_number":165,"context_line":"        for selflink in crds_to_reconcile_selflink:"},{"line_number":166,"context_line":"            try:"},{"line_number":167,"context_line":"                kubernetes.patch_crd(\u0027status\u0027, selflink, {})"},{"line_number":168,"context_line":"            except k_exc.K8sResourceNotFound:"},{"line_number":169,"context_line":"                LOG.debug(\u0027Unable to reconcile the KuryLoadBalancer CRD %s\u0027,"},{"line_number":170,"context_line":"                          selflink)"},{"line_number":171,"context_line":"                return"}],"source_content_type":"text/x-python","patch_set":10,"id":"7722aca0_4b08898b","line":168,"range":{"start_line":168,"start_character":12,"end_line":168,"end_character":45},"updated":"2021-07-08 06:41:14.000000000","message":"what if there is other type of exception? I think at least K8sClientException should be handled too","commit_id":"062ade6efd134efe18f44786c0a78e011b8d19bc"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"7fd486e6a4066077d91380069a5eb46fc7adc7f9","unresolved":true,"context_lines":[{"line_number":134,"context_line":"        name \u003d \u0027KuryrLoadBalancers\u0027"},{"line_number":135,"context_line":"        klb_path \u003d k_const.K8S_API_CRD_KURYRLOADBALANCERS"},{"line_number":136,"context_line":"        try:"},{"line_number":137,"context_line":"            loadbalancer_crds \u003d driver_utils.get_kuryrk8s_resource(name,"},{"line_number":138,"context_line":"                                                                   klb_path)"},{"line_number":139,"context_line":"        except k_exc.K8sClientException:"},{"line_number":140,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"}],"source_content_type":"text/x-python","patch_set":12,"id":"4aec6b0d_768698b2","line":137,"updated":"2021-07-12 08:59:04.000000000","message":"This same function is called at line 172 also for KuryrLoadBalancers. They can be moved to outside (line 122) and pass the return as parameter of the trigger_* functions, avoiding to call k8s API twice.","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"aec8e0c6dc6b8b8bbddc50c20af9d269d0afa227","unresolved":true,"context_lines":[{"line_number":175,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":176,"context_line":"            return"},{"line_number":177,"context_line":"        # get the listener id in the CRD status"},{"line_number":178,"context_line":"        crd_lsnrs_id \u003d [list((l[\u0027id\u0027], utils.get_res_link(loadbalancer_crd)))"},{"line_number":179,"context_line":"                        for loadbalancer_crd in loadbalancer_crds for l in"},{"line_number":180,"context_line":"                        loadbalancer_crd.get(\u0027status\u0027, {}).get("},{"line_number":181,"context_line":"                        \u0027listeners\u0027, [])]"}],"source_content_type":"text/x-python","patch_set":12,"id":"69298ff3_c844a1d9","line":178,"range":{"start_line":178,"start_character":24,"end_line":178,"end_character":76},"updated":"2021-07-12 07:32:35.000000000","message":"perhaps this could be a dict instead, same for the loadbalancer reconciliation function","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"7fd486e6a4066077d91380069a5eb46fc7adc7f9","unresolved":true,"context_lines":[{"line_number":175,"context_line":"            LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":176,"context_line":"            return"},{"line_number":177,"context_line":"        # get the listener id in the CRD status"},{"line_number":178,"context_line":"        crd_lsnrs_id \u003d [list((l[\u0027id\u0027], utils.get_res_link(loadbalancer_crd)))"},{"line_number":179,"context_line":"                        for loadbalancer_crd in loadbalancer_crds for l in"},{"line_number":180,"context_line":"                        loadbalancer_crd.get(\u0027status\u0027, {}).get("},{"line_number":181,"context_line":"                        \u0027listeners\u0027, [])]"}],"source_content_type":"text/x-python","patch_set":12,"id":"8b881d9d_a4667bfc","line":178,"range":{"start_line":178,"start_character":24,"end_line":178,"end_character":76},"in_reply_to":"69298ff3_c844a1d9","updated":"2021-07-12 08:59:04.000000000","message":"+1","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"aec8e0c6dc6b8b8bbddc50c20af9d269d0afa227","unresolved":true,"context_lines":[{"line_number":183,"context_line":"        lbaas_spec \u003d {}"},{"line_number":184,"context_line":"        self._drv_lbaas.add_tags(\u0027listener\u0027, lbaas_spec)"},{"line_number":185,"context_line":"        listeners \u003d lbaas.listeners(**lbaas_spec)"},{"line_number":186,"context_line":"        listeners_id \u003d list(lsnr[\u0027id\u0027] for lsnr in listeners)"},{"line_number":187,"context_line":"        crds_to_reconcile_selflink \u003d [lsnr[1] for lsnr in crd_lsnrs_id"},{"line_number":188,"context_line":"                                      if lsnr[0] not in listeners_id]"},{"line_number":189,"context_line":"        if not crds_to_reconcile_selflink:"},{"line_number":190,"context_line":"            LOG.debug(\"KuryrLoadBalancer CRDs already in sync\")"},{"line_number":191,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":12,"id":"a91889b5_2eb07743","line":188,"range":{"start_line":186,"start_character":1,"end_line":188,"end_character":69},"updated":"2021-07-12 07:32:35.000000000","message":"perhaps this needs a bit extra thinking/checking. It is not enough the listener exists... for example the listeners must be associated to the loadbalancer we expect.","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"d16420bdf4d85701a869cf7257b999facdc7cc40","unresolved":true,"context_lines":[{"line_number":183,"context_line":"        lbaas_spec \u003d {}"},{"line_number":184,"context_line":"        self._drv_lbaas.add_tags(\u0027listener\u0027, lbaas_spec)"},{"line_number":185,"context_line":"        listeners \u003d lbaas.listeners(**lbaas_spec)"},{"line_number":186,"context_line":"        listeners_id \u003d list(lsnr[\u0027id\u0027] for lsnr in listeners)"},{"line_number":187,"context_line":"        crds_to_reconcile_selflink \u003d [lsnr[1] for lsnr in crd_lsnrs_id"},{"line_number":188,"context_line":"                                      if lsnr[0] not in listeners_id]"},{"line_number":189,"context_line":"        if not crds_to_reconcile_selflink:"},{"line_number":190,"context_line":"            LOG.debug(\"KuryrLoadBalancer CRDs already in sync\")"},{"line_number":191,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":12,"id":"b75850b8_6fe24b6c","line":188,"range":{"start_line":186,"start_character":1,"end_line":188,"end_character":69},"in_reply_to":"a91889b5_2eb07743","updated":"2021-07-12 08:31:03.000000000","message":"\u003e perhaps this needs a bit extra thinking/checking. It is not enough the listener exists... for example the listeners must be associated to the loadbalancer we expect.\n\nI need to implement a check to confirm if the listener is associated with the right loadbalancer?","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":23567,"name":"Luis Tomas Bolivar","email":"ltomasbo@redhat.com","username":"ltomasbo"},"change_message_id":"aec8e0c6dc6b8b8bbddc50c20af9d269d0afa227","unresolved":true,"context_lines":[{"line_number":207,"context_line":"                          selflink)"},{"line_number":208,"context_line":"                return"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def _reconcile_listeners(self, crds_to_reconcile_selflink):"},{"line_number":211,"context_line":"        kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":212,"context_line":"        for selflink in crds_to_reconcile_selflink:"},{"line_number":213,"context_line":"            try:"},{"line_number":214,"context_line":"                kubernetes.patch_crd(\u0027status\u0027, selflink, {})"},{"line_number":215,"context_line":"            except k_exc.K8sResourceNotFound:"},{"line_number":216,"context_line":"                LOG.debug(\u0027Unable to reconcile the KuryLoadBalancer CRD %s\u0027,"},{"line_number":217,"context_line":"                          selflink)"},{"line_number":218,"context_line":"                return"},{"line_number":219,"context_line":"            except k_exc.K8sClientException:"},{"line_number":220,"context_line":"                LOG.debug(\u0027Unable fetch the KuryLoadBalancer CRD %s\u0027,"},{"line_number":221,"context_line":"                          selflink)"},{"line_number":222,"context_line":"                return"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def _should_ignore(self, loadbalancer_crd):"},{"line_number":225,"context_line":"        return (not(self._has_endpoints(loadbalancer_crd) or"}],"source_content_type":"text/x-python","patch_set":12,"id":"ccdddc5b_d94ee82e","line":222,"range":{"start_line":210,"start_character":1,"end_line":222,"end_character":22},"updated":"2021-07-12 07:32:35.000000000","message":"this is exactly the same as the previous function","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"7fd486e6a4066077d91380069a5eb46fc7adc7f9","unresolved":true,"context_lines":[{"line_number":207,"context_line":"                          selflink)"},{"line_number":208,"context_line":"                return"},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"    def _reconcile_listeners(self, crds_to_reconcile_selflink):"},{"line_number":211,"context_line":"        kubernetes \u003d clients.get_kubernetes_client()"},{"line_number":212,"context_line":"        for selflink in crds_to_reconcile_selflink:"},{"line_number":213,"context_line":"            try:"},{"line_number":214,"context_line":"                kubernetes.patch_crd(\u0027status\u0027, selflink, {})"},{"line_number":215,"context_line":"            except k_exc.K8sResourceNotFound:"},{"line_number":216,"context_line":"                LOG.debug(\u0027Unable to reconcile the KuryLoadBalancer CRD %s\u0027,"},{"line_number":217,"context_line":"                          selflink)"},{"line_number":218,"context_line":"                return"},{"line_number":219,"context_line":"            except k_exc.K8sClientException:"},{"line_number":220,"context_line":"                LOG.debug(\u0027Unable fetch the KuryLoadBalancer CRD %s\u0027,"},{"line_number":221,"context_line":"                          selflink)"},{"line_number":222,"context_line":"                return"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"    def _should_ignore(self, loadbalancer_crd):"},{"line_number":225,"context_line":"        return (not(self._has_endpoints(loadbalancer_crd) or"}],"source_content_type":"text/x-python","patch_set":12,"id":"70c725a2_e77ac18d","line":222,"range":{"start_line":210,"start_character":1,"end_line":222,"end_character":22},"in_reply_to":"ccdddc5b_d94ee82e","updated":"2021-07-12 08:59:04.000000000","message":"+1 no need to have this extra function","commit_id":"7ecb3e2ba2eabd65bcd564d5a438de97b20c9b3a"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5b3a5a627ff19925c03ca03b19e299548865dc94","unresolved":true,"context_lines":[{"line_number":123,"context_line":"            loadbalancer_crds \u003d []"},{"line_number":124,"context_line":"            try:"},{"line_number":125,"context_line":"                loadbalancer_crds \u003d driver_utils.get_kuryrloadbalancer_crds()"},{"line_number":126,"context_line":"            except k_exc.K8sClientException:"},{"line_number":127,"context_line":"                LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":128,"context_line":"                return"},{"line_number":129,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":14,"id":"f46c2fdb_0e2b7acd","line":126,"range":{"start_line":126,"start_character":25,"end_line":126,"end_character":43},"updated":"2021-07-13 09:53:58.000000000","message":"this exception will never be raised as you\u0027re raising ResourceNotReady on get_kuryrloadbalancer_crds when it happen. Might be better to not raise ResourceNotReady.","commit_id":"ec85a19a058c68f099b7ef2ed33d3097381e629f"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5b3a5a627ff19925c03ca03b19e299548865dc94","unresolved":true,"context_lines":[{"line_number":136,"context_line":"    def _trigger_loadbalancer_reconciliation(self, loadbalancer_crds):"},{"line_number":137,"context_line":"        LOG.debug(\"Reconciling the loadbalancer CRDs\")"},{"line_number":138,"context_line":"        # get the loadbalancers id in the CRD status"},{"line_number":139,"context_line":"        crd_loadbalancer_ids \u003d [list((loadbalancer_crd.get(\u0027status\u0027, {}).get("},{"line_number":140,"context_line":"                                \u0027loadbalancer\u0027, {}).get(\u0027id\u0027, {}),"},{"line_number":141,"context_line":"                                utils.get_res_link(loadbalancer_crd))) for"},{"line_number":142,"context_line":"                                loadbalancer_crd in loadbalancer_crds]"},{"line_number":143,"context_line":"        lbaas \u003d clients.get_loadbalancer_client()"},{"line_number":144,"context_line":"        lbaas_spec \u003d {}"}],"source_content_type":"text/x-python","patch_set":14,"id":"343d7a04_2c1916dc","line":141,"range":{"start_line":139,"start_character":32,"end_line":141,"end_character":70},"updated":"2021-07-13 09:53:58.000000000","message":"could this be converted to a dict instead?","commit_id":"ec85a19a058c68f099b7ef2ed33d3097381e629f"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"5b3a5a627ff19925c03ca03b19e299548865dc94","unresolved":true,"context_lines":[{"line_number":153,"context_line":"                                      crd_loadbalancer_ids if"},{"line_number":154,"context_line":"                                      lb_id[0] not in loadbalancers_id]"},{"line_number":155,"context_line":"        if not crds_to_reconcile_selflink:"},{"line_number":156,"context_line":"            LOG.debug(\"KuryrLoadBalancer CRDs already in sync\")"},{"line_number":157,"context_line":"            return"},{"line_number":158,"context_line":"        LOG.debug(\"Reconciling the following KuryrLoadBalancer Crds: %r\","},{"line_number":159,"context_line":"                  crds_to_reconcile_selflink)"}],"source_content_type":"text/x-python","patch_set":14,"id":"bdda20b1_5b4ea9f1","line":156,"range":{"start_line":156,"start_character":57,"end_line":156,"end_character":61},"updated":"2021-07-13 09:53:58.000000000","message":"sync with OpenStack","commit_id":"ec85a19a058c68f099b7ef2ed33d3097381e629f"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"33d82192b8c3ccbd530db1c1791eff98b94ebfb5","unresolved":true,"context_lines":[{"line_number":13,"context_line":"#    License for the specific language governing permissions and limitations"},{"line_number":14,"context_line":"#    under the License."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import eventlet"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import time"},{"line_number":19,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"77a5e76b_83296be7","line":16,"range":{"start_line":16,"start_character":0,"end_line":16,"end_character":15},"updated":"2021-08-03 13:14:50.000000000","message":"eventlet is a third-party lib, should be placed either with oslo_log or in a new section between time and oslo_log [1].\n\n[1] https://docs.openstack.org/hacking/latest/user/hacking.html#import-order-template","commit_id":"7f2c925a51794558c00501bfb8b39a13f158c545"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"33d82192b8c3ccbd530db1c1791eff98b94ebfb5","unresolved":true,"context_lines":[{"line_number":125,"context_line":"                loadbalancer_crds \u003d driver_utils.get_kuryrloadbalancer_crds()"},{"line_number":126,"context_line":"            except k_exc.K8sClientException:"},{"line_number":127,"context_line":"                LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":128,"context_line":"                return"},{"line_number":129,"context_line":"            try:"},{"line_number":130,"context_line":"                self._trigger_loadbalancer_reconciliation(loadbalancer_crds)"},{"line_number":131,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":26,"id":"381ace1b_c626bd80","line":128,"range":{"start_line":128,"start_character":0,"end_line":128,"end_character":22},"updated":"2021-08-03 13:14:50.000000000","message":"This means that on K8s API error the thread will be quietly stopped and never executed until all kuryr-controller is restarted.","commit_id":"7f2c925a51794558c00501bfb8b39a13f158c545"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"2aae7039e9d6966eadfc607c5b246a2b42df8708","unresolved":true,"context_lines":[{"line_number":125,"context_line":"                loadbalancer_crds \u003d driver_utils.get_kuryrloadbalancer_crds()"},{"line_number":126,"context_line":"            except k_exc.K8sClientException:"},{"line_number":127,"context_line":"                LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":128,"context_line":"                return"},{"line_number":129,"context_line":"            try:"},{"line_number":130,"context_line":"                self._trigger_loadbalancer_reconciliation(loadbalancer_crds)"},{"line_number":131,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":26,"id":"382ddef1_f9b1d902","line":128,"range":{"start_line":128,"start_character":0,"end_line":128,"end_character":22},"in_reply_to":"1a1bad05_8e9a3923","updated":"2021-08-10 10:45:03.000000000","message":"Yes, no need to return it.","commit_id":"7f2c925a51794558c00501bfb8b39a13f158c545"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"039563bea5ad04b9cc1e162d5e03489b270cac90","unresolved":true,"context_lines":[{"line_number":125,"context_line":"                loadbalancer_crds \u003d driver_utils.get_kuryrloadbalancer_crds()"},{"line_number":126,"context_line":"            except k_exc.K8sClientException:"},{"line_number":127,"context_line":"                LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":128,"context_line":"                return"},{"line_number":129,"context_line":"            try:"},{"line_number":130,"context_line":"                self._trigger_loadbalancer_reconciliation(loadbalancer_crds)"},{"line_number":131,"context_line":"            except Exception:"}],"source_content_type":"text/x-python","patch_set":26,"id":"1a1bad05_8e9a3923","line":128,"range":{"start_line":128,"start_character":0,"end_line":128,"end_character":22},"in_reply_to":"381ace1b_c626bd80","updated":"2021-08-04 14:01:10.000000000","message":"\u003e This means that on K8s API error the thread will be quietly stopped and never executed until all kuryr-controller is restarted.\n\nThanks Michal.\nSo I need to allow the thread to proceed somehow to avoid needing to restart the kuryr-controller?","commit_id":"7f2c925a51794558c00501bfb8b39a13f158c545"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"33d82192b8c3ccbd530db1c1791eff98b94ebfb5","unresolved":true,"context_lines":[{"line_number":117,"context_line":"                    self._update_lb_status(loadbalancer_crd)"},{"line_number":118,"context_line":"                    self._patch_status(loadbalancer_crd)"},{"line_number":119,"context_line":""},{"line_number":120,"context_line":"    def _reconcile_loadbalancers(self):"},{"line_number":121,"context_line":"        while True:"},{"line_number":122,"context_line":"            eventlet.sleep(CRD_RECONCILIATION_FREQUENCY)"},{"line_number":123,"context_line":"            loadbalancer_crds \u003d []"},{"line_number":124,"context_line":"            try:"},{"line_number":125,"context_line":"                loadbalancer_crds \u003d driver_utils.get_kuryrloadbalancer_crds()"},{"line_number":126,"context_line":"            except k_exc.K8sClientException:"},{"line_number":127,"context_line":"                LOG.debug(\"Error retriving KuryrLoadBalanders CRDs\")"},{"line_number":128,"context_line":"                return"},{"line_number":129,"context_line":"            try:"},{"line_number":130,"context_line":"                self._trigger_loadbalancer_reconciliation(loadbalancer_crds)"},{"line_number":131,"context_line":"            except Exception:"},{"line_number":132,"context_line":"                LOG.exception(\u0027Error while running loadbalancers \u0027"},{"line_number":133,"context_line":"                              \u0027reconciliation. It will be retried in %s\u0027,"},{"line_number":134,"context_line":"                              CRD_RECONCILIATION_FREQUENCY)"},{"line_number":135,"context_line":""},{"line_number":136,"context_line":"    def _trigger_loadbalancer_reconciliation(self, loadbalancer_crds):"},{"line_number":137,"context_line":"        LOG.debug(\"Reconciling the loadbalancer CRDs\")"}],"source_content_type":"text/x-python","patch_set":26,"id":"59ad7be7_5785aa53","line":134,"range":{"start_line":120,"start_character":0,"end_line":134,"end_character":59},"updated":"2021-08-03 13:14:50.000000000","message":"Looking at this design now, it would probably be better to make this a periodic task at KuryrK8sService [1] that would iterate over all the handlers [2] running some method like \"check_resources()\". Obviously for most handlers it would be just `pass`.\n\n[1] https://github.com/openstack/kuryr-kubernetes/blob/9553f78f9aadf37ce8e1cbaf74f9d82d27941be7/kuryr_kubernetes/controller/service.py#L76-L77\n[2] https://github.com/openstack/kuryr-kubernetes/blob/9553f78f9aadf37ce8e1cbaf74f9d82d27941be7/kuryr_kubernetes/controller/service.py#L92-L94","commit_id":"7f2c925a51794558c00501bfb8b39a13f158c545"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"33d82192b8c3ccbd530db1c1791eff98b94ebfb5","unresolved":true,"context_lines":[{"line_number":142,"context_line":"                                loadbalancer_crd in loadbalancer_crds]"},{"line_number":143,"context_line":"        lbaas \u003d clients.get_loadbalancer_client()"},{"line_number":144,"context_line":"        lbaas_spec \u003d {}"},{"line_number":145,"context_line":"        self._drv_lbaas.add_tags(\u0027loadbalancer\u0027, lbaas_spec)"},{"line_number":146,"context_line":"        loadbalancers \u003d lbaas.load_balancers(**lbaas_spec)"},{"line_number":147,"context_line":"        # get the Loadbalaancer IDs from Openstack"},{"line_number":148,"context_line":"        loadbalancers_id \u003d [loadbalancer[\u0027id\u0027]"},{"line_number":149,"context_line":"                            for loadbalancer in loadbalancers]"}],"source_content_type":"text/x-python","patch_set":26,"id":"a63ffb3a_4e77d986","line":146,"range":{"start_line":145,"start_character":0,"end_line":146,"end_character":58},"updated":"2021-08-03 13:14:50.000000000","message":"There\u0027s a race condition - it\u0027s possible that LB will get created in the time between this GET from API and iteration. But Kuryr should still be able to discover it and recreate `status` of the KLB, right?","commit_id":"7f2c925a51794558c00501bfb8b39a13f158c545"},{"author":{"_account_id":11600,"name":"Michał Dulko","email":"michal.dulko@gmail.com","username":"dulek"},"change_message_id":"33d82192b8c3ccbd530db1c1791eff98b94ebfb5","unresolved":true,"context_lines":[{"line_number":169,"context_line":"                          selflink)"},{"line_number":170,"context_line":"                return"},{"line_number":171,"context_line":"            except k_exc.K8sClientException:"},{"line_number":172,"context_line":"                LOG.debug(\u0027Unable fetch the KuryLoadBalancer CRD %s\u0027,"},{"line_number":173,"context_line":"                          selflink)"},{"line_number":174,"context_line":"                return"},{"line_number":175,"context_line":""}],"source_content_type":"text/x-python","patch_set":26,"id":"bdee85d9_5f864efd","line":172,"range":{"start_line":172,"start_character":20,"end_line":172,"end_character":25},"updated":"2021-08-03 13:14:50.000000000","message":"This one should probably be a warning.","commit_id":"7f2c925a51794558c00501bfb8b39a13f158c545"}],"kuryr_kubernetes/tests/unit/controller/handlers/test_loadbalancer.py":[{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bec9df172b9622b3c5825352f73377f2790ac89c","unresolved":true,"context_lines":[{"line_number":137,"context_line":"        }"},{"line_number":138,"context_line":""},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"def get_lb_crds():"},{"line_number":141,"context_line":"    return [{"},{"line_number":142,"context_line":"            \u0027apiVersion\u0027: \u0027openstack.org/v1\u0027,"},{"line_number":143,"context_line":"            \u0027kind\u0027: \u0027KuryrLoadBalancer\u0027,"}],"source_content_type":"text/x-python","patch_set":20,"id":"e058df18_62708539","line":140,"range":{"start_line":140,"start_character":4,"end_line":140,"end_character":15},"updated":"2021-07-19 08:55:51.000000000","message":"Lines that are not used on the unit test could be removed, like listeners, pools, members, some on the spec and a few on metadata","commit_id":"f8e8aba14e6a4eb3364ea9836b4fb1889c4c20b2"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bec9df172b9622b3c5825352f73377f2790ac89c","unresolved":true,"context_lines":[{"line_number":815,"context_line":"    @mock.patch(\u0027kuryr_kubernetes.controller.drivers.utils\u0027"},{"line_number":816,"context_line":"                \u0027.get_kuryrloadbalancer_crds\u0027)"},{"line_number":817,"context_line":"    @mock.patch(\u0027kuryr_kubernetes.utils.get_res_link\u0027)"},{"line_number":818,"context_line":"    def test_reconcile_loadbalancers(self, m_get_res_link,"},{"line_number":819,"context_line":"                                     m_get_kuryrloadbalancer_crds, m_k8s):"},{"line_number":820,"context_line":"        loadbalancer_crds \u003d get_lb_crds()"},{"line_number":821,"context_line":"        m_get_kuryrloadbalancer_crds.return_value \u003d loadbalancer_crds"}],"source_content_type":"text/x-python","patch_set":20,"id":"151bef16_acffc6fb","line":818,"updated":"2021-07-19 08:55:51.000000000","message":"You need to call the function _reconcile_loadbalancers, otherwise the test won\u0027t exercise any corner cases on that function, similar to how is done at line 808.","commit_id":"f8e8aba14e6a4eb3364ea9836b4fb1889c4c20b2"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"bec9df172b9622b3c5825352f73377f2790ac89c","unresolved":true,"context_lines":[{"line_number":831,"context_line":"                   {}).get(\u0027id\u0027) for loadbalancer_crd in loadbalancer_crds]"},{"line_number":832,"context_line":"        crd_loadbalancer_ids \u003d [{\u0027id\u0027: crds_id[0], \u0027selflink\u0027: selflink[0]},"},{"line_number":833,"context_line":"                                {\u0027id\u0027: crds_id[1], \u0027selflink\u0027: selflink[1]}]"},{"line_number":834,"context_line":"        loadbalancers \u003d get_openstack_loadbalancers()"},{"line_number":835,"context_line":"        m_k8s.load_balancers.return_value \u003d loadbalancers"},{"line_number":836,"context_line":"        loadbalancers_id \u003d [loadbalancer[\u0027id\u0027]"},{"line_number":837,"context_line":"                            for loadbalancer in loadbalancers]"}],"source_content_type":"text/x-python","patch_set":20,"id":"b4582484_d771d56e","line":834,"updated":"2021-07-19 08:55:51.000000000","message":"You can a fixture Kuryr provides to access the octavia client and consequently octavia resources\nhttps://github.com/openstack/kuryr-kubernetes/blob/master/kuryr_kubernetes/tests/unit/controller/drivers/test_lbaasv2.py#L489","commit_id":"f8e8aba14e6a4eb3364ea9836b4fb1889c4c20b2"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"35a41a10e39fcd4fb199e5a83c96670896ed3415","unresolved":true,"context_lines":[{"line_number":649,"context_line":"    def test_reconcile_loadbalancers(self, m_get_drv_lbaas, m_get_res_link,"},{"line_number":650,"context_line":"                                     m_get_kuryrloadbalancer_crds, m_k8s):"},{"line_number":651,"context_line":"        loadbalancer_crds \u003d get_lb_crds()"},{"line_number":652,"context_line":"        m_get_kuryrloadbalancer_crds.return_value \u003d loadbalancer_crds"},{"line_number":653,"context_line":"        h \u003d h_lb.KuryrLoadBalancerHandler()"},{"line_number":654,"context_line":"        lbaas \u003d self.useFixture(k_fix.MockLBaaSClient()).client"},{"line_number":655,"context_line":"        loadbalancers \u003d get_openstack_loadbalancers()"}],"source_content_type":"text/x-python","patch_set":21,"id":"330682bd_3c80df34","line":652,"range":{"start_line":652,"start_character":8,"end_line":652,"end_character":69},"updated":"2021-07-21 09:13:40.000000000","message":"as you\u0027re testing the _trigger_loadbalancer_reconciliation method and not _reconcile_loadbalancers there is no need to mock get_kuryrloadbalancer_crds and define the return_value.","commit_id":"e302c45beddf91c89fae86df450892d321eb0da3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"35a41a10e39fcd4fb199e5a83c96670896ed3415","unresolved":true,"context_lines":[{"line_number":652,"context_line":"        m_get_kuryrloadbalancer_crds.return_value \u003d loadbalancer_crds"},{"line_number":653,"context_line":"        h \u003d h_lb.KuryrLoadBalancerHandler()"},{"line_number":654,"context_line":"        lbaas \u003d self.useFixture(k_fix.MockLBaaSClient()).client"},{"line_number":655,"context_line":"        loadbalancers \u003d get_openstack_loadbalancers()"},{"line_number":656,"context_line":"        lbaas.load_balancers.return_value \u003d loadbalancers"},{"line_number":657,"context_line":""},{"line_number":658,"context_line":"        h._trigger_loadbalancer_reconciliation(loadbalancer_crds)"}],"source_content_type":"text/x-python","patch_set":21,"id":"b24831e6_e023c474","line":655,"range":{"start_line":655,"start_character":24,"end_line":655,"end_character":51},"updated":"2021-07-21 09:13:40.000000000","message":"no need to create a method to return empty list and assign to a variable, instead you could define directly [] as the return_value of lbaas.load_balancers call","commit_id":"e302c45beddf91c89fae86df450892d321eb0da3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"35a41a10e39fcd4fb199e5a83c96670896ed3415","unresolved":true,"context_lines":[{"line_number":659,"context_line":"        self._trigger_lb_recon_impl(m_get_res_link, m_k8s, loadbalancer_crds)"},{"line_number":660,"context_line":""},{"line_number":661,"context_line":"    def _trigger_lb_recon_impl(self, m_get_res_link, m_k8s, loadbalancer_crds):"},{"line_number":662,"context_line":"        selflink \u003d [\u0027/apis/openstack.org/v1/namespaces/default/\u0027"},{"line_number":663,"context_line":"                    \u0027kuryrloadbalancers/test\u0027,"},{"line_number":664,"context_line":"                    \u0027/apis/openstack.org/v1/namespaces/default/\u0027"},{"line_number":665,"context_line":"                    \u0027kuryrloadbalancers/demo\u0027]"},{"line_number":666,"context_line":"        m_get_res_link.return_value \u003d selflink"},{"line_number":667,"context_line":"        crds_id \u003d [loadbalancer_crd.get(\u0027status\u0027, {}).get(\u0027loadbalancer\u0027,"},{"line_number":668,"context_line":"                   {}).get(\u0027id\u0027) for loadbalancer_crd in loadbalancer_crds]"}],"source_content_type":"text/x-python","patch_set":21,"id":"19143f30_19fe09cb","line":665,"range":{"start_line":662,"start_character":0,"end_line":665,"end_character":46},"updated":"2021-07-21 09:13:40.000000000","message":"the CRDs defined on get_lb_crds are missing the name and namespace in order to better align the return_value of m_get_res_link","commit_id":"e302c45beddf91c89fae86df450892d321eb0da3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"35a41a10e39fcd4fb199e5a83c96670896ed3415","unresolved":true,"context_lines":[{"line_number":668,"context_line":"                   {}).get(\u0027id\u0027) for loadbalancer_crd in loadbalancer_crds]"},{"line_number":669,"context_line":"        crd_loadbalancer_ids \u003d [{\u0027id\u0027: crds_id[0], \u0027selflink\u0027: selflink[0]},"},{"line_number":670,"context_line":"                                {\u0027id\u0027: crds_id[1], \u0027selflink\u0027: selflink[1]}]"},{"line_number":671,"context_line":"        loadbalancers \u003d get_openstack_loadbalancers()"},{"line_number":672,"context_line":"        m_k8s.load_balancers.return_value \u003d loadbalancers"},{"line_number":673,"context_line":"        loadbalancers_id \u003d [loadbalancer[\u0027id\u0027]"},{"line_number":674,"context_line":"                            for loadbalancer in loadbalancers]"},{"line_number":675,"context_line":"        crds_to_reconcile_selflink \u003d [crd_lb[\u0027selflink\u0027] for crd_lb in"}],"source_content_type":"text/x-python","patch_set":21,"id":"7500d93e_aa0b194f","line":672,"range":{"start_line":671,"start_character":0,"end_line":672,"end_character":57},"updated":"2021-07-21 09:13:40.000000000","message":"this was already defined at line 655, no need to repeat","commit_id":"e302c45beddf91c89fae86df450892d321eb0da3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"35a41a10e39fcd4fb199e5a83c96670896ed3415","unresolved":true,"context_lines":[{"line_number":672,"context_line":"        m_k8s.load_balancers.return_value \u003d loadbalancers"},{"line_number":673,"context_line":"        loadbalancers_id \u003d [loadbalancer[\u0027id\u0027]"},{"line_number":674,"context_line":"                            for loadbalancer in loadbalancers]"},{"line_number":675,"context_line":"        crds_to_reconcile_selflink \u003d [crd_lb[\u0027selflink\u0027] for crd_lb in"},{"line_number":676,"context_line":"                                      crd_loadbalancer_ids if"},{"line_number":677,"context_line":"                                      crd_lb[\u0027id\u0027] not in loadbalancers_id]"},{"line_number":678,"context_line":"        self._reconcile_lbaas_impl(crds_to_reconcile_selflink, m_k8s)"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    def _reconcile_lbaas_impl(self, crds_to_reconcile_selflink, m_k8s):"}],"source_content_type":"text/x-python","patch_set":21,"id":"f1003835_f2c9a5ad","line":677,"range":{"start_line":675,"start_character":0,"end_line":677,"end_character":75},"updated":"2021-07-21 09:13:40.000000000","message":"no need to calculate which crds need to be reconcile as you already know it, instead you can check that the _reconcile_lbaas method on the handler is called with the CRDs selflinks.","commit_id":"e302c45beddf91c89fae86df450892d321eb0da3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"35a41a10e39fcd4fb199e5a83c96670896ed3415","unresolved":true,"context_lines":[{"line_number":677,"context_line":"                                      crd_lb[\u0027id\u0027] not in loadbalancers_id]"},{"line_number":678,"context_line":"        self._reconcile_lbaas_impl(crds_to_reconcile_selflink, m_k8s)"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    def _reconcile_lbaas_impl(self, crds_to_reconcile_selflink, m_k8s):"},{"line_number":681,"context_line":"        for selflink in crds_to_reconcile_selflink:"},{"line_number":682,"context_line":"            m_k8s.patch_crd(\u0027status\u0027, selflink, {})"},{"line_number":683,"context_line":"        self.assertEqual(crds_to_reconcile_selflink,"}],"source_content_type":"text/x-python","patch_set":21,"id":"f549b2ea_06ddca78","line":680,"range":{"start_line":680,"start_character":8,"end_line":680,"end_character":29},"updated":"2021-07-21 09:13:40.000000000","message":"Can you combine all the methods at test_reconcile_loadbalancers? this is only testing _trigger_loadbalancer_reconciliation there is no need to split.","commit_id":"e302c45beddf91c89fae86df450892d321eb0da3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"35a41a10e39fcd4fb199e5a83c96670896ed3415","unresolved":true,"context_lines":[{"line_number":678,"context_line":"        self._reconcile_lbaas_impl(crds_to_reconcile_selflink, m_k8s)"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    def _reconcile_lbaas_impl(self, crds_to_reconcile_selflink, m_k8s):"},{"line_number":681,"context_line":"        for selflink in crds_to_reconcile_selflink:"},{"line_number":682,"context_line":"            m_k8s.patch_crd(\u0027status\u0027, selflink, {})"},{"line_number":683,"context_line":"        self.assertEqual(crds_to_reconcile_selflink,"},{"line_number":684,"context_line":"                         [\u0027/apis/openstack.org/v1/namespaces/\u0027"},{"line_number":685,"context_line":"                          \u0027default/kuryrloadbalancers/test\u0027,"}],"source_content_type":"text/x-python","patch_set":21,"id":"5b533951_30fce87c","line":682,"range":{"start_line":681,"start_character":0,"end_line":682,"end_character":51},"updated":"2021-07-21 09:13:40.000000000","message":"This could be removed as by you\u0027re testing on the scope of _trigger_loadbalancer_reconciliation, you can just check that the _reconcile_lbaas is called with the needed parametes.","commit_id":"e302c45beddf91c89fae86df450892d321eb0da3"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"349d229eac65155eb9a42769afab506e0cf562a9","unresolved":true,"context_lines":[{"line_number":185,"context_line":"                },"},{"line_number":186,"context_line":"             \"status\": {"},{"line_number":187,"context_line":"                 \"loadbalancer\": {"},{"line_number":188,"context_line":"                     \"id\": \"01234567890\","},{"line_number":189,"context_line":"                     \"ip\": \"1.2.3.4\","},{"line_number":190,"context_line":"                     \"name\": \"default/demo\","},{"line_number":191,"context_line":"                     \"port_id\": \"1023456789120\","}],"source_content_type":"text/x-python","patch_set":25,"id":"271caa8f_8ca5a14c","line":188,"range":{"start_line":188,"start_character":28,"end_line":188,"end_character":39},"updated":"2021-07-23 08:38:26.000000000","message":"Should these two loadbalancers have different ID? it\u0027s not possible for them to have same ID.","commit_id":"e790508842493d7d32f13c94ddda76d973f2f619"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"ad6182fbb15150a8cdb78e2006b68250aea412e8","unresolved":true,"context_lines":[{"line_number":185,"context_line":"                },"},{"line_number":186,"context_line":"             \"status\": {"},{"line_number":187,"context_line":"                 \"loadbalancer\": {"},{"line_number":188,"context_line":"                     \"id\": \"01234567890\","},{"line_number":189,"context_line":"                     \"ip\": \"1.2.3.4\","},{"line_number":190,"context_line":"                     \"name\": \"default/demo\","},{"line_number":191,"context_line":"                     \"port_id\": \"1023456789120\","}],"source_content_type":"text/x-python","patch_set":25,"id":"32a0bece_da5ac9ae","line":188,"range":{"start_line":188,"start_character":28,"end_line":188,"end_character":39},"in_reply_to":"271caa8f_8ca5a14c","updated":"2021-07-23 08:58:35.000000000","message":"\u003e Should these two loadbalancers have different ID? it\u0027s not possible for them to have same ID.\n\nOhh! I missed that, I changed the last digit to \u00271\u0027. Let me address it","commit_id":"e790508842493d7d32f13c94ddda76d973f2f619"},{"author":{"_account_id":27032,"name":"Maysa de Macedo Souza","email":"maysa.macedo95@gmail.com","username":"maysa"},"change_message_id":"349d229eac65155eb9a42769afab506e0cf562a9","unresolved":true,"context_lines":[{"line_number":675,"context_line":"                    \u0027kuryrloadbalancers/demo\u0027]"},{"line_number":676,"context_line":"        h_lb.KuryrLoadBalancerHandler._trigger_loadbalancer_reconciliation("},{"line_number":677,"context_line":"            m_handler, loadbalancer_crds)"},{"line_number":678,"context_line":"        m_handler._reconcile_lbaas.assert_called_with(selflink)"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    @mock.patch(\u0027kuryr_kubernetes.clients.get_kubernetes_client\u0027)"},{"line_number":681,"context_line":"    @mock.patch(\u0027kuryr_kubernetes.controller.drivers.base\u0027"}],"source_content_type":"text/x-python","patch_set":25,"id":"d60efc47_aa17b9df","line":678,"range":{"start_line":678,"start_character":35,"end_line":678,"end_character":53},"updated":"2021-07-23 08:38:26.000000000","message":"maybe you can also assert that load_balancers is called as you\u0027re mocking the return value at line 670. (same applies for the other test case)","commit_id":"e790508842493d7d32f13c94ddda76d973f2f619"},{"author":{"_account_id":33240,"name":"Sunday Mgbogu","email":"digitalsimboja@gmail.com","username":"digitalsimboja"},"change_message_id":"bfcafce2eca8b8d91c03da406a4c8bc2399b74b8","unresolved":true,"context_lines":[{"line_number":675,"context_line":"                    \u0027kuryrloadbalancers/demo\u0027]"},{"line_number":676,"context_line":"        h_lb.KuryrLoadBalancerHandler._trigger_loadbalancer_reconciliation("},{"line_number":677,"context_line":"            m_handler, loadbalancer_crds)"},{"line_number":678,"context_line":"        m_handler._reconcile_lbaas.assert_called_with(selflink)"},{"line_number":679,"context_line":""},{"line_number":680,"context_line":"    @mock.patch(\u0027kuryr_kubernetes.clients.get_kubernetes_client\u0027)"},{"line_number":681,"context_line":"    @mock.patch(\u0027kuryr_kubernetes.controller.drivers.base\u0027"}],"source_content_type":"text/x-python","patch_set":25,"id":"616ece23_3d1c6dd5","line":678,"range":{"start_line":678,"start_character":35,"end_line":678,"end_character":53},"in_reply_to":"d60efc47_aa17b9df","updated":"2021-07-23 08:57:17.000000000","message":"\u003e maybe you can also assert that load_balancers is called as you\u0027re mocking the return value at line 670. (same applies for the other test case)\n\nOkay! Let me make that check","commit_id":"e790508842493d7d32f13c94ddda76d973f2f619"}]}