)]}'
{"manila/share/drivers/cephfs/driver.py":[{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"694ad10ce12a5347134ab6b7443dcda61b0c678d","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"def rados_command(rados_client, prefix\u003dNone, args\u003dNone, json_obj\u003dFalse,"},{"line_number":154,"context_line":"                  target\u003d(\u0027mon-mgr\u0027, ), version\u003d\"14.2.18\"):"},{"line_number":155,"context_line":"    \"\"\"Safer wrapper for ceph_argparse.json_command"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    Raises error exception instead of relying on caller to check return"}],"source_content_type":"text/x-python","patch_set":1,"id":"35dca3cd_4bcee278","line":154,"updated":"2021-06-02 16:58:16.000000000","message":"Is there a reason you supply a particular version as default here?  Is the idea to always supply a version when rados_command is called?  Then would it make sense to default to None so there will be an error if the code is changed and this is forgotten?","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"28c8930911e9624d603f2896ecda08c7b9ad2b8e","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"def rados_command(rados_client, prefix\u003dNone, args\u003dNone, json_obj\u003dFalse,"},{"line_number":154,"context_line":"                  target\u003d(\u0027mon-mgr\u0027, ), version\u003d\"14.2.18\"):"},{"line_number":155,"context_line":"    \"\"\"Safer wrapper for ceph_argparse.json_command"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    Raises error exception instead of relying on caller to check return"}],"source_content_type":"text/x-python","patch_set":1,"id":"067a911e_c0244eb7","line":154,"range":{"start_line":154,"start_character":18,"end_line":154,"end_character":38},"updated":"2021-06-02 17:09:05.000000000","message":"Minor rewrite suggestion; \n\nset target\u003dNone as the default here\n\n\nso in the method you can do:\n\n \n target \u003d target or self.default_target  # or what you call the variable you initialize in the driver\u0027s init method\n\n(We override target only in one method afaics)","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"fdf4b10b5b1300a64c15cc78ced0bf9c0c60eadd","unresolved":false,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"def rados_command(rados_client, prefix\u003dNone, args\u003dNone, json_obj\u003dFalse,"},{"line_number":154,"context_line":"                  target\u003d(\u0027mon-mgr\u0027, ), version\u003d\"14.2.18\"):"},{"line_number":155,"context_line":"    \"\"\"Safer wrapper for ceph_argparse.json_command"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    Raises error exception instead of relying on caller to check return"}],"source_content_type":"text/x-python","patch_set":1,"id":"2b105121_7463ecf0","line":154,"range":{"start_line":154,"start_character":18,"end_line":154,"end_character":38},"in_reply_to":"067a911e_c0244eb7","updated":"2021-06-02 22:52:03.000000000","message":"Like the idea, that would make my life so easier. But the only issue is that rados_command function is defined outside of the CephFS driver class. We call it from both the driver class and from the helper class, so it is more clean to do it this way. Unless I make default_target a global var (which is ugly, IMHO)","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eeae4fabc3bbc05241bce635a649cfcb75ce5c6a","unresolved":false,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"def rados_command(rados_client, prefix\u003dNone, args\u003dNone, json_obj\u003dFalse,"},{"line_number":154,"context_line":"                  target\u003d(\u0027mon-mgr\u0027, ), version\u003d\"14.2.18\"):"},{"line_number":155,"context_line":"    \"\"\"Safer wrapper for ceph_argparse.json_command"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    Raises error exception instead of relying on caller to check return"}],"source_content_type":"text/x-python","patch_set":1,"id":"cf453e15_5d4f610b","line":154,"range":{"start_line":154,"start_character":18,"end_line":154,"end_character":38},"in_reply_to":"2b105121_7463ecf0","updated":"2021-06-09 22:15:13.000000000","message":"Thanks for pointing that out - I wasn\u0027t reading this right. Still better to set this once rather than including the version and checking it on every call - we\u0027re using global variables a few other places in this driver, carefully\n\ndef setup_default_ceph_cmd_target():\n    global ceph_default_target\n    if not ceph_default_target:\n        ceph_default_target \u003d (\u0027mon-mgr\u0027, )\n        try:\n            ceph_version \u003d package_version(\n                rados_command(self.rados_client, \"version\", target\u003d(\u0027mon\u0027, ))\n            )\n            if ceph_version \u003c package_version(\u002714.2.21\u0027):\n                ceph_default_target \u003d (\u0027mon\u0027, )\n        except Exception:\n            LOG.exception(\"Error reading ceph version to set the default \"\n                          \"target. Defaulting to mon-mgr.\")\n\n\nAnd call this setup method when self.rados_client has been initialized - one time.. \n\n\nthat way, the rados_command method signature becomes easier:\n\n  def rados_command(rados_client, prefix\u003dNone, args\u003dNone, json_obj\u003dFalse,\n                  target\u003dNone):\n \n\n     target \u003d target or ceph_default_target\n     ....","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"fdf4b10b5b1300a64c15cc78ced0bf9c0c60eadd","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"def rados_command(rados_client, prefix\u003dNone, args\u003dNone, json_obj\u003dFalse,"},{"line_number":154,"context_line":"                  target\u003d(\u0027mon-mgr\u0027, ), version\u003d\"14.2.18\"):"},{"line_number":155,"context_line":"    \"\"\"Safer wrapper for ceph_argparse.json_command"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    Raises error exception instead of relying on caller to check return"}],"source_content_type":"text/x-python","patch_set":1,"id":"c3672acc_712f62af","line":154,"in_reply_to":"35dca3cd_4bcee278","updated":"2021-06-02 22:52:03.000000000","message":"Hmm good point. 14.2.18 is the minor version we support since Wallaby, that\u0027s why I was passing it as default. But None makes more sense.","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"aeae2d01c0cefaacd350f084d25799ab656cb774","unresolved":false,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"def rados_command(rados_client, prefix\u003dNone, args\u003dNone, json_obj\u003dFalse,"},{"line_number":154,"context_line":"                  target\u003d(\u0027mon-mgr\u0027, ), version\u003d\"14.2.18\"):"},{"line_number":155,"context_line":"    \"\"\"Safer wrapper for ceph_argparse.json_command"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":"    Raises error exception instead of relying on caller to check return"}],"source_content_type":"text/x-python","patch_set":1,"id":"e65f476a_38bd2f25","line":154,"range":{"start_line":154,"start_character":18,"end_line":154,"end_character":38},"in_reply_to":"cf453e15_5d4f610b","updated":"2021-06-10 11:25:12.000000000","message":"Ok, let\u0027s use a global var","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"28c8930911e9624d603f2896ecda08c7b9ad2b8e","unresolved":true,"context_lines":[{"line_number":168,"context_line":"            ceph command)"},{"line_number":169,"context_line":"    \"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if re.match(r\u002714.\\d.\\d\u0027, version):"},{"line_number":172,"context_line":"        if target \u003d\u003d (\u0027mon-mgr\u0027):"},{"line_number":173,"context_line":"            target \u003d (\u0027mgr\u0027)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    if args is None:"},{"line_number":176,"context_line":"        args \u003d {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"23f06555_8e25e354","line":173,"range":{"start_line":171,"start_character":4,"end_line":173,"end_character":28},"updated":"2021-06-02 17:09:05.000000000","message":"Also, i wonder if you can do something like this:\n\n  from packaging.version import parse as package_version\n\n  \n  if package_version(ceph_version) \u003c package_version(\u002714.2.21\u0027):\n     target \u003d \u0027mgr\u0027\n\n\nhttps://opendev.org/openstack/cinder/src/branch/master/cinder/volume/drivers/dell_emc/powermax/utils.py#L2044-L2053","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"fdf4b10b5b1300a64c15cc78ced0bf9c0c60eadd","unresolved":false,"context_lines":[{"line_number":168,"context_line":"            ceph command)"},{"line_number":169,"context_line":"    \"\"\""},{"line_number":170,"context_line":""},{"line_number":171,"context_line":"    if re.match(r\u002714.\\d.\\d\u0027, version):"},{"line_number":172,"context_line":"        if target \u003d\u003d (\u0027mon-mgr\u0027):"},{"line_number":173,"context_line":"            target \u003d (\u0027mgr\u0027)"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"    if args is None:"},{"line_number":176,"context_line":"        args \u003d {}"}],"source_content_type":"text/x-python","patch_set":1,"id":"58dad5a9_b1836b6c","line":173,"range":{"start_line":171,"start_character":4,"end_line":173,"end_character":28},"in_reply_to":"23f06555_8e25e354","updated":"2021-06-02 22:52:03.000000000","message":"Oh sweet, I wasn\u0027t aware of this library. I\u0027ll use that.","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"f79da97f624e7583723b22d382390586367525b3","unresolved":true,"context_lines":[{"line_number":224,"context_line":"        self._volname \u003d None"},{"line_number":225,"context_line":"        self._ceph_version \u003d None"},{"line_number":226,"context_line":"        self.configuration.append_config_values(cephfs_opts)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        try:"},{"line_number":229,"context_line":"            int(self.configuration.cephfs_volume_mode, 8)"},{"line_number":230,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"64ca831f_fbb1bf0e","line":227,"updated":"2021-06-02 16:57:37.000000000","message":"You can set a self.ceph_target right here (either \"mon-mgr\" or \"mon\") and use that in the rados_command method instead of passing the version with calls - that way you don\u0027t have to pass it to the protocol_helper too.","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"fdf4b10b5b1300a64c15cc78ced0bf9c0c60eadd","unresolved":false,"context_lines":[{"line_number":224,"context_line":"        self._volname \u003d None"},{"line_number":225,"context_line":"        self._ceph_version \u003d None"},{"line_number":226,"context_line":"        self.configuration.append_config_values(cephfs_opts)"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        try:"},{"line_number":229,"context_line":"            int(self.configuration.cephfs_volume_mode, 8)"},{"line_number":230,"context_line":"        except ValueError:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bb0c4201_3b9dc35e","line":227,"in_reply_to":"64ca831f_fbb1bf0e","updated":"2021-06-02 22:52:03.000000000","message":"Comment above","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"43b4901a7c733ffed79482892ee2f432eb409557","unresolved":true,"context_lines":[{"line_number":325,"context_line":"        if self._ceph_version:"},{"line_number":326,"context_line":"            return self._ceph_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        version \u003d self._execute(\u0027ceph\u0027, \u0027version\u0027, \u0027--user\u0027, \u0027manila\u0027)"},{"line_number":329,"context_line":"        self._ceph_version \u003d version[0].split(\u0027 \u0027)[2].split(\u0027-\u0027)[0]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        return self._ceph_version"}],"source_content_type":"text/x-python","patch_set":1,"id":"31398ccb_6d66c5c9","line":328,"updated":"2021-06-02 19:25:50.000000000","message":"Suggest using rados_command as Goutham mentioned in earlier comment to get the version. See below,\n```\nversion_str \u003d rados_command(self.rados_client, \"version\", target\u003d(\u0027mon\u0027, ))\n```\n\nThen ceph_version can return a dictionary that can have keys \"major\", \"minor\", \"extra\" similar to what rook does, \nhttps://github.com/rook/rook/blob/master/pkg/operator/ceph/version/version.go#L29\nand maybe not worry about \"build\" field for now\n\n```\np \u003d  re.compile(\u0027ceph version (\\d+)\\.(\\d+)\\.(\\d+)\u0027)\nmajor, minor, extra \u003d p.match(version_str).groups()\nself._ceph_version[\u0027major\u0027] \u003d major\nself._ceph_version[\u0027minor\u0027] \u003d minor\nself._ceph_version[\u0027extra\u0027] \u003d extra\n```\nself._ceph_version is a dictionary.","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"aeae2d01c0cefaacd350f084d25799ab656cb774","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        if self._ceph_version:"},{"line_number":326,"context_line":"            return self._ceph_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        version \u003d self._execute(\u0027ceph\u0027, \u0027version\u0027, \u0027--user\u0027, \u0027manila\u0027)"},{"line_number":329,"context_line":"        self._ceph_version \u003d version[0].split(\u0027 \u0027)[2].split(\u0027-\u0027)[0]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        return self._ceph_version"}],"source_content_type":"text/x-python","patch_set":1,"id":"d5eb6b98_44a4a5c1","line":328,"in_reply_to":"2c434a43_b302ea8c","updated":"2021-06-10 11:25:12.000000000","message":"Ack","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"fdf4b10b5b1300a64c15cc78ced0bf9c0c60eadd","unresolved":true,"context_lines":[{"line_number":325,"context_line":"        if self._ceph_version:"},{"line_number":326,"context_line":"            return self._ceph_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        version \u003d self._execute(\u0027ceph\u0027, \u0027version\u0027, \u0027--user\u0027, \u0027manila\u0027)"},{"line_number":329,"context_line":"        self._ceph_version \u003d version[0].split(\u0027 \u0027)[2].split(\u0027-\u0027)[0]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        return self._ceph_version"}],"source_content_type":"text/x-python","patch_set":1,"id":"8373562e_3e8091eb","line":328,"in_reply_to":"31398ccb_6d66c5c9","updated":"2021-06-02 22:52:03.000000000","message":"Hmm not following Ramana. We need the deployed Ceph cluster version. For what I understand, Rook is is checking for the minimum and maximum compatible versions that the rados client supports. Correct me if I am wrong :)","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eeae4fabc3bbc05241bce635a649cfcb75ce5c6a","unresolved":true,"context_lines":[{"line_number":325,"context_line":"        if self._ceph_version:"},{"line_number":326,"context_line":"            return self._ceph_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        version \u003d self._execute(\u0027ceph\u0027, \u0027version\u0027, \u0027--user\u0027, \u0027manila\u0027)"},{"line_number":329,"context_line":"        self._ceph_version \u003d version[0].split(\u0027 \u0027)[2].split(\u0027-\u0027)[0]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        return self._ceph_version"}],"source_content_type":"text/x-python","patch_set":1,"id":"2c434a43_b302ea8c","line":328,"in_reply_to":"67d97e10_ab873545","updated":"2021-06-09 22:15:13.000000000","message":"\u003e With regards to mention of rook\u0027s Ceph version checking code in my earlier comment, I wanted to point out that the storing the version of Ceph (be it that of the monitor daemon or the client library) as a dictionary would be useful.\n\nUseful in terms of future proofing?","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"3f80ee61f9e9ebaaa153b4e9ab4fc5355c664c1c","unresolved":true,"context_lines":[{"line_number":325,"context_line":"        if self._ceph_version:"},{"line_number":326,"context_line":"            return self._ceph_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        version \u003d self._execute(\u0027ceph\u0027, \u0027version\u0027, \u0027--user\u0027, \u0027manila\u0027)"},{"line_number":329,"context_line":"        self._ceph_version \u003d version[0].split(\u0027 \u0027)[2].split(\u0027-\u0027)[0]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        return self._ceph_version"}],"source_content_type":"text/x-python","patch_set":1,"id":"67d97e10_ab873545","line":328,"in_reply_to":"8373562e_3e8091eb","updated":"2021-06-03 04:54:08.000000000","message":"```\nrados_command(self.rados_client, \"version\", target\u003d(\u0027mon\u0027, ))\n```\nThe above should fetch the version of the monitor.\nSee, https://github.com/ceph/ceph/blob/nautilus/src/mon/MonCommands.h#L255\n\nThis is equivalent to the CLI command\n```\n$ ceph --id\u003d\u003cmanila\u0027s cephfs_auth_id\u003e version\n```\nor python code\n```\nself._execute(\u0027ceph\u0027, \u0027--id\u0027, self.configuration.safe_get(\u0027cephfs_auth_id\u0027), \u0027version\u0027)\n```\nNote that I didn\u0027t hardcode the auth ID as \"manila\" like you did.\n\n\nTechnically, shouldn\u0027t we also check the version of python3-ceph-argparse package? The underlying issue is also in the ceph_argparse library. But I guess it\u0027s OK to assume that the Ceph backend version matches the python-rados and ceph-argparse versions in the manila share service node?\n\nWith regards to mention of rook\u0027s Ceph version checking code in my earlier comment, I wanted to point out that the storing the version of Ceph (be it that of the monitor daemon or the client library) as a dictionary would be useful.","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"45826a6eb0816f6adae3aef0f2fc5f3ddb909a0c","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        if self._ceph_version:"},{"line_number":326,"context_line":"            return self._ceph_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        version \u003d self._execute(\u0027ceph\u0027, \u0027version\u0027, \u0027--user\u0027, \u0027manila\u0027)"},{"line_number":329,"context_line":"        self._ceph_version \u003d version[0].split(\u0027 \u0027)[2].split(\u0027-\u0027)[0]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        return self._ceph_version"}],"source_content_type":"text/x-python","patch_set":1,"id":"19efcfbb_844a4bee","line":328,"in_reply_to":"89937647_e3e7d161","updated":"2021-06-10 22:14:13.000000000","message":"\u003e Technically, shouldn\u0027t we also check the version of python3-ceph-argparse package? The underlying issue is also in the ceph_argparse library. But I guess it\u0027s OK to assume that the Ceph backend version matches the python-rados and ceph-argparse versions in the manila share service node?\n\nIt\u0027s simple to fetch mon version/backend ceph version as we\u0027ve already discussed. I\u0027m not sure how we can fetch the version of the Ceph client, i.e., python rados client (python3-rados package)\n\n   import rados\n   print(rados.__version__)\n\ndoesn\u0027t work as __version__ attribute is missing. I guess we\u0027ll have to do something like mentioned here https://stackoverflow.com/a/32965521 ?","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"24d5583e972f670470de307a62d44c9e0c6532e4","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        if self._ceph_version:"},{"line_number":326,"context_line":"            return self._ceph_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        version \u003d self._execute(\u0027ceph\u0027, \u0027version\u0027, \u0027--user\u0027, \u0027manila\u0027)"},{"line_number":329,"context_line":"        self._ceph_version \u003d version[0].split(\u0027 \u0027)[2].split(\u0027-\u0027)[0]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        return self._ceph_version"}],"source_content_type":"text/x-python","patch_set":1,"id":"4e5b7d49_4e0f6f7f","line":328,"in_reply_to":"89937647_e3e7d161","updated":"2021-06-10 21:57:28.000000000","message":"Ah, makes sense then. Thanks!","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"e637a8783c3910596b9ac983299d110ee23194be","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        if self._ceph_version:"},{"line_number":326,"context_line":"            return self._ceph_version"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"        version \u003d self._execute(\u0027ceph\u0027, \u0027version\u0027, \u0027--user\u0027, \u0027manila\u0027)"},{"line_number":329,"context_line":"        self._ceph_version \u003d version[0].split(\u0027 \u0027)[2].split(\u0027-\u0027)[0]"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"        return self._ceph_version"}],"source_content_type":"text/x-python","patch_set":1,"id":"89937647_e3e7d161","line":328,"in_reply_to":"d5eb6b98_44a4a5c1","updated":"2021-06-10 19:54:38.000000000","message":"\u003e Useful in terms of future proofing?\n\nYes. We might want to check versions granularly, e.g., CephFS subvolume group snapshot gets backported in 16.2.10 from 17.x. The driver\u0027s sharegroup snapshot API can check whether Ceph cluster version \u003e\u003d 16.2.10.","commit_id":"1cd354bc02530d147343ee015838b5cd488f393d"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"f3dd29f763a4e7254dfc73e12ab5b394dafa74ab","unresolved":true,"context_lines":[{"line_number":325,"context_line":"        if not ceph_default_target:"},{"line_number":326,"context_line":"            ceph_default_target \u003d (\u0027mon-mgr\u0027, )"},{"line_number":327,"context_line":"            try:"},{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"2f5282a3_747aecd9","line":328,"updated":"2021-06-14 16:28:36.000000000","message":"Since we are not getting the backport in Ceph Nautilus I thought that directly we can check against the major version. WDYT Ramana and Goutham?","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"cb8f8ab98516e15b6808812c8095c43000396522","unresolved":true,"context_lines":[{"line_number":325,"context_line":"        if not ceph_default_target:"},{"line_number":326,"context_line":"            ceph_default_target \u003d (\u0027mon-mgr\u0027, )"},{"line_number":327,"context_line":"            try:"},{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"8a44e86b_eb5ae272","line":328,"in_reply_to":"2f5282a3_747aecd9","updated":"2021-06-14 19:45:12.000000000","message":"Agree. Let\u0027s just check for major version.","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"0e61bdedb4204b682e118e41aad0e8a30f2642d2","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        if not ceph_default_target:"},{"line_number":326,"context_line":"            ceph_default_target \u003d (\u0027mon-mgr\u0027, )"},{"line_number":327,"context_line":"            try:"},{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"}],"source_content_type":"text/x-python","patch_set":3,"id":"92f720dc_c9e2ef8b","line":328,"in_reply_to":"8a44e86b_eb5ae272","updated":"2021-06-18 17:09:21.000000000","message":"Done","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"cb8f8ab98516e15b6808812c8095c43000396522","unresolved":true,"context_lines":[{"line_number":326,"context_line":"            ceph_default_target \u003d (\u0027mon-mgr\u0027, )"},{"line_number":327,"context_line":"            try:"},{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"},{"line_number":332,"context_line":"                    ceph_default_target \u003d (\u0027mon\u0027, )"}],"source_content_type":"text/x-python","patch_set":3,"id":"86b5735c_4a95e7af","line":329,"updated":"2021-06-14 19:45:12.000000000","message":"Not sure why you need package_version module here? Can\u0027t you just check do,\n\nmajor_version \u003d int(ceph_version[\u0027major\u0027])\nif major_version \u003d\u003d 14:\n    ceph_default_target \u003d (\u0027mon\u0027, )\nelif major_version \u003c 14:\n    msg \u003d _(\"CephFSDriver does not support Ceph cluster version less than nautilus 14.x\")\n    raise exception.ShareBackendException(msg)\n\n\nAlso, I\u0027m not sure whether ceph_default_target for nautilus needs to be \u0027mon\u0027 or \u0027mgr\u0027. \nDid you check with nautilus Ceph cluster in your local setup? The CI tests only octopus, right?","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"0e61bdedb4204b682e118e41aad0e8a30f2642d2","unresolved":false,"context_lines":[{"line_number":326,"context_line":"            ceph_default_target \u003d (\u0027mon-mgr\u0027, )"},{"line_number":327,"context_line":"            try:"},{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"},{"line_number":332,"context_line":"                    ceph_default_target \u003d (\u0027mon\u0027, )"}],"source_content_type":"text/x-python","patch_set":3,"id":"8ffc2f59_7e805a0e","line":329,"in_reply_to":"767cf8c3_275eb879","updated":"2021-06-18 17:09:21.000000000","message":"Done","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"f0de8152a3da2a570b545d53e4adf5017c4a755c","unresolved":true,"context_lines":[{"line_number":326,"context_line":"            ceph_default_target \u003d (\u0027mon-mgr\u0027, )"},{"line_number":327,"context_line":"            try:"},{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"},{"line_number":332,"context_line":"                    ceph_default_target \u003d (\u0027mon\u0027, )"}],"source_content_type":"text/x-python","patch_set":3,"id":"767cf8c3_275eb879","line":329,"in_reply_to":"86b5735c_4a95e7af","updated":"2021-06-15 07:04:46.000000000","message":"Yeah, I guess that the packaging lib was useful if testing the version with the major.minor.extra format.\n\nAbout which target to use... good question, I was assuming it was mon, but I think it is mgr. I\u0027ll check in my env.","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"cb8f8ab98516e15b6808812c8095c43000396522","unresolved":true,"context_lines":[{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"},{"line_number":332,"context_line":"                    ceph_default_target \u003d (\u0027mon\u0027, )"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                LOG.exception(\"Error reading ceph version to set the default \""}],"source_content_type":"text/x-python","patch_set":3,"id":"e7496319_eb18103f","line":331,"updated":"2021-06-14 19:45:12.000000000","message":"Ceph versions less than 14 don\u0027t have mgr-volumes module. The driver won\u0027t work.\nYou\u0027d need to error out with relevant error message.","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"0e61bdedb4204b682e118e41aad0e8a30f2642d2","unresolved":false,"context_lines":[{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"},{"line_number":332,"context_line":"                    ceph_default_target \u003d (\u0027mon\u0027, )"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                LOG.exception(\"Error reading ceph version to set the default \""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a93b0df_5d08167c","line":331,"in_reply_to":"c200100d_2fe5cac0","updated":"2021-06-18 17:09:21.000000000","message":"Done","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"f0de8152a3da2a570b545d53e4adf5017c4a755c","unresolved":true,"context_lines":[{"line_number":328,"context_line":"                cv \u003d self.ceph_version[\u0027major\u0027]"},{"line_number":329,"context_line":"                cv \u003d package_version.parse(cv)"},{"line_number":330,"context_line":""},{"line_number":331,"context_line":"                if cv \u003c\u003d package_version.parse(\u002714\u0027):"},{"line_number":332,"context_line":"                    ceph_default_target \u003d (\u0027mon\u0027, )"},{"line_number":333,"context_line":"            except Exception:"},{"line_number":334,"context_line":"                LOG.exception(\"Error reading ceph version to set the default \""}],"source_content_type":"text/x-python","patch_set":3,"id":"c200100d_2fe5cac0","line":331,"in_reply_to":"e7496319_eb18103f","updated":"2021-06-15 07:04:46.000000000","message":"Yes, good catch","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"cb8f8ab98516e15b6808812c8095c43000396522","unresolved":true,"context_lines":[{"line_number":335,"context_line":"                              \"target. Defaulting to mon-mgr.\")"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    @property"},{"line_number":338,"context_line":"    def ceph_version(self):"},{"line_number":339,"context_line":"        if self._ceph_version:"},{"line_number":340,"context_line":"            return self._ceph_version"},{"line_number":341,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"8bbd3089_5e1152c2","line":338,"updated":"2021-06-14 19:45:12.000000000","message":"Maybe better to name as ceph_mon_version and be explicit? We may later want to find out ceph_client_version within the manila driver.","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"0e61bdedb4204b682e118e41aad0e8a30f2642d2","unresolved":false,"context_lines":[{"line_number":335,"context_line":"                              \"target. Defaulting to mon-mgr.\")"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    @property"},{"line_number":338,"context_line":"    def ceph_version(self):"},{"line_number":339,"context_line":"        if self._ceph_version:"},{"line_number":340,"context_line":"            return self._ceph_version"},{"line_number":341,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"21f6a5b1_2ed3a265","line":338,"in_reply_to":"66500c15_2d93c104","updated":"2021-06-18 17:09:21.000000000","message":"Done","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"f0de8152a3da2a570b545d53e4adf5017c4a755c","unresolved":true,"context_lines":[{"line_number":335,"context_line":"                              \"target. Defaulting to mon-mgr.\")"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    @property"},{"line_number":338,"context_line":"    def ceph_version(self):"},{"line_number":339,"context_line":"        if self._ceph_version:"},{"line_number":340,"context_line":"            return self._ceph_version"},{"line_number":341,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"66500c15_2d93c104","line":338,"in_reply_to":"8bbd3089_5e1152c2","updated":"2021-06-15 07:04:46.000000000","message":"Sounds good, will update","commit_id":"a779c0c5fd51b0cd0e5d6ee5640a19f2fe4b306a"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"0e61bdedb4204b682e118e41aad0e8a30f2642d2","unresolved":true,"context_lines":[{"line_number":327,"context_line":"                ceph_major_version \u003d self.ceph_mon_version[\u0027major\u0027]"},{"line_number":328,"context_line":""},{"line_number":329,"context_line":"                if ceph_major_version \u003d\u003d \u002714\u0027:"},{"line_number":330,"context_line":"                    ceph_default_target \u003d (\u0027mgr\u0027, )"},{"line_number":331,"context_line":"                elif ceph_major_version \u003c \u002714\u0027:"},{"line_number":332,"context_line":"                    msg \u003d _(\"CephFSDriver does not support Ceph \""},{"line_number":333,"context_line":"                            \"cluster version less than 14.x (Nautilus)\")"}],"source_content_type":"text/x-python","patch_set":4,"id":"dfc75cad_6e1cdb99","line":330,"updated":"2021-06-18 17:09:21.000000000","message":"Setting the target to (\u0027mgr\u0027, ) worked with nautilus Ceph cluster in your local testing? And the driver worked with manila client  caps mentioned here,\nhttps://review.opendev.org/c/openstack/manila/+/782871/7/doc/source/admin/cephfs_driver.rst#186 ?","commit_id":"7901f17219ff666af49cdebc74cebeb832706ac0"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"65b969f48f0d2fb331d5d0a835af3ac783401168","unresolved":true,"context_lines":[{"line_number":242,"context_line":"            protocol_helper_class \u003d getattr("},{"line_number":243,"context_line":"                sys.modules[__name__], \u0027NFSProtocolHelper\u0027)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        self.setup_default_ceph_cmd_target()"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        self.protocol_helper \u003d protocol_helper_class("},{"line_number":248,"context_line":"            self._execute,"}],"source_content_type":"text/x-python","patch_set":7,"id":"058a5e4c_c16d3feb","line":245,"updated":"2021-06-25 15:53:44.000000000","message":"I don\u0027t get why moving this method within do_setup() from __init__() solved the issue with `fs volume ls` hang that we experienced while testing. Can you please elaborate?","commit_id":"2e7522a5451a4ff08821cc69d471ee2c2bbe065c"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"7a80e38099dea61d6f2b4c1ff1e8941cf21312a8","unresolved":true,"context_lines":[{"line_number":242,"context_line":"            protocol_helper_class \u003d getattr("},{"line_number":243,"context_line":"                sys.modules[__name__], \u0027NFSProtocolHelper\u0027)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        self.setup_default_ceph_cmd_target()"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        self.protocol_helper \u003d protocol_helper_class("},{"line_number":248,"context_line":"            self._execute,"}],"source_content_type":"text/x-python","patch_set":7,"id":"ddd513bc_1b34be61","line":245,"in_reply_to":"058a5e4c_c16d3feb","updated":"2021-06-25 20:06:16.000000000","message":"Sure thing. Actually, I\u0027m quite convinced we should move setup_rados() and setup_json_command() to do_setup() as well.\n\nThe share manager uses import_object from the oslo import utils when discovering the configured backend, therefore getting the class and creating an instance of it. That happens here https://github.com/openstack/manila/blob/master/manila/share/manager.py#L266-L269.\n\nAnd the actual driver initialization happens when we select the correct driver (CephFS Native or CephFS NFS), and that happens when we execute do_setup. That happens here  https://github.com/openstack/manila/blob/master/manila/share/manager.py#L343.\n\n__init__ doesn\u0027t provide a way to surface errors, so we should limit its usage to loading modules, we shouldn\u0027t be establishing connections to the backend or reading any stats.\n\nAnd I believe that when this class constructor is called, very early on the execution, there are missing attributes that leads us to a failed state which we cannot properly debug.\n\nHope this clarifies a bit, let me know your thoughts.","commit_id":"2e7522a5451a4ff08821cc69d471ee2c2bbe065c"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"994f089f0ebc7eecf43669cbb8bd28088b5bbf36","unresolved":false,"context_lines":[{"line_number":242,"context_line":"            protocol_helper_class \u003d getattr("},{"line_number":243,"context_line":"                sys.modules[__name__], \u0027NFSProtocolHelper\u0027)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        self.setup_default_ceph_cmd_target()"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        self.protocol_helper \u003d protocol_helper_class("},{"line_number":248,"context_line":"            self._execute,"}],"source_content_type":"text/x-python","patch_set":7,"id":"b5acf975_33b7733b","line":245,"in_reply_to":"22657bc0_d8711465","updated":"2021-06-26 01:06:24.000000000","message":"Done","commit_id":"2e7522a5451a4ff08821cc69d471ee2c2bbe065c"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"092da56dfe0e0e51bb988839a27ce5025239fb62","unresolved":true,"context_lines":[{"line_number":242,"context_line":"            protocol_helper_class \u003d getattr("},{"line_number":243,"context_line":"                sys.modules[__name__], \u0027NFSProtocolHelper\u0027)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":"        self.setup_default_ceph_cmd_target()"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"        self.protocol_helper \u003d protocol_helper_class("},{"line_number":248,"context_line":"            self._execute,"}],"source_content_type":"text/x-python","patch_set":7,"id":"22657bc0_d8711465","line":245,"in_reply_to":"ddd513bc_1b34be61","updated":"2021-06-26 00:50:11.000000000","message":"Agreed. Drivers shouldn\u0027t be performing any calls to the backend in the __init__ method; if there are issues making those calls, error handling becomes quite difficult.","commit_id":"2e7522a5451a4ff08821cc69d471ee2c2bbe065c"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"65b969f48f0d2fb331d5d0a835af3ac783401168","unresolved":true,"context_lines":[{"line_number":328,"context_line":"                ceph_major_version \u003d self.ceph_mon_version[\u0027major\u0027]"},{"line_number":329,"context_line":"            except Exception:"},{"line_number":330,"context_line":"                msg \u003d _(\"Error reading ceph version to set the default \""},{"line_number":331,"context_line":"                        \"target. Defaulting to mon-mgr.\")"},{"line_number":332,"context_line":"                raise exception.ShareBackendException(msg\u003dmsg)"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"            if ceph_major_version \u003d\u003d \u002714\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"5fbc26c1_eb23fa4a","line":331,"updated":"2021-06-25 15:53:44.000000000","message":"I don\u0027t follow this error message, \"Defaulting to mon-mgr.\" So even if the below exception is raised, the driver continues to function, and uses mon-mgr as the default cmd target?","commit_id":"2e7522a5451a4ff08821cc69d471ee2c2bbe065c"},{"author":{"_account_id":8056,"name":"Ramana Raja","email":"rraja@redhat.com","username":"Ram_Raja"},"change_message_id":"994f089f0ebc7eecf43669cbb8bd28088b5bbf36","unresolved":false,"context_lines":[{"line_number":328,"context_line":"                ceph_major_version \u003d self.ceph_mon_version[\u0027major\u0027]"},{"line_number":329,"context_line":"            except Exception:"},{"line_number":330,"context_line":"                msg \u003d _(\"Error reading ceph version to set the default \""},{"line_number":331,"context_line":"                        \"target. Defaulting to mon-mgr.\")"},{"line_number":332,"context_line":"                raise exception.ShareBackendException(msg\u003dmsg)"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"            if ceph_major_version \u003d\u003d \u002714\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"6bc166ad_9d3828a7","line":331,"in_reply_to":"1ce265f8_c194b6c6","updated":"2021-06-26 01:06:24.000000000","message":"Done","commit_id":"2e7522a5451a4ff08821cc69d471ee2c2bbe065c"},{"author":{"_account_id":6413,"name":"Victoria Martinez de la Cruz","email":"victoria@redhat.com","username":"vkmc"},"change_message_id":"7a80e38099dea61d6f2b4c1ff1e8941cf21312a8","unresolved":true,"context_lines":[{"line_number":328,"context_line":"                ceph_major_version \u003d self.ceph_mon_version[\u0027major\u0027]"},{"line_number":329,"context_line":"            except Exception:"},{"line_number":330,"context_line":"                msg \u003d _(\"Error reading ceph version to set the default \""},{"line_number":331,"context_line":"                        \"target. Defaulting to mon-mgr.\")"},{"line_number":332,"context_line":"                raise exception.ShareBackendException(msg\u003dmsg)"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"            if ceph_major_version \u003d\u003d \u002714\u0027:"}],"source_content_type":"text/x-python","patch_set":7,"id":"1ce265f8_c194b6c6","line":331,"in_reply_to":"5fbc26c1_eb23fa4a","updated":"2021-06-25 20:06:16.000000000","message":"So if there is an error that prevents us for getting the version, we cannot determine if the target that needs to be used. In that case, I think it\u0027s better to stop the exec here. The \u0027defaulting to mon-mgr\u0027 should be dropped.","commit_id":"2e7522a5451a4ff08821cc69d471ee2c2bbe065c"}]}