)]}'
{"manila/share/driver.py":[{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"37ec92f2a5f6f78bc56ebd97f516a9bba2668c6e","unresolved":true,"context_lines":[{"line_number":1315,"context_line":"            replication_domain\u003dself.replication_domain,"},{"line_number":1316,"context_line":"            filter_function\u003dself.get_filter_function(),"},{"line_number":1317,"context_line":"            goodness_function\u003dself.get_goodness_function(),"},{"line_number":1318,"context_line":"            max_shares_per_share_server\u003dself.max_shares_per_share_server,"},{"line_number":1319,"context_line":"            max_share_server_size\u003dself.max_share_server_size"},{"line_number":1320,"context_line":"        )"},{"line_number":1321,"context_line":"        if isinstance(data, dict):"},{"line_number":1322,"context_line":"            common.update(data)"},{"line_number":1323,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"47e445af_44edde89","line":1320,"range":{"start_line":1318,"start_character":12,"end_line":1320,"end_character":9},"updated":"2021-03-06 19:46:14.000000000","message":"Is this necessary as a driver capability? if yes, I\u0027d like to avoid adding this if DHSS\u003dFalse.","commit_id":"5f817ffb430c32fe7f57ed70a65516073d323645"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"c51f9be49dc1bbe2119ad681c4a801f39178ede5","unresolved":true,"context_lines":[{"line_number":1315,"context_line":"            replication_domain\u003dself.replication_domain,"},{"line_number":1316,"context_line":"            filter_function\u003dself.get_filter_function(),"},{"line_number":1317,"context_line":"            goodness_function\u003dself.get_goodness_function(),"},{"line_number":1318,"context_line":"            max_shares_per_share_server\u003dself.max_shares_per_share_server,"},{"line_number":1319,"context_line":"            max_share_server_size\u003dself.max_share_server_size"},{"line_number":1320,"context_line":"        )"},{"line_number":1321,"context_line":"        if isinstance(data, dict):"},{"line_number":1322,"context_line":"            common.update(data)"},{"line_number":1323,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"bbb7a442_c94b182d","line":1320,"range":{"start_line":1318,"start_character":12,"end_line":1320,"end_character":9},"in_reply_to":"47e445af_44edde89","updated":"2021-03-07 21:53:47.000000000","message":"I\u0027d say they shouldn\u0027t be here for DHSS\u003dFalse. Good catch.\nWill fix the issue and propose a new patch.","commit_id":"5f817ffb430c32fe7f57ed70a65516073d323645"}],"manila/share/manager.py":[{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"7d0284cae292b9c0af2c43eef7078c28c69cb1c9","unresolved":true,"context_lines":[{"line_number":522,"context_line":"            share_instance_ids \u003d [si[\u0027id\u0027] for si in share_instances]"},{"line_number":523,"context_line":"            share_snapshot_instances \u003d ("},{"line_number":524,"context_line":"                self.db.share_snapshot_instance_get_all_with_filters("},{"line_number":525,"context_line":"                    context, {\"share_instance_ids\": share_instance_ids},"},{"line_number":526,"context_line":"                    with_share_data\u003dTrue))"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            server_instances_size_sum \u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"995d2940_634494ef","line":525,"updated":"2021-01-07 12:01:54.000000000","message":"A share server without share instances is possible (e.g. config delete_share_server_with_last_share \u003d False). So share_instance_ids can be empty, i.e. \u0027[]\u0027. Consequently no filter is applied and all share snapshot instances are returned.\n\nMaybe put this behind a check like \u0027if len(share_instances) \u003e 0\u0027?","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"c2593d079308cb6797a2880e2126e2d377d2615e","unresolved":false,"context_lines":[{"line_number":522,"context_line":"            share_instance_ids \u003d [si[\u0027id\u0027] for si in share_instances]"},{"line_number":523,"context_line":"            share_snapshot_instances \u003d ("},{"line_number":524,"context_line":"                self.db.share_snapshot_instance_get_all_with_filters("},{"line_number":525,"context_line":"                    context, {\"share_instance_ids\": share_instance_ids},"},{"line_number":526,"context_line":"                    with_share_data\u003dTrue))"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            server_instances_size_sum \u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"7c038f03_ec8237d0","line":525,"in_reply_to":"6e05c1d6_3b726535","updated":"2021-02-09 19:15:19.000000000","message":"Good find, carthaca!\nYeah, validating if there are share_instances is a good idea!\nThanks, carthaca and Douglas :)","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"61162042f3d157947382f73a6a70e8fb162710c9","unresolved":true,"context_lines":[{"line_number":522,"context_line":"            share_instance_ids \u003d [si[\u0027id\u0027] for si in share_instances]"},{"line_number":523,"context_line":"            share_snapshot_instances \u003d ("},{"line_number":524,"context_line":"                self.db.share_snapshot_instance_get_all_with_filters("},{"line_number":525,"context_line":"                    context, {\"share_instance_ids\": share_instance_ids},"},{"line_number":526,"context_line":"                    with_share_data\u003dTrue))"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            server_instances_size_sum \u003d 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"6e05c1d6_3b726535","line":525,"in_reply_to":"995d2940_634494ef","updated":"2021-02-01 11:59:05.000000000","message":"Good catch carthaca. \nAn idea:\n\"\nif not share_instances: \n    continue\n\" \nIt should avoid further processing, consider this share server as valid, and go to the next share server in the loop.","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"f792c298666d9cfd7e9858a7a1ad5d163494ab19","unresolved":true,"context_lines":[{"line_number":536,"context_line":""},{"line_number":537,"context_line":"            # NOTE(carloss): If a share instance was not provided, means that"},{"line_number":538,"context_line":"            # a share group is being requested and there is no need to add"},{"line_number":539,"context_line":"            # something else to the gigabytes sum"},{"line_number":540,"context_line":"            if share_instance:"},{"line_number":541,"context_line":"                server_instances_size_sum +\u003d share_instance[\u0027size\u0027]"},{"line_number":542,"context_line":"                num_instances +\u003d 1"}],"source_content_type":"text/x-python","patch_set":1,"id":"130b6118_a3311966","line":539,"updated":"2020-12-15 12:27:21.000000000","message":"It is not adding to the gigabytes sum as well as the instance number.\n\nCan you elaborate a bit, please? I\u0027m not familiar with share groups. Is a share server for a share group created directly on share group creation? So it is existing even before the first share in that group is created?","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"c2593d079308cb6797a2880e2126e2d377d2615e","unresolved":false,"context_lines":[{"line_number":536,"context_line":""},{"line_number":537,"context_line":"            # NOTE(carloss): If a share instance was not provided, means that"},{"line_number":538,"context_line":"            # a share group is being requested and there is no need to add"},{"line_number":539,"context_line":"            # something else to the gigabytes sum"},{"line_number":540,"context_line":"            if share_instance:"},{"line_number":541,"context_line":"                server_instances_size_sum +\u003d share_instance[\u0027size\u0027]"},{"line_number":542,"context_line":"                num_instances +\u003d 1"}],"source_content_type":"text/x-python","patch_set":1,"id":"26611d47_38cbe4ae","line":539,"in_reply_to":"130b6118_a3311966","updated":"2021-02-09 19:15:19.000000000","message":"Yes... The share server creation will be triggered in the moment a share group is created. So even if there aren\u0027t requests for creating shares yet, the share server will up in the backend\nI will modify this note a bit :)","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"61162042f3d157947382f73a6a70e8fb162710c9","unresolved":true,"context_lines":[{"line_number":541,"context_line":"                server_instances_size_sum +\u003d share_instance[\u0027size\u0027]"},{"line_number":542,"context_line":"                num_instances +\u003d 1"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"            achieved_gigabytes_limit \u003d ("},{"line_number":545,"context_line":"                server_instances_size_sum \u003e max_server_size \u003e -1)"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"            achieved_instances_limit \u003d num_instances \u003e max_shares_limit \u003e -1"},{"line_number":548,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"dcc8cb3d_894cfc4e","line":545,"range":{"start_line":544,"start_character":39,"end_line":545,"end_character":65},"updated":"2021-02-01 11:59:05.000000000","message":"I believe that this can become more readable, of course, if you agree. Since \u0027-1\u0027 was set as a special value for unlimited, I don\u0027t think that we should use \u0027\u003e\u0027 or \u0027\u003c\u0027. But this is just my thought.\n\nachieved_gigabytes_limit \u003d (\n    max_server_size !\u003d -1 and server_instances_size_sum \u003e max_server_size)","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"c2593d079308cb6797a2880e2126e2d377d2615e","unresolved":false,"context_lines":[{"line_number":541,"context_line":"                server_instances_size_sum +\u003d share_instance[\u0027size\u0027]"},{"line_number":542,"context_line":"                num_instances +\u003d 1"},{"line_number":543,"context_line":""},{"line_number":544,"context_line":"            achieved_gigabytes_limit \u003d ("},{"line_number":545,"context_line":"                server_instances_size_sum \u003e max_server_size \u003e -1)"},{"line_number":546,"context_line":""},{"line_number":547,"context_line":"            achieved_instances_limit \u003d num_instances \u003e max_shares_limit \u003e -1"},{"line_number":548,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f6c60744_a84f1906","line":545,"range":{"start_line":544,"start_character":39,"end_line":545,"end_character":65},"in_reply_to":"dcc8cb3d_894cfc4e","updated":"2021-02-09 19:15:19.000000000","message":"Thanks for the suggestion.\nDone!","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":30407,"name":"haixin","email":"haixin_haixin@qq.com","username":"haixin"},"change_message_id":"7c9e02185062129eb7950fe6f560bf757e3f463d","unresolved":true,"context_lines":[{"line_number":525,"context_line":"                    context, {\"share_instance_ids\": share_instance_ids},"},{"line_number":526,"context_line":"                    with_share_data\u003dTrue))"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            if not share_instances:"},{"line_number":529,"context_line":"                continue"},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"            server_instances_size_sum \u003d 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"b5605010_83eae840","line":528,"updated":"2021-03-08 06:46:01.000000000","message":"this better move to line 522, before share_instance_ids \u003d [si[\u0027id\u0027] for si in share_instances],\nbecause if not share_instances, it is not necessary to get share_instance_ids, This can improve the efficiency of the cycle,\nand here can not avoid the question  carthaca said","commit_id":"5f817ffb430c32fe7f57ed70a65516073d323645"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"e5e7c5e4c03fcbfeeff3025d119be71f522bded1","unresolved":true,"context_lines":[{"line_number":525,"context_line":"                    context, {\"share_instance_ids\": share_instance_ids},"},{"line_number":526,"context_line":"                    with_share_data\u003dTrue))"},{"line_number":527,"context_line":""},{"line_number":528,"context_line":"            if not share_instances:"},{"line_number":529,"context_line":"                continue"},{"line_number":530,"context_line":""},{"line_number":531,"context_line":"            server_instances_size_sum \u003d 0"}],"source_content_type":"text/x-python","patch_set":2,"id":"f01b5523_11a7fd6d","line":528,"in_reply_to":"b5605010_83eae840","updated":"2021-03-09 00:36:37.000000000","message":"Yeah, makes sense. That\u0027ll keep us from querying the database one more time.","commit_id":"5f817ffb430c32fe7f57ed70a65516073d323645"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"69525097b3f5490423c520dedf32d715922102b4","unresolved":true,"context_lines":[{"line_number":553,"context_line":"                share_instance and share_instance[\u0027status\u0027] \u003d\u003d"},{"line_number":554,"context_line":"                constants.STATUS_MIGRATING_TO)"},{"line_number":555,"context_line":""},{"line_number":556,"context_line":"            src_server_id_equals_current_iteration \u003d False"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"            if providing_server_for_share_migration:"},{"line_number":559,"context_line":"                share \u003d self.db.share_get(context, share_instance[\u0027share_id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"f370a9a4_4ad1be49","line":556,"range":{"start_line":556,"start_character":12,"end_line":556,"end_character":50},"updated":"2021-03-11 21:00:02.000000000","message":"\"this_is_the_share_server_of_share_being_migrated\"\n?\n\n:D Naming is hard, but I had to read your code a few times to understand what you\u0027re checking.","commit_id":"2fb871cb8991d366597e4f64a37c31ade52ada33"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"dd5365103ff50d7f0992fae26323f30cdf52733f","unresolved":true,"context_lines":[{"line_number":553,"context_line":"                share_instance and share_instance[\u0027status\u0027] \u003d\u003d"},{"line_number":554,"context_line":"                constants.STATUS_MIGRATING_TO)"},{"line_number":555,"context_line":""},{"line_number":556,"context_line":"            src_server_id_equals_current_iteration \u003d False"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"            if providing_server_for_share_migration:"},{"line_number":559,"context_line":"                share \u003d self.db.share_get(context, share_instance[\u0027share_id\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"4f30fb94_d873c051","line":556,"range":{"start_line":556,"start_character":12,"end_line":556,"end_character":50},"in_reply_to":"f370a9a4_4ad1be49","updated":"2021-03-11 23:58:17.000000000","message":"In fact naming is hard.\nI liked your variable name. I can fix it in a follow up patch or something like that","commit_id":"2fb871cb8991d366597e4f64a37c31ade52ada33"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"5f25a7388efbf0c7f3d4f72336ad2c6b752356b8","unresolved":true,"context_lines":[{"line_number":555,"context_line":""},{"line_number":556,"context_line":"            src_server_id_equals_current_iteration \u003d False"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"            if providing_server_for_share_migration:"},{"line_number":559,"context_line":"                share \u003d self.db.share_get(context, share_instance[\u0027share_id\u0027])"},{"line_number":560,"context_line":"                src_instance_id, dest_instance_id \u003d ("},{"line_number":561,"context_line":"                    self.share_api.get_migrating_instances(share))"},{"line_number":562,"context_line":"                src_instance \u003d self.db.share_instance_get("},{"line_number":563,"context_line":"                    context, src_instance_id)"},{"line_number":564,"context_line":"                src_server_id_equals_current_iteration \u003d ("},{"line_number":565,"context_line":"                    src_instance[\u0027share_server_id\u0027] \u003d\u003d ss[\u0027id\u0027])"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"            if (not src_server_id_equals_current_iteration and ("},{"line_number":568,"context_line":"                    achieved_gigabytes_limit or achieved_instances_limit)):"}],"source_content_type":"text/x-python","patch_set":3,"id":"15604bf5_5b49d3eb","line":565,"range":{"start_line":558,"start_character":0,"end_line":565,"end_character":64},"updated":"2021-03-10 19:59:45.000000000","message":"In case you upload a new PS, please add a note explaining this part.","commit_id":"2fb871cb8991d366597e4f64a37c31ade52ada33"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"69525097b3f5490423c520dedf32d715922102b4","unresolved":true,"context_lines":[{"line_number":555,"context_line":""},{"line_number":556,"context_line":"            src_server_id_equals_current_iteration \u003d False"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"            if providing_server_for_share_migration:"},{"line_number":559,"context_line":"                share \u003d self.db.share_get(context, share_instance[\u0027share_id\u0027])"},{"line_number":560,"context_line":"                src_instance_id, dest_instance_id \u003d ("},{"line_number":561,"context_line":"                    self.share_api.get_migrating_instances(share))"},{"line_number":562,"context_line":"                src_instance \u003d self.db.share_instance_get("},{"line_number":563,"context_line":"                    context, src_instance_id)"},{"line_number":564,"context_line":"                src_server_id_equals_current_iteration \u003d ("},{"line_number":565,"context_line":"                    src_instance[\u0027share_server_id\u0027] \u003d\u003d ss[\u0027id\u0027])"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"            if (not src_server_id_equals_current_iteration and ("},{"line_number":568,"context_line":"                    achieved_gigabytes_limit or achieved_instances_limit)):"}],"source_content_type":"text/x-python","patch_set":3,"id":"db7d8b4c_c978ee53","line":565,"range":{"start_line":558,"start_character":0,"end_line":565,"end_character":64},"in_reply_to":"15604bf5_5b49d3eb","updated":"2021-03-11 21:00:02.000000000","message":"+1 add a doc comment here that you don\u0027t want to disqualify the share server for a migration if a limit has been reached, since the limit won\u0027t be exceeded after the migration is complete.","commit_id":"2fb871cb8991d366597e4f64a37c31ade52ada33"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"dd5365103ff50d7f0992fae26323f30cdf52733f","unresolved":true,"context_lines":[{"line_number":555,"context_line":""},{"line_number":556,"context_line":"            src_server_id_equals_current_iteration \u003d False"},{"line_number":557,"context_line":""},{"line_number":558,"context_line":"            if providing_server_for_share_migration:"},{"line_number":559,"context_line":"                share \u003d self.db.share_get(context, share_instance[\u0027share_id\u0027])"},{"line_number":560,"context_line":"                src_instance_id, dest_instance_id \u003d ("},{"line_number":561,"context_line":"                    self.share_api.get_migrating_instances(share))"},{"line_number":562,"context_line":"                src_instance \u003d self.db.share_instance_get("},{"line_number":563,"context_line":"                    context, src_instance_id)"},{"line_number":564,"context_line":"                src_server_id_equals_current_iteration \u003d ("},{"line_number":565,"context_line":"                    src_instance[\u0027share_server_id\u0027] \u003d\u003d ss[\u0027id\u0027])"},{"line_number":566,"context_line":""},{"line_number":567,"context_line":"            if (not src_server_id_equals_current_iteration and ("},{"line_number":568,"context_line":"                    achieved_gigabytes_limit or achieved_instances_limit)):"}],"source_content_type":"text/x-python","patch_set":3,"id":"60c4fd60_ffa99ded","line":565,"range":{"start_line":558,"start_character":0,"end_line":565,"end_character":64},"in_reply_to":"db7d8b4c_c978ee53","updated":"2021-03-11 23:58:17.000000000","message":"Makes total sense","commit_id":"2fb871cb8991d366597e4f64a37c31ade52ada33"}],"manila/tests/share/test_manager.py":[{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"f792c298666d9cfd7e9858a7a1ad5d163494ab19","unresolved":true,"context_lines":[{"line_number":2259,"context_line":"    @ddt.data("},{"line_number":2260,"context_line":"        (True, 1, 3, 10, 0),"},{"line_number":2261,"context_line":"        (False, 1, 100, 5, 0),"},{"line_number":2262,"context_line":"        (True, 1, 3, 3, 0),"},{"line_number":2263,"context_line":"        (False, 1, 10, 10, 3),"},{"line_number":2264,"context_line":"        (False, 1, -1, 100, 3),"},{"line_number":2265,"context_line":"        (False, 1, 10, -1, 3),"}],"source_content_type":"text/x-python","patch_set":1,"id":"6143d3cc_a034de5a","line":2262,"updated":"2020-12-15 12:27:21.000000000","message":"The intention is to test max_gigabytes here, right? So the number for max_shares should be higher, e.g. (True, 1, 10, 3, 0)","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"c2593d079308cb6797a2880e2126e2d377d2615e","unresolved":false,"context_lines":[{"line_number":2259,"context_line":"    @ddt.data("},{"line_number":2260,"context_line":"        (True, 1, 3, 10, 0),"},{"line_number":2261,"context_line":"        (False, 1, 100, 5, 0),"},{"line_number":2262,"context_line":"        (True, 1, 3, 3, 0),"},{"line_number":2263,"context_line":"        (False, 1, 10, 10, 3),"},{"line_number":2264,"context_line":"        (False, 1, -1, 100, 3),"},{"line_number":2265,"context_line":"        (False, 1, 10, -1, 3),"}],"source_content_type":"text/x-python","patch_set":1,"id":"050d4883_7894d866","line":2262,"in_reply_to":"6143d3cc_a034de5a","updated":"2021-02-09 19:15:19.000000000","message":"Yes, you\u0027re right! Setting the third parameter to 10 makes more sense here","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":18816,"name":"Maurice Escher","display_name":"carthaca","email":"maurice.escher@sap.com","username":"mapocace"},"change_message_id":"f792c298666d9cfd7e9858a7a1ad5d163494ab19","unresolved":true,"context_lines":[{"line_number":2265,"context_line":"        (False, 1, 10, -1, 3),"},{"line_number":2266,"context_line":"    )"},{"line_number":2267,"context_line":"    @ddt.unpack"},{"line_number":2268,"context_line":"    def test__check_share_server_backend_limits("},{"line_number":2269,"context_line":"            self, with_share_instance, resource_size, max_shares,"},{"line_number":2270,"context_line":"            max_gigabytes, expected_share_servers_len):"},{"line_number":2271,"context_line":"        share_servers \u003d [db_utils.create_share_server() for i in range(3)]"}],"source_content_type":"text/x-python","patch_set":1,"id":"63a93dfe_db892133","line":2268,"updated":"2020-12-15 12:27:21.000000000","message":"Can you add a note please to sum up, what resources are set up?\nI think it is 3 share servers, with 3 share instances and 3 snapshot instances each.\nWith all being 1 GB this makes a num_instances usage of 3 per share server and a server_instances_size_sum of 6 GB per share server.\nWith the first parameter being true this adds one additional instance resource, i.e. 4 num_instances and 7 GB server_instances_size_sum to check against. Is this right?","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"},{"author":{"_account_id":29632,"name":"Carlos Eduardo","email":"ces.eduardo98@gmail.com","username":"silvacarlos"},"change_message_id":"c2593d079308cb6797a2880e2126e2d377d2615e","unresolved":false,"context_lines":[{"line_number":2265,"context_line":"        (False, 1, 10, -1, 3),"},{"line_number":2266,"context_line":"    )"},{"line_number":2267,"context_line":"    @ddt.unpack"},{"line_number":2268,"context_line":"    def test__check_share_server_backend_limits("},{"line_number":2269,"context_line":"            self, with_share_instance, resource_size, max_shares,"},{"line_number":2270,"context_line":"            max_gigabytes, expected_share_servers_len):"},{"line_number":2271,"context_line":"        share_servers \u003d [db_utils.create_share_server() for i in range(3)]"}],"source_content_type":"text/x-python","patch_set":1,"id":"9b520091_933b6417","line":2268,"in_reply_to":"63a93dfe_db892133","updated":"2021-02-09 19:15:19.000000000","message":"You\u0027re correct, carthaca!\nSo, the first parameter (with_share_instance) is just a parameter to help the test to control some stuff.\nThe thing is: when the _provide_share_server_for_share method receives a call, a share instance is present in the parameters. This share instance is forwarded to the new method, which filters out the list of available share servers according to wheter the limits for amount of instances and/or gigabytes were reached.\nIn this case, we must account the new share instance, because it can blow up the limit if for example: the limit of gigabytes is 300, the share server has only 20GB of available space, and the requested share is going to be 30GB size. If we do not check the \u0027new\u0027 share server size accounting the share instance, it could get to 310GB.\nBut when a share group is being requested and the _provide_share_server_for_share_group method is called, a share instance won\u0027t be forwarded to this method, so there is no need to account one more instance, neither it\u0027s size, considering that it is going to be a brand new share group.\nSo the \u0027with_share_instance\u0027 parameter can sound a bit confusing, but its idea here is just to help the test to cover both share and share group creation scenario.\nI have added some comments and a note in the test, and I hope it became a bit more clear now.\nIf you to have any other suggestion on how to improve it or any question, please let me know :)","commit_id":"3528996c15e2ebece7fdc1fc01f6877aad94cb3e"}]}