)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":2463,"name":"Florian Haas","email":"florian.haas@cleura.com","username":"fghaas"},"change_message_id":"e530dfede262941c9a7b0a16937fcd6f9af916fa","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"9ca973ef_689e8a99","updated":"2022-06-28 17:06:37.000000000","message":"Hi, I\u0027ve just rebased this branch on current master and I wonder if someone might be inclined to review it? :)","commit_id":"58d17b4af5e6b6783e07f2131d8b2376344d47bc"}],"openstack/cloud/_coe.py":[{"author":{"_account_id":27900,"name":"Artem Goncharov","email":"artem.goncharov@gmail.com","username":"gtema"},"change_message_id":"d8558db3042561db3d741c4d1ab1f50926fe92b3","unresolved":true,"context_lines":[{"line_number":72,"context_line":"            the OpenStack API call."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        with _utils.shade_exceptions(\"Error fetching cluster\"):"},{"line_number":75,"context_line":"            data \u003d self._container_infra_client.get(f\u0027/clusters/{id}\u0027)"},{"line_number":76,"context_line":"        return self._normalize_coe_cluster(data)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_coe_cluster(self, name_or_id, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"50d06be1_3d16b39d","line":75,"updated":"2022-07-15 15:06:43.000000000","message":"this is not how we should add new features. We should not use simple client and instead rely on the proper implementation of the proxy and resource interface (see any other service/resource)","commit_id":"58d17b4af5e6b6783e07f2131d8b2376344d47bc"},{"author":{"_account_id":2463,"name":"Florian Haas","email":"florian.haas@cleura.com","username":"fghaas"},"change_message_id":"9c06a0bcdd8bbd26bb8a7ba8a3f437db36c25e2e","unresolved":true,"context_lines":[{"line_number":72,"context_line":"            the OpenStack API call."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        with _utils.shade_exceptions(\"Error fetching cluster\"):"},{"line_number":75,"context_line":"            data \u003d self._container_infra_client.get(f\u0027/clusters/{id}\u0027)"},{"line_number":76,"context_line":"        return self._normalize_coe_cluster(data)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_coe_cluster(self, name_or_id, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"60849071_64751cd4","line":75,"in_reply_to":"50d06be1_3d16b39d","updated":"2022-07-19 12:06:02.000000000","message":"Okay, can you please elaborate because I don\u0027t quite follow what you are trying to tell me here. Are you saying that the entire _coe module should be rewritten to use another interface? Or that one and the same module should use two separate interfaces, one for existing features and one for new ones being added?","commit_id":"58d17b4af5e6b6783e07f2131d8b2376344d47bc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"bdc15e41c6d70f60a3178eb66be25ae950e323ce","unresolved":true,"context_lines":[{"line_number":72,"context_line":"            the OpenStack API call."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        with _utils.shade_exceptions(\"Error fetching cluster\"):"},{"line_number":75,"context_line":"            data \u003d self._container_infra_client.get(f\u0027/clusters/{id}\u0027)"},{"line_number":76,"context_line":"        return self._normalize_coe_cluster(data)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_coe_cluster(self, name_or_id, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"60c89169_5c72c51b","line":75,"in_reply_to":"60849071_64751cd4","updated":"2022-11-21 17:52:00.000000000","message":"Yeah, what Artem is saying is that everything in here should be relying on a proxy API, which in turn should rely on a resource object and relevant methods. For example, any method in \u0027openstack.cloud._compute\u0027 should rely on methods from \u0027openstack.compute.v2._proxy\u0027, which are built on other objects from the \u0027openstack.compute.v2\u0027 module. Does that make sense?\n\nWith that being said, given we don\u0027t actually *have* an \u0027openstack.container_orchestration_engine\u0027 module, I would be tempted to let this requirement slide to get this in. I\u0027m not going to overrule Artem though and in the longer term we\u0027re definitely going to have to write that module or deprecate this.","commit_id":"58d17b4af5e6b6783e07f2131d8b2376344d47bc"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"5242dfdb40db5520acaef016659e7b300486c273","unresolved":true,"context_lines":[{"line_number":72,"context_line":"            the OpenStack API call."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        with _utils.shade_exceptions(\"Error fetching cluster\"):"},{"line_number":75,"context_line":"            data \u003d self._container_infra_client.get(f\u0027/clusters/{id}\u0027)"},{"line_number":76,"context_line":"        return self._normalize_coe_cluster(data)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_coe_cluster(self, name_or_id, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"c8297ee8_409ec86a","line":75,"in_reply_to":"60c89169_5c72c51b","updated":"2022-11-22 11:40:46.000000000","message":"I started an effort here [1] but since I\u0027ve never deployed Magnum I have no way to test it or continue that work. If you\u0027d be interested in picking this up, I\u0027d be more than happy to offer guidance and reviews.\n\n[1] https://review.opendev.org/c/openstack/openstacksdk/+/865266","commit_id":"58d17b4af5e6b6783e07f2131d8b2376344d47bc"},{"author":{"_account_id":2463,"name":"Florian Haas","email":"florian.haas@cleura.com","username":"fghaas"},"change_message_id":"7aa287308ab30fbf59017db98d3c133c6b184522","unresolved":true,"context_lines":[{"line_number":72,"context_line":"            the OpenStack API call."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        with _utils.shade_exceptions(\"Error fetching cluster\"):"},{"line_number":75,"context_line":"            data \u003d self._container_infra_client.get(f\u0027/clusters/{id}\u0027)"},{"line_number":76,"context_line":"        return self._normalize_coe_cluster(data)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def get_coe_cluster(self, name_or_id, filters\u003dNone):"}],"source_content_type":"text/x-python","patch_set":5,"id":"0b1ebdf5_87175cbb","line":75,"in_reply_to":"c8297ee8_409ec86a","updated":"2022-11-28 14:47:58.000000000","message":"Regretfully, I might have had the bandwidth to do that sort of work in June when I first submitted this patch, but I don\u0027t at this point. Apologies.","commit_id":"58d17b4af5e6b6783e07f2131d8b2376344d47bc"}],"openstack/tests/unit/cloud/test_coe_clusters.py":[{"author":{"_account_id":2463,"name":"Florian Haas","email":"florian.haas@cleura.com","username":"fghaas"},"change_message_id":"b9070733664340a1b70a5a79a383c2690a3ab179","unresolved":true,"context_lines":[{"line_number":122,"context_line":"        self.assertIsNotNone(r)"},{"line_number":123,"context_line":"        self.assertDictEqual("},{"line_number":124,"context_line":"            r, self.cloud._normalize_coe_cluster(coe_cluster_obj))"},{"line_number":125,"context_line":"        self.assert_calls()"},{"line_number":126,"context_line":""},{"line_number":127,"context_line":"    def test_get_coe_cluster_not_found(self):"},{"line_number":128,"context_line":"        self.register_uris([dict("}],"source_content_type":"text/x-python","patch_set":3,"id":"7ad386e9_f5c0f3d3","line":125,"updated":"2022-02-11 12:51:06.000000000","message":"Here I\u0027d like to add a Mock assert_called() invocation, to ensure that get_coe_cluster() correctly identifies the UUID as such, and hands off to get_coe_cluster_by_id(). But I don\u0027t quite know what would be the proper way to include that assertion.\n\nIf a reviewer could help me out here with a suggestion, I\u0027d much appreciate that. :)","commit_id":"8180b7c0daab85551117d9e6be673778e9ce86ea"}]}
