)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"Access rules for each share instance now transition"},{"line_number":19,"context_line":"from \u0027new\u0027 to \u0027applying\u0027 to \u0027active\u0027 or \u0027error\u0027; and from"},{"line_number":20,"context_line":"\u0027active\u0027, \u0027new\u0027, \u0027applying\u0027 or \u0027error\u0027 to \u0027deleted\u0027."},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"APIImpact"},{"line_number":23,"context_line":"DocImpact"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"da6895a0_3068cbf5","line":20,"range":{"start_line":20,"start_character":42,"end_line":20,"end_character":50},"updated":"2016-10-21 11:29:05.000000000","message":"to \u0027denying\u0027, then \u0027deleted\u0027.","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"300d6ee6636bd73b0c950228ad54d4f5ecbf65ea","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactor Access Rules APIs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Pull up policy check to beginning of the APIs"},{"line_number":10,"context_line":"- Avoid making access rules changes when one or"},{"line_number":11,"context_line":"  more instances of the share are in an invalid state."},{"line_number":12,"context_line":"- Add back the per rule share instance access status."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a3c09a3_6887f5b4","line":9,"updated":"2017-01-18 17:37:43.000000000","message":"nit: period at end of sentence.","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0d6fd79d3868c7c0b4a2f18d8d4aedf3a75c35ed","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Refactor Access Rules APIs"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"- Pull up policy check to beginning of the APIs"},{"line_number":10,"context_line":"- Avoid making access rules changes when one or"},{"line_number":11,"context_line":"  more instances of the share are in an invalid state."},{"line_number":12,"context_line":"- Add back the per rule share instance access status."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a3c09a3_19cdda65","line":9,"in_reply_to":"7a3c09a3_6887f5b4","updated":"2017-01-18 19:20:56.000000000","message":"will fix.","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"}],"api-ref/source/parameters.yaml":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"53117fddfbc35107396e9c4dce4b987783b4d14a","unresolved":false,"context_lines":[{"line_number":506,"context_line":"  type: string"},{"line_number":507,"context_line":"access_rules_status:"},{"line_number":508,"context_line":"  description: |"},{"line_number":509,"context_line":"    The share instance access rules status. Valid value are ``active``,"},{"line_number":510,"context_line":"    ``error``, and ``syncing``. In versions prior to 2.26, ``syncing`` was"},{"line_number":511,"context_line":"    represented with status ``out_of_sync``."},{"line_number":512,"context_line":"  in: body"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"ba5201f7_dd2f3e9f","line":509,"range":{"start_line":509,"start_character":50,"end_line":509,"end_character":55},"updated":"2017-01-11 12:58:17.000000000","message":"values","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"94b36afdd21f5f892aed9d923c7b0fdba4c77c3c","unresolved":false,"context_lines":[{"line_number":506,"context_line":"  type: string"},{"line_number":507,"context_line":"access_rules_status:"},{"line_number":508,"context_line":"  description: |"},{"line_number":509,"context_line":"    The share instance access rules status. Valid value are ``active``,"},{"line_number":510,"context_line":"    ``error``, and ``syncing``. In versions prior to 2.26, ``syncing`` was"},{"line_number":511,"context_line":"    represented with status ``out_of_sync``."},{"line_number":512,"context_line":"  in: body"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"ba5201f7_7532da5f","line":509,"range":{"start_line":509,"start_character":50,"end_line":509,"end_character":55},"in_reply_to":"ba5201f7_dd2f3e9f","updated":"2017-01-11 16:32:17.000000000","message":"Done","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"53117fddfbc35107396e9c4dce4b987783b4d14a","unresolved":false,"context_lines":[{"line_number":3100,"context_line":"    Prior to versions 2.26, the state of all access rules of a given share"},{"line_number":3101,"context_line":"    is the same at all times. This could be ``new``, ``active`` or"},{"line_number":3102,"context_line":"    ``error``. Since 2.26, the status of each access rule of a share is"},{"line_number":3103,"context_line":"    independant of the others and can be ``queued_to_apply``,"},{"line_number":3104,"context_line":"    ``applying``, ``active``, ``error``, ``queued_to_deny`` and ``denying``."},{"line_number":3105,"context_line":"  in: body"},{"line_number":3106,"context_line":"  required: true"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"ba5201f7_a013315b","line":3103,"range":{"start_line":3103,"start_character":4,"end_line":3103,"end_character":15},"updated":"2017-01-11 12:58:17.000000000","message":"independent","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"94b36afdd21f5f892aed9d923c7b0fdba4c77c3c","unresolved":false,"context_lines":[{"line_number":3100,"context_line":"    Prior to versions 2.26, the state of all access rules of a given share"},{"line_number":3101,"context_line":"    is the same at all times. This could be ``new``, ``active`` or"},{"line_number":3102,"context_line":"    ``error``. Since 2.26, the status of each access rule of a share is"},{"line_number":3103,"context_line":"    independant of the others and can be ``queued_to_apply``,"},{"line_number":3104,"context_line":"    ``applying``, ``active``, ``error``, ``queued_to_deny`` and ``denying``."},{"line_number":3105,"context_line":"  in: body"},{"line_number":3106,"context_line":"  required: true"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"ba5201f7_f546cabe","line":3103,"range":{"start_line":3103,"start_character":4,"end_line":3103,"end_character":15},"in_reply_to":"ba5201f7_a013315b","updated":"2017-01-11 16:32:17.000000000","message":"Done","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":507,"context_line":"access_rules_status:"},{"line_number":508,"context_line":"  description: |"},{"line_number":509,"context_line":"    The share instance access rules status. A valid value is ``active``,"},{"line_number":510,"context_line":"    ``error``, and ``syncing``. In versions prior to 2.27, ``syncing`` was"},{"line_number":511,"context_line":"    represented with status ``out_of_sync``."},{"line_number":512,"context_line":"  in: body"},{"line_number":513,"context_line":"  required: true"}],"source_content_type":"text/x-yaml","patch_set":16,"id":"7a3c09a3_cdf78589","line":510,"range":{"start_line":510,"start_character":15,"end_line":510,"end_character":18},"updated":"2017-01-17 01:53:28.000000000","message":"s/and/or/  It would sound more normal to use \"Valid values... are\", but I guess you might be intentionally making it singular (only pick one) so if you want to use \"is\" then you should use \"or\".","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":507,"context_line":"access_rules_status:"},{"line_number":508,"context_line":"  description: |"},{"line_number":509,"context_line":"    The share instance access rules status. A valid value is ``active``,"},{"line_number":510,"context_line":"    ``error``, and ``syncing``. In versions prior to 2.27, ``syncing`` was"},{"line_number":511,"context_line":"    represented with status ``out_of_sync``."},{"line_number":512,"context_line":"  in: body"},{"line_number":513,"context_line":"  required: true"}],"source_content_type":"text/x-yaml","patch_set":16,"id":"7a3c09a3_5c2889e2","line":510,"range":{"start_line":510,"start_character":15,"end_line":510,"end_character":18},"in_reply_to":"7a3c09a3_cdf78589","updated":"2017-01-17 13:47:35.000000000","message":"+1 Thanks.","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":3086,"context_line":"    is the same at all times. This could be ``new``, ``active`` or"},{"line_number":3087,"context_line":"    ``error``. Since 2.27, the state of each access rule of a share is"},{"line_number":3088,"context_line":"    independent of the others and can be ``queued_to_apply``,"},{"line_number":3089,"context_line":"    ``applying``, ``active``, ``error``, ``queued_to_deny`` and ``denying``."},{"line_number":3090,"context_line":"    A new rule starts out in ``queued_to_apply`` state and is successfully"},{"line_number":3091,"context_line":"    applied if it transitions to ``active`` state."},{"line_number":3092,"context_line":"  in: body"}],"source_content_type":"text/x-yaml","patch_set":16,"id":"7a3c09a3_cd25c509","line":3089,"range":{"start_line":3089,"start_character":60,"end_line":3089,"end_character":63},"updated":"2017-01-17 01:53:28.000000000","message":"s/and/or/","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":3086,"context_line":"    is the same at all times. This could be ``new``, ``active`` or"},{"line_number":3087,"context_line":"    ``error``. Since 2.27, the state of each access rule of a share is"},{"line_number":3088,"context_line":"    independent of the others and can be ``queued_to_apply``,"},{"line_number":3089,"context_line":"    ``applying``, ``active``, ``error``, ``queued_to_deny`` and ``denying``."},{"line_number":3090,"context_line":"    A new rule starts out in ``queued_to_apply`` state and is successfully"},{"line_number":3091,"context_line":"    applied if it transitions to ``active`` state."},{"line_number":3092,"context_line":"  in: body"}],"source_content_type":"text/x-yaml","patch_set":16,"id":"7a3c09a3_9c4cb1c9","line":3089,"range":{"start_line":3089,"start_character":60,"end_line":3089,"end_character":63},"in_reply_to":"7a3c09a3_cd25c509","updated":"2017-01-17 13:47:35.000000000","message":"Done","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"}],"api-ref/source/share-actions.inc":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"53117fddfbc35107396e9c4dce4b987783b4d14a","unresolved":false,"context_lines":[{"line_number":48,"context_line":"methods:"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"- ``ip``. Authenticates an instance through its IP address. A valid"},{"line_number":51,"context_line":"  format is ``XX.XX.XX.XX`` or ``XX.XX.XX.XX/XX``. For example"},{"line_number":52,"context_line":"  ``0.0.0.0/0``."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"- ``cert``. Authenticates an instance through a TLS certificate."}],"source_content_type":"text/x-c++src","patch_set":8,"id":"ba5201f7_a0afb1e1","line":51,"range":{"start_line":51,"start_character":14,"end_line":51,"end_character":44},"updated":"2017-01-11 12:58:17.000000000","message":"nit: I guess those should be XXX.XXX.XXX.XXX","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"94b36afdd21f5f892aed9d923c7b0fdba4c77c3c","unresolved":false,"context_lines":[{"line_number":48,"context_line":"methods:"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"- ``ip``. Authenticates an instance through its IP address. A valid"},{"line_number":51,"context_line":"  format is ``XX.XX.XX.XX`` or ``XX.XX.XX.XX/XX``. For example"},{"line_number":52,"context_line":"  ``0.0.0.0/0``."},{"line_number":53,"context_line":""},{"line_number":54,"context_line":"- ``cert``. Authenticates an instance through a TLS certificate."}],"source_content_type":"text/x-c++src","patch_set":8,"id":"ba5201f7_55c236ea","line":51,"range":{"start_line":51,"start_character":14,"end_line":51,"end_character":44},"in_reply_to":"ba5201f7_a0afb1e1","updated":"2017-01-11 16:32:17.000000000","message":"We\u0027ll need to revisit this with the ipv6 changes...","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"}],"manila/api/openstack/api_version_request.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"53117fddfbc35107396e9c4dce4b987783b4d14a","unresolved":false,"context_lines":[{"line_number":87,"context_line":"             transitional access_rules_status values of"},{"line_number":88,"context_line":"             shares (share_instances) with \u0027syncing\u0027. Share action API"},{"line_number":89,"context_line":"             \u0027access_allow\u0027 now accepts rules even when a share or any of"},{"line_number":90,"context_line":"             its instances may have an access_rules_status set to \u0027error\u0027."},{"line_number":91,"context_line":"\"\"\""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"# The minimum and maximum versions of the API supported"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_c06d959a","line":90,"range":{"start_line":90,"start_character":39,"end_line":90,"end_character":74},"updated":"2017-01-11 12:58:17.000000000","message":"or status of \u0027migrating\u0027. \n\nFrom the spec: \n\n\"Adding access rules while the share is being migrated will be possible, as long as all share instances have a valid host. This behavior will be exposed only from a new micro-version of the allow-access API.\"","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"94b36afdd21f5f892aed9d923c7b0fdba4c77c3c","unresolved":false,"context_lines":[{"line_number":87,"context_line":"             transitional access_rules_status values of"},{"line_number":88,"context_line":"             shares (share_instances) with \u0027syncing\u0027. Share action API"},{"line_number":89,"context_line":"             \u0027access_allow\u0027 now accepts rules even when a share or any of"},{"line_number":90,"context_line":"             its instances may have an access_rules_status set to \u0027error\u0027."},{"line_number":91,"context_line":"\"\"\""},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"# The minimum and maximum versions of the API supported"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_3564d220","line":90,"range":{"start_line":90,"start_character":39,"end_line":90,"end_character":74},"in_reply_to":"ba5201f7_c06d959a","updated":"2017-01-11 16:32:17.000000000","message":"Not in this microversion.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"}],"manila/api/openstack/rest_api_version_history.rst":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"53117fddfbc35107396e9c4dce4b987783b4d14a","unresolved":false,"context_lines":[{"line_number":154,"context_line":""},{"line_number":155,"context_line":"2.26"},{"line_number":156,"context_line":"----"},{"line_number":157,"context_line":"  Added transitional states (\u0027queued_to_apply\u0027 - was previously \u0027new\u0027,"},{"line_number":158,"context_line":"  \u0027queued_to_deny\u0027, \u0027applying\u0027 and \u0027denying\u0027) to access rules."},{"line_number":159,"context_line":"  \u0027updating\u0027, \u0027updating_multiple\u0027 and \u0027out_of_sync\u0027 are no longer valid"},{"line_number":160,"context_line":"  values for the \u0027access_rules_status\u0027 field of shares, they have"},{"line_number":161,"context_line":"  been collapsed into the transitional state \u0027syncing\u0027. Access rule changes"},{"line_number":162,"context_line":"  can be made independent of a share\u0027s \u0027access_rules_status\u0027."}],"source_content_type":"text/x-rst","patch_set":8,"id":"ba5201f7_809c6d5e","line":162,"range":{"start_line":157,"start_character":2,"end_line":162,"end_character":61},"updated":"2017-01-11 12:58:17.000000000","message":"doesn\u0027t it make sense to have the same description as in the api_version_request.py file?","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"94b36afdd21f5f892aed9d923c7b0fdba4c77c3c","unresolved":false,"context_lines":[{"line_number":154,"context_line":""},{"line_number":155,"context_line":"2.26"},{"line_number":156,"context_line":"----"},{"line_number":157,"context_line":"  Added transitional states (\u0027queued_to_apply\u0027 - was previously \u0027new\u0027,"},{"line_number":158,"context_line":"  \u0027queued_to_deny\u0027, \u0027applying\u0027 and \u0027denying\u0027) to access rules."},{"line_number":159,"context_line":"  \u0027updating\u0027, \u0027updating_multiple\u0027 and \u0027out_of_sync\u0027 are no longer valid"},{"line_number":160,"context_line":"  values for the \u0027access_rules_status\u0027 field of shares, they have"},{"line_number":161,"context_line":"  been collapsed into the transitional state \u0027syncing\u0027. Access rule changes"},{"line_number":162,"context_line":"  can be made independent of a share\u0027s \u0027access_rules_status\u0027."}],"source_content_type":"text/x-rst","patch_set":8,"id":"ba5201f7_d52386bb","line":162,"range":{"start_line":157,"start_character":2,"end_line":162,"end_character":61},"in_reply_to":"ba5201f7_809c6d5e","updated":"2017-01-11 16:32:17.000000000","message":"That one is more succinct.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"}],"manila/api/v1/shares.py":[{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        if (not allow_on_error_status and"},{"line_number":420,"context_line":"                self._any_instance_has_errored_rules(share)):"},{"line_number":421,"context_line":"            msg \u003d _(\"Access rules cannot be added while the share, any of its \""},{"line_number":422,"context_line":"                    \"replicas or migration copies has its \""},{"line_number":423,"context_line":"                    \"access_rules_status set to %s. Deny any error-ed rules \""},{"line_number":424,"context_line":"                    \"and try again.\") % constants.SHARE_INSTANCE_RULES_ERROR"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_b87ef916","line":421,"range":{"start_line":421,"start_character":65,"end_line":421,"end_character":66},"updated":"2017-01-17 01:53:28.000000000","message":"replace comma with \u0027or\u0027 so it is like \"its-replicas\" or \"its-mig-copies\"","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":418,"context_line":""},{"line_number":419,"context_line":"        if (not allow_on_error_status and"},{"line_number":420,"context_line":"                self._any_instance_has_errored_rules(share)):"},{"line_number":421,"context_line":"            msg \u003d _(\"Access rules cannot be added while the share, any of its \""},{"line_number":422,"context_line":"                    \"replicas or migration copies has its \""},{"line_number":423,"context_line":"                    \"access_rules_status set to %s. Deny any error-ed rules \""},{"line_number":424,"context_line":"                    \"and try again.\") % constants.SHARE_INSTANCE_RULES_ERROR"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_3cebbd79","line":421,"range":{"start_line":421,"start_character":65,"end_line":421,"end_character":66},"in_reply_to":"7a3c09a3_b87ef916","updated":"2017-01-17 13:47:35.000000000","message":"Done","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":420,"context_line":"                self._any_instance_has_errored_rules(share)):"},{"line_number":421,"context_line":"            msg \u003d _(\"Access rules cannot be added while the share, any of its \""},{"line_number":422,"context_line":"                    \"replicas or migration copies has its \""},{"line_number":423,"context_line":"                    \"access_rules_status set to %s. Deny any error-ed rules \""},{"line_number":424,"context_line":"                    \"and try again.\") % constants.SHARE_INSTANCE_RULES_ERROR"},{"line_number":425,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":426,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_989cdd5e","line":423,"range":{"start_line":423,"start_character":61,"end_line":423,"end_character":75},"updated":"2017-01-17 01:53:28.000000000","message":"\"error-ed\" is OK by me, but maybe s/error-ed rules/rules in error states/?","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":420,"context_line":"                self._any_instance_has_errored_rules(share)):"},{"line_number":421,"context_line":"            msg \u003d _(\"Access rules cannot be added while the share, any of its \""},{"line_number":422,"context_line":"                    \"replicas or migration copies has its \""},{"line_number":423,"context_line":"                    \"access_rules_status set to %s. Deny any error-ed rules \""},{"line_number":424,"context_line":"                    \"and try again.\") % constants.SHARE_INSTANCE_RULES_ERROR"},{"line_number":425,"context_line":"            raise webob.exc.HTTPBadRequest(explanation\u003dmsg)"},{"line_number":426,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_5c8b29c3","line":423,"range":{"start_line":423,"start_character":61,"end_line":423,"end_character":75},"in_reply_to":"7a3c09a3_989cdd5e","updated":"2017-01-17 13:47:35.000000000","message":"Done","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"}],"manila/api/v2/shares.py":[{"author":{"_account_id":23083,"name":"TommyLike","email":"tommylikehu@gmail.com","username":"TommyLike"},"change_message_id":"9152856c29c8234e1cba6d3434a242bf21ee66a5","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        \"\"\"Add share access rule.\"\"\""},{"line_number":326,"context_line":"        args \u003d (req, id, body)"},{"line_number":327,"context_line":"        kwargs \u003d {}"},{"line_number":328,"context_line":"        if req.api_version_request \u003e\u003d api_version.APIVersionRequest(\"2.13\"):"},{"line_number":329,"context_line":"            kwargs[\u0027enable_ceph\u0027] \u003d True"},{"line_number":330,"context_line":"        if req.api_version_request \u003e\u003d api_version.APIVersionRequest(\"2.28\"):"},{"line_number":331,"context_line":"            kwargs[\u0027allow_on_error_status\u0027] \u003d True"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        return self._allow_access(*args, **kwargs)"},{"line_number":334,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_f0d57eb9","line":331,"range":{"start_line":328,"start_character":7,"end_line":331,"end_character":50},"updated":"2017-01-19 02:16:21.000000000","message":"We can use microversion for actions","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"833411b7e0d15f7fc2b7aec67a84577ae69c91a1","unresolved":false,"context_lines":[{"line_number":325,"context_line":"        \"\"\"Add share access rule.\"\"\""},{"line_number":326,"context_line":"        args \u003d (req, id, body)"},{"line_number":327,"context_line":"        kwargs \u003d {}"},{"line_number":328,"context_line":"        if req.api_version_request \u003e\u003d api_version.APIVersionRequest(\"2.13\"):"},{"line_number":329,"context_line":"            kwargs[\u0027enable_ceph\u0027] \u003d True"},{"line_number":330,"context_line":"        if req.api_version_request \u003e\u003d api_version.APIVersionRequest(\"2.28\"):"},{"line_number":331,"context_line":"            kwargs[\u0027allow_on_error_status\u0027] \u003d True"},{"line_number":332,"context_line":""},{"line_number":333,"context_line":"        return self._allow_access(*args, **kwargs)"},{"line_number":334,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_d6823ac8","line":331,"range":{"start_line":328,"start_character":7,"end_line":331,"end_character":50},"in_reply_to":"5a3905b3_f0d57eb9","updated":"2017-01-19 02:51:37.000000000","message":"You actually sound right, Tommy, I tested this change by using proper micro versioning.. i wonder what vponomaryov and ganso found that I am not seeing:\n\nhttps://bugs.launchpad.net/manila/+bug/1512403","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"}],"manila/common/constants.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"834295b0ca82356b35f0ef7cf910cae7e2db325a","unresolved":false,"context_lines":[{"line_number":41,"context_line":"STATUS_NEW \u003d \u0027new\u0027"},{"line_number":42,"context_line":"STATUS_APPLYING \u003d \u0027applying\u0027"},{"line_number":43,"context_line":"STATUS_OUT_OF_SYNC \u003d \u0027out_of_sync\u0027"},{"line_number":44,"context_line":"STATUS_ACTIVE \u003d \u0027active\u0027"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"TASK_STATE_MIGRATION_STARTING \u003d \u0027migration_starting\u0027"},{"line_number":47,"context_line":"TASK_STATE_MIGRATION_IN_PROGRESS \u003d \u0027migration_in_progress\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_3b41903a","line":44,"range":{"start_line":44,"start_character":0,"end_line":44,"end_character":24},"updated":"2016-09-13 23:50:21.000000000","message":"this is used by share servers as well","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"d83193eae514079b87da057c06c5d86f7f6b1049","unresolved":false,"context_lines":[{"line_number":41,"context_line":"STATUS_NEW \u003d \u0027new\u0027"},{"line_number":42,"context_line":"STATUS_APPLYING \u003d \u0027applying\u0027"},{"line_number":43,"context_line":"STATUS_OUT_OF_SYNC \u003d \u0027out_of_sync\u0027"},{"line_number":44,"context_line":"STATUS_ACTIVE \u003d \u0027active\u0027"},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"TASK_STATE_MIGRATION_STARTING \u003d \u0027migration_starting\u0027"},{"line_number":47,"context_line":"TASK_STATE_MIGRATION_IN_PROGRESS \u003d \u0027migration_in_progress\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_b6e2eed3","line":44,"range":{"start_line":44,"start_character":0,"end_line":44,"end_character":24},"in_reply_to":"7a8ec9b2_3b41903a","updated":"2016-09-14 17:39:32.000000000","message":"added a comment","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"53117fddfbc35107396e9c4dce4b987783b4d14a","unresolved":false,"context_lines":[{"line_number":49,"context_line":"SHARE_INSTANCE_RULES_SYNCING \u003d \u0027syncing\u0027"},{"line_number":50,"context_line":"SHARE_INSTANCE_RULES_ERROR \u003d \u0027error\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"# States for multiple resources."},{"line_number":53,"context_line":"STATUS_NEW \u003d \u0027new\u0027"},{"line_number":54,"context_line":"STATUS_OUT_OF_SYNC \u003d \u0027out_of_sync\u0027"},{"line_number":55,"context_line":"STATUS_ACTIVE \u003d \u0027active\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_f3fef1ac","line":52,"range":{"start_line":52,"start_character":0,"end_line":52,"end_character":32},"updated":"2017-01-11 12:58:17.000000000","message":"nit: I guess this one should be all caps or the others should be lowercase. Also, be consistent with the period","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"94b36afdd21f5f892aed9d923c7b0fdba4c77c3c","unresolved":false,"context_lines":[{"line_number":49,"context_line":"SHARE_INSTANCE_RULES_SYNCING \u003d \u0027syncing\u0027"},{"line_number":50,"context_line":"SHARE_INSTANCE_RULES_ERROR \u003d \u0027error\u0027"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"# States for multiple resources."},{"line_number":53,"context_line":"STATUS_NEW \u003d \u0027new\u0027"},{"line_number":54,"context_line":"STATUS_OUT_OF_SYNC \u003d \u0027out_of_sync\u0027"},{"line_number":55,"context_line":"STATUS_ACTIVE \u003d \u0027active\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_95149e4d","line":52,"range":{"start_line":52,"start_character":0,"end_line":52,"end_character":32},"in_reply_to":"ba5201f7_f3fef1ac","updated":"2017-01-11 16:32:17.000000000","message":"I\u0027ll lowercase the rest.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"}],"manila/data/helper.py":[{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                self.context, access_ref[\u0027id\u0027], updates\u003dupdates,"},{"line_number":128,"context_line":"                share_instance_id\u003dinstance[\u0027id\u0027])"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        self.share_rpc.update_access(self.context, instance)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        utils.wait_for_access_update("},{"line_number":133,"context_line":"            self.context, self.db, instance, self.wait_access_rules_timeout)"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_0880988a","line":130,"updated":"2017-01-17 01:53:28.000000000","message":"if this is now a bulk update-access instead of a just an allow|deny then it shouldn\u0027t be called in a loop like below. L175","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":127,"context_line":"                self.context, access_ref[\u0027id\u0027], updates\u003dupdates,"},{"line_number":128,"context_line":"                share_instance_id\u003dinstance[\u0027id\u0027])"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"        self.share_rpc.update_access(self.context, instance)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        utils.wait_for_access_update("},{"line_number":133,"context_line":"            self.context, self.db, instance, self.wait_access_rules_timeout)"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_f764eeb9","line":130,"in_reply_to":"7a3c09a3_0880988a","updated":"2017-01-17 13:47:35.000000000","message":"Good catch! Fixed","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":172,"context_line":"                self.context, self.share[\u0027id\u0027], access[\u0027access_type\u0027],"},{"line_number":173,"context_line":"                access[\u0027access_to\u0027])"},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"            # Create new access rule and deny all old ones corresponding to"},{"line_number":176,"context_line":"            # the mapping. Since this is a bulk update, all access changes"},{"line_number":177,"context_line":"            # are made in one go."},{"line_number":178,"context_line":"            access_ref \u003d self.db.share_instance_access_create("},{"line_number":179,"context_line":"                self.context, values, share_instance[\u0027id\u0027])"},{"line_number":180,"context_line":"            self._change_data_access_to_instance("},{"line_number":181,"context_line":"                share_instance, old_access_list, deny\u003dTrue)"},{"line_number":182,"context_line":""},{"line_number":183,"context_line":"            if allow_access_to_destination_instance:"},{"line_number":184,"context_line":"                access_ref \u003d self.db.share_instance_access_create("}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_dfb45d36","line":181,"range":{"start_line":175,"start_character":12,"end_line":181,"end_character":59},"updated":"2017-01-17 17:41:48.000000000","message":"+1","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"}],"manila/db/migrations/alembic/versions/344c1ac4747f_add_share_instance_access_rules_status.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"53117fddfbc35107396e9c4dce4b987783b4d14a","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    for instance in connection.execute(instances_query):"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        # NOTE(u_glide): We cannot determine is it applied rule or not in"},{"line_number":116,"context_line":"        # Manila, so administrator should manually handle such access rules."},{"line_number":117,"context_line":"        if instance[\u0027access_rules_status\u0027] \u003d\u003d \u0027active\u0027:"},{"line_number":118,"context_line":"            state \u003d \u0027active\u0027"},{"line_number":119,"context_line":"        else:"},{"line_number":120,"context_line":"            state \u003d \u0027error\u0027"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        op.execute("},{"line_number":123,"context_line":"            instance_access_table.update().where("}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_9302053d","line":120,"range":{"start_line":115,"start_character":8,"end_line":120,"end_character":27},"updated":"2017-01-11 12:58:17.000000000","message":"why this change? it is a good refactor, but logic is the same and doesn\u0027t seem to impact current versions, since your version will need to be downgraded first.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"94b36afdd21f5f892aed9d923c7b0fdba4c77c3c","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    for instance in connection.execute(instances_query):"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        # NOTE(u_glide): We cannot determine is it applied rule or not in"},{"line_number":116,"context_line":"        # Manila, so administrator should manually handle such access rules."},{"line_number":117,"context_line":"        if instance[\u0027access_rules_status\u0027] \u003d\u003d \u0027active\u0027:"},{"line_number":118,"context_line":"            state \u003d \u0027active\u0027"},{"line_number":119,"context_line":"        else:"},{"line_number":120,"context_line":"            state \u003d \u0027error\u0027"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        op.execute("},{"line_number":123,"context_line":"            instance_access_table.update().where("}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_a7a7b909","line":120,"range":{"start_line":115,"start_character":8,"end_line":120,"end_character":27},"in_reply_to":"ba5201f7_9302053d","updated":"2017-01-11 16:32:17.000000000","message":"The downgrade_data_mapping was out of date with the new access_rules_status values that were introduced after this migration; specifically \"updating\", \"updating_multiple\"","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    for instance in connection.execute(instances_query):"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        # NOTE(u_glide): We cannot determine is it applied rule or not in"},{"line_number":116,"context_line":"        # Manila, so administrator should manually handle such access rules."},{"line_number":117,"context_line":"        if instance[\u0027access_rules_status\u0027] \u003d\u003d \u0027active\u0027:"},{"line_number":118,"context_line":"            state \u003d \u0027active\u0027"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_fbe318a2","line":115,"range":{"start_line":115,"start_character":45,"end_line":115,"end_character":47},"updated":"2017-01-17 01:53:28.000000000","message":"s/is/if/","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    for instance in connection.execute(instances_query):"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        # NOTE(u_glide): We cannot determine is it applied rule or not in"},{"line_number":116,"context_line":"        # Manila, so administrator should manually handle such access rules."},{"line_number":117,"context_line":"        if instance[\u0027access_rules_status\u0027] \u003d\u003d \u0027active\u0027:"},{"line_number":118,"context_line":"            state \u003d \u0027active\u0027"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_f70b0ed4","line":115,"range":{"start_line":115,"start_character":45,"end_line":115,"end_character":47},"in_reply_to":"7a3c09a3_fbe318a2","updated":"2017-01-17 13:47:35.000000000","message":"Done","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"9c8a55b43193d9632f7202b4018d12f05ef3edc3","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    \u0027active\u0027: \u0027active\u0027,"},{"line_number":41,"context_line":"    \u0027new\u0027: \u0027out_of_sync\u0027,"},{"line_number":42,"context_line":"    \u0027error\u0027: \u0027error\u0027,"},{"line_number":43,"context_line":"}"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_fa32040f","line":43,"updated":"2017-01-17 17:57:11.000000000","message":"Change in this file looks redundant.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":40,"context_line":"    \u0027active\u0027: \u0027active\u0027,"},{"line_number":41,"context_line":"    \u0027new\u0027: \u0027out_of_sync\u0027,"},{"line_number":42,"context_line":"    \u0027error\u0027: \u0027error\u0027,"},{"line_number":43,"context_line":"}"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":""},{"line_number":46,"context_line":"def upgrade():"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_cd5d9016","line":43,"in_reply_to":"7a3c09a3_fa32040f","updated":"2017-01-17 18:01:57.000000000","message":"it\u0027s not... The upgrade data mapping here does not include \u0027updating\u0027 and \u0027updating_multiple\u0027... These were added after this migration file was written. If I use these values in migration data checks, things begin to fail with KeyError.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"87484e6f5aba5ac7b059547d48482688c76419fc","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        .where(share_instances_table.c.deleted \u003d\u003d \u0027False\u0027)"},{"line_number":111,"context_line":"    )"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"    for instance in connection.execute(instances_query):"},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"        # NOTE(u_glide): We cannot determine if a rule is applied or not in"},{"line_number":116,"context_line":"        # Manila, so administrator should manually handle such access rules."},{"line_number":117,"context_line":"        if instance[\u0027access_rules_status\u0027] \u003d\u003d \u0027active\u0027:"},{"line_number":118,"context_line":"            state \u003d \u0027active\u0027"},{"line_number":119,"context_line":"        else:"},{"line_number":120,"context_line":"            state \u003d \u0027error\u0027"},{"line_number":121,"context_line":""},{"line_number":122,"context_line":"        op.execute("}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_df0321bd","line":119,"range":{"start_line":113,"start_character":0,"end_line":119,"end_character":13},"updated":"2017-01-18 23:52:59.000000000","message":"OK :D","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"}],"manila/db/migrations/alembic/versions/54667b9cade7_restore_share_instance_access_map_state.py":[{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"3dc6260bd2da19f62c70d0d5af5fcf32e38b45af","unresolved":false,"context_lines":[{"line_number":60,"context_line":"                                             connection)"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"    instances_query \u003d ("},{"line_number":63,"context_line":"        share_instances_table.select()"},{"line_number":64,"context_line":"        .where(share_instances_table.c.status \u003d\u003d constants.STATUS_AVAILABLE)"},{"line_number":65,"context_line":"        .where(share_instances_table.c.deleted \u003d\u003d \u0027False\u0027)"},{"line_number":66,"context_line":"    )"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    for instance in connection.execute(instances_query):"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a95cdbc_975c1327","line":65,"range":{"start_line":63,"start_character":8,"end_line":65,"end_character":58},"updated":"2016-10-05 10:51:27.000000000","message":"this","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"3dc6260bd2da19f62c70d0d5af5fcf32e38b45af","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        access_rule_status \u003d instance[\u0027access_rules_status\u0027]"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"        op.execute("},{"line_number":72,"context_line":"            instance_access_table.update().where("},{"line_number":73,"context_line":"                instance_access_table.c.share_instance_id \u003d\u003d instance[\u0027id\u0027]"},{"line_number":74,"context_line":"            ).values({"},{"line_number":75,"context_line":"                \u0027state\u0027: access_rules_status_to_state_mapping["},{"line_number":76,"context_line":"                    access_rule_status],"}],"source_content_type":"text/x-python","patch_set":3,"id":"1a95cdbc_b7410f3c","line":73,"range":{"start_line":72,"start_character":12,"end_line":73,"end_character":75},"updated":"2016-10-05 10:51:27.000000000","message":"and this are inconsistent with each other. And this one preffered one to be used.","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"2d1024b82b42f35964fd09e631b38b2487e12221","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    for instance in connection.execute(instances_query):"},{"line_number":68,"context_line":"        access_rule_status \u003d instance[\u0027access_rules_status\u0027]"},{"line_number":69,"context_line":"        op.execute("},{"line_number":70,"context_line":"            instance_access_map_table.update().where("},{"line_number":71,"context_line":"                instance_access_map_table.c.share_instance_id \u003d\u003d instance[\u0027id\u0027]"},{"line_number":72,"context_line":"            ).values({"},{"line_number":73,"context_line":"                \u0027state\u0027: access_rules_status_to_state_mapping["},{"line_number":74,"context_line":"                    access_rule_status],"},{"line_number":75,"context_line":"            })"},{"line_number":76,"context_line":"        )"},{"line_number":77,"context_line":"        op.execute("},{"line_number":78,"context_line":"            share_instances_table.update().where("},{"line_number":79,"context_line":"                share_instances_table.c.id \u003d\u003d instance[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_2d5534f9","line":76,"range":{"start_line":69,"start_character":8,"end_line":76,"end_character":9},"updated":"2017-01-17 18:11:42.000000000","message":"I like this style and you use different one on lines 61-65. Would be better to make them consistent.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"609d56a9ec4c9f9bc891e43fff679cd4ae8ef9fc","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    for instance in connection.execute(instances_query):"},{"line_number":68,"context_line":"        access_rule_status \u003d instance[\u0027access_rules_status\u0027]"},{"line_number":69,"context_line":"        op.execute("},{"line_number":70,"context_line":"            instance_access_map_table.update().where("},{"line_number":71,"context_line":"                instance_access_map_table.c.share_instance_id \u003d\u003d instance[\u0027id\u0027]"},{"line_number":72,"context_line":"            ).values({"},{"line_number":73,"context_line":"                \u0027state\u0027: access_rules_status_to_state_mapping["},{"line_number":74,"context_line":"                    access_rule_status],"},{"line_number":75,"context_line":"            })"},{"line_number":76,"context_line":"        )"},{"line_number":77,"context_line":"        op.execute("},{"line_number":78,"context_line":"            share_instances_table.update().where("},{"line_number":79,"context_line":"                share_instances_table.c.id \u003d\u003d instance[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_9071df78","line":76,"range":{"start_line":69,"start_character":8,"end_line":76,"end_character":9},"in_reply_to":"7a3c09a3_2d5534f9","updated":"2017-01-17 18:12:46.000000000","message":"thanks will fix.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":66,"context_line":""},{"line_number":67,"context_line":"    for instance in connection.execute(instances_query):"},{"line_number":68,"context_line":"        access_rule_status \u003d instance[\u0027access_rules_status\u0027]"},{"line_number":69,"context_line":"        op.execute("},{"line_number":70,"context_line":"            instance_access_map_table.update().where("},{"line_number":71,"context_line":"                instance_access_map_table.c.share_instance_id \u003d\u003d instance[\u0027id\u0027]"},{"line_number":72,"context_line":"            ).values({"},{"line_number":73,"context_line":"                \u0027state\u0027: access_rules_status_to_state_mapping["},{"line_number":74,"context_line":"                    access_rule_status],"},{"line_number":75,"context_line":"            })"},{"line_number":76,"context_line":"        )"},{"line_number":77,"context_line":"        op.execute("},{"line_number":78,"context_line":"            share_instances_table.update().where("},{"line_number":79,"context_line":"                share_instances_table.c.id \u003d\u003d instance[\u0027id\u0027]"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_10bdefb6","line":76,"range":{"start_line":69,"start_character":8,"end_line":76,"end_character":9},"in_reply_to":"7a3c09a3_9071df78","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"}],"manila/db/sqlalchemy/api.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":1919,"context_line":"        return access"},{"line_number":1920,"context_line":""},{"line_number":1921,"context_line":""},{"line_number":1922,"context_line":"def share_instance_access_update_all_for_instance(context, instance_id,"},{"line_number":1923,"context_line":"                                                  updates):"},{"line_number":1924,"context_line":"    session \u003d get_session()"},{"line_number":1925,"context_line":"    with session.begin():"},{"line_number":1926,"context_line":"        access_mappings \u003d _share_instance_access_query("}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_93a3619e","line":1923,"range":{"start_line":1922,"start_character":0,"end_line":1923,"end_character":59},"updated":"2016-10-21 11:29:05.000000000","message":"shouldn\u0027t this require the decorators @require_context and @wrap_db_retry?","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"        if create_share_instance:"},{"line_number":1476,"context_line":"            _share_instance_create(context, share_ref[\u0027id\u0027],"},{"line_number":1477,"context_line":"                                   share_instance_values, session\u003dsession)"},{"line_number":1478,"context_line":"        # NOTE(u_glide): Do so to prevent errors with relationships"},{"line_number":1479,"context_line":"        return share_get(context, share_ref[\u0027id\u0027], session\u003dsession)"},{"line_number":1480,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_7e58f68a","line":1477,"updated":"2017-01-17 01:53:28.000000000","message":"gratuitous whitespace removal?","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"        if create_share_instance:"},{"line_number":1476,"context_line":"            _share_instance_create(context, share_ref[\u0027id\u0027],"},{"line_number":1477,"context_line":"                                   share_instance_values, session\u003dsession)"},{"line_number":1478,"context_line":"        # NOTE(u_glide): Do so to prevent errors with relationships"},{"line_number":1479,"context_line":"        return share_get(context, share_ref[\u0027id\u0027], session\u003dsession)"},{"line_number":1480,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_17241a5e","line":1477,"in_reply_to":"7a3c09a3_7e58f68a","updated":"2017-01-17 13:47:35.000000000","message":"Done","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":1812,"context_line":"@require_context"},{"line_number":1813,"context_line":"def share_access_get_all_for_instance(context, instance_id, filters\u003dNone,"},{"line_number":1814,"context_line":"                                      with_share_access_data\u003dTrue,"},{"line_number":1815,"context_line":"                                      session\u003dNone):"},{"line_number":1816,"context_line":"    \"\"\"Get all access rules related to a certain share instance.\"\"\""},{"line_number":1817,"context_line":"    session \u003d session or get_session()"},{"line_number":1818,"context_line":"    filters \u003d copy.deepcopy(filters) if filters else {}"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_3e658e1d","line":1815,"range":{"start_line":1815,"start_character":38,"end_line":1815,"end_character":52},"updated":"2017-01-17 01:53:28.000000000","message":"you removed session in the other api.py","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":1812,"context_line":"@require_context"},{"line_number":1813,"context_line":"def share_access_get_all_for_instance(context, instance_id, filters\u003dNone,"},{"line_number":1814,"context_line":"                                      with_share_access_data\u003dTrue,"},{"line_number":1815,"context_line":"                                      session\u003dNone):"},{"line_number":1816,"context_line":"    \"\"\"Get all access rules related to a certain share instance.\"\"\""},{"line_number":1817,"context_line":"    session \u003d session or get_session()"},{"line_number":1818,"context_line":"    filters \u003d copy.deepcopy(filters) if filters else {}"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_f7c06e52","line":1815,"range":{"start_line":1815,"start_character":38,"end_line":1815,"end_character":52},"in_reply_to":"7a3c09a3_3e658e1d","updated":"2017-01-17 13:47:35.000000000","message":"Yes, but it seems like a pattern for this file... \n\nwe\u0027d use this if we wanted this to be part of the same transaction initiated through a method in this file.","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"}],"manila/db/sqlalchemy/models.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"87cb08f9517fcc0543118957a8f81d6c28e21e9b","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        An access rule is supposed to be truly \u0027active\u0027 when it has been"},{"line_number":558,"context_line":"        applies across all of the share instances of the parent share object."},{"line_number":559,"context_line":"        \"\"\""},{"line_number":560,"context_line":"        order \u003d (constants.STATUS_ERROR, constants.STATUS_APPLYING,"},{"line_number":561,"context_line":"                 constants.STATUS_NEW, constants.STATUS_ACTIVE)"},{"line_number":562,"context_line":"        other_statuses \u003d [x[\u0027state\u0027] for x in self.instance_mappings if"},{"line_number":563,"context_line":"                          x[\u0027state\u0027] not in order]"},{"line_number":564,"context_line":"        order \u003d (order + tuple(other_statuses))"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"        sorted_instance_mappings \u003d sorted("},{"line_number":567,"context_line":"            self.instance_mappings, key\u003dlambda x: order.index(x[\u0027state\u0027]))"},{"line_number":568,"context_line":"        return sorted_instance_mappings[0].state"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":""},{"line_number":571,"context_line":"class ShareInstanceAccessMapping(BASE, ManilaBase):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_cf31bfee","line":568,"range":{"start_line":560,"start_character":8,"end_line":568,"end_character":48},"updated":"2016-09-14 00:45:41.000000000","message":"dummy driver is reporting a crash here, sorted_instance_mappings is empty. I guess this is when you have an access rule not mapped to any instance? Would the state be \u0027new\u0027 in this case?","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"5324d8f24177d8db43bed967d36c18a5164423a2","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        An access rule is supposed to be truly \u0027active\u0027 when it has been"},{"line_number":558,"context_line":"        applies across all of the share instances of the parent share object."},{"line_number":559,"context_line":"        \"\"\""},{"line_number":560,"context_line":"        order \u003d (constants.STATUS_ERROR, constants.STATUS_APPLYING,"},{"line_number":561,"context_line":"                 constants.STATUS_NEW, constants.STATUS_ACTIVE)"},{"line_number":562,"context_line":"        other_statuses \u003d [x[\u0027state\u0027] for x in self.instance_mappings if"},{"line_number":563,"context_line":"                          x[\u0027state\u0027] not in order]"},{"line_number":564,"context_line":"        order \u003d (order + tuple(other_statuses))"},{"line_number":565,"context_line":""},{"line_number":566,"context_line":"        sorted_instance_mappings \u003d sorted("},{"line_number":567,"context_line":"            self.instance_mappings, key\u003dlambda x: order.index(x[\u0027state\u0027]))"},{"line_number":568,"context_line":"        return sorted_instance_mappings[0].state"},{"line_number":569,"context_line":""},{"line_number":570,"context_line":""},{"line_number":571,"context_line":"class ShareInstanceAccessMapping(BASE, ManilaBase):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_6fd18bfb","line":568,"range":{"start_line":560,"start_character":8,"end_line":568,"end_character":48},"in_reply_to":"7a8ec9b2_cf31bfee","updated":"2016-09-14 01:14:26.000000000","message":"Yes, noticed this.. for the brief moment between deletion of all instance mappings and finally the share access mapping, the access-list API reads the access rules. guess only the dummy driver can trigger such a failure.. nevertheless, will add a filter for len(instance_mappings) \u003e 0 in the share_access_get_all db query","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"48452cbddd7970a765d038a5fde497d5f6d4ec5a","unresolved":false,"context_lines":[{"line_number":555,"context_line":"                     constants.ACCESS_STATE_QUEUED_TO_APPLY,"},{"line_number":556,"context_line":"                     constants.ACCESS_STATE_APPLYING,"},{"line_number":557,"context_line":"                     constants.ACCESS_STATE_ACTIVE)"},{"line_number":558,"context_line":"            other_statuses \u003d [x[\u0027state\u0027] for x in self.instance_mappings if"},{"line_number":559,"context_line":"                              x[\u0027state\u0027] not in order]"},{"line_number":560,"context_line":"            order \u003d (order + tuple(other_statuses))"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"            sorted_instance_mappings \u003d sorted("},{"line_number":563,"context_line":"                self.instance_mappings, key\u003dlambda x: order.index(x[\u0027state\u0027]))"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_af5f953f","line":560,"range":{"start_line":558,"start_character":12,"end_line":560,"end_character":51},"updated":"2017-01-11 19:19:03.000000000","message":"what other states exist?","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2307d66a51cdf13e911af61fe2482bccada0201e","unresolved":false,"context_lines":[{"line_number":555,"context_line":"                     constants.ACCESS_STATE_QUEUED_TO_APPLY,"},{"line_number":556,"context_line":"                     constants.ACCESS_STATE_APPLYING,"},{"line_number":557,"context_line":"                     constants.ACCESS_STATE_ACTIVE)"},{"line_number":558,"context_line":"            other_statuses \u003d [x[\u0027state\u0027] for x in self.instance_mappings if"},{"line_number":559,"context_line":"                              x[\u0027state\u0027] not in order]"},{"line_number":560,"context_line":"            order \u003d (order + tuple(other_statuses))"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"            sorted_instance_mappings \u003d sorted("},{"line_number":563,"context_line":"                self.instance_mappings, key\u003dlambda x: order.index(x[\u0027state\u0027]))"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_9b3205a6","line":560,"range":{"start_line":558,"start_character":12,"end_line":560,"end_character":51},"in_reply_to":"ba5201f7_585d0314","updated":"2017-01-11 19:54:55.000000000","message":"if new states come in, they should take the order into account, specially if they are more important than any other state, else they will be considered less important, being put last in the order, so maybe we should not consider them, even it if allows new states to come in more easily.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0c81951e4e23a56d3ae0d1da8d8aeadc20845115","unresolved":false,"context_lines":[{"line_number":555,"context_line":"                     constants.ACCESS_STATE_QUEUED_TO_APPLY,"},{"line_number":556,"context_line":"                     constants.ACCESS_STATE_APPLYING,"},{"line_number":557,"context_line":"                     constants.ACCESS_STATE_ACTIVE)"},{"line_number":558,"context_line":"            other_statuses \u003d [x[\u0027state\u0027] for x in self.instance_mappings if"},{"line_number":559,"context_line":"                              x[\u0027state\u0027] not in order]"},{"line_number":560,"context_line":"            order \u003d (order + tuple(other_statuses))"},{"line_number":561,"context_line":""},{"line_number":562,"context_line":"            sorted_instance_mappings \u003d sorted("},{"line_number":563,"context_line":"                self.instance_mappings, key\u003dlambda x: order.index(x[\u0027state\u0027]))"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_585d0314","line":560,"range":{"start_line":558,"start_character":12,"end_line":560,"end_character":51},"in_reply_to":"ba5201f7_af5f953f","updated":"2017-01-11 19:47:25.000000000","message":"none besides the above for now..","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"}],"manila/share/access.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"834295b0ca82356b35f0ef7cf910cae7e2db325a","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":50,"context_line":"        share_id \u003d share_instance[\"share_id\"]"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        # TODO(merooney): explore getting rid of this"},{"line_number":53,"context_line":"        @utils.synchronized("},{"line_number":54,"context_line":"            \"update_access_rules_for_share_%s\" % share_id, external\u003dTrue)"},{"line_number":55,"context_line":"        def _update_access_rules_locked(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_0aab988a","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":53},"updated":"2016-09-13 23:50:21.000000000","message":"indeed, the lock should not be necessary anymore, but since you are receiving every update_access request, this method would be running concurrently. In my opinion, it shouldn\u0027t.","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e6d143838d6f8e496972a7dce1f97ef883375ff7","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":50,"context_line":"        share_id \u003d share_instance[\"share_id\"]"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"        # TODO(merooney): explore getting rid of this"},{"line_number":53,"context_line":"        @utils.synchronized("},{"line_number":54,"context_line":"            \"update_access_rules_for_share_%s\" % share_id, external\u003dTrue)"},{"line_number":55,"context_line":"        def _update_access_rules_locked(*args, **kwargs):"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a8bc5a2_6fa91090","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":53},"in_reply_to":"7a8ec9b2_0aab988a","updated":"2016-10-03 15:00:10.000000000","message":"refactored.","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":43,"context_line":"        :param share_instance_id: ID of the share instance"},{"line_number":44,"context_line":"        :param delete_all_rules: set this parameter to True if all"},{"line_number":45,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":46,"context_line":"        :param resync_rules: set this parameter to True if calling method would"},{"line_number":47,"context_line":"            like access rules to be re-synced on the share instance. Re-syncing"},{"line_number":48,"context_line":"            is not be a synchronous operation if the share instance\u0027s access"},{"line_number":49,"context_line":"            rules are being modified when the request arrives. Calling"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_64e28105","line":46,"range":{"start_line":46,"start_character":15,"end_line":46,"end_character":27},"updated":"2016-10-21 11:29:05.000000000","message":"I haven\u0027t found where this is used so far","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":45,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":46,"context_line":"        :param resync_rules: set this parameter to True if calling method would"},{"line_number":47,"context_line":"            like access rules to be re-synced on the share instance. Re-syncing"},{"line_number":48,"context_line":"            is not be a synchronous operation if the share instance\u0027s access"},{"line_number":49,"context_line":"            rules are being modified when the request arrives. Calling"},{"line_number":50,"context_line":"            methods will have to wait for the share instance\u0027s"},{"line_number":51,"context_line":"            access_rules_status to be \u0027active\u0027 in order to be sure of the"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_06046d7c","line":48,"range":{"start_line":48,"start_character":19,"end_line":48,"end_character":21},"updated":"2016-10-21 11:29:05.000000000","message":"remove \"be\"","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        delete_rules \u003d []"},{"line_number":111,"context_line":"        rules_to_be_removed_from_db \u003d []"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"        access_filters \u003d {"},{"line_number":114,"context_line":"            \u0027state\u0027: (constants.STATUS_NEW, constants.STATUS_ACTIVE,"},{"line_number":115,"context_line":"                      constants.STATUS_DENYING),"},{"line_number":116,"context_line":"        }"},{"line_number":117,"context_line":"        existing_rules_in_db \u003d self.db.share_access_get_all_for_instance("},{"line_number":118,"context_line":"            context, share_instance[\u0027id\u0027], filters\u003daccess_filters)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_a979d231","line":115,"range":{"start_line":113,"start_character":8,"end_line":115,"end_character":48},"updated":"2016-10-21 11:29:05.000000000","message":"this possibly deserves a comment as to why rules with other states are being disregarded.","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"f3e3830c4e571c9a384a6b458b8af35ca6670709","unresolved":false,"context_lines":[{"line_number":138,"context_line":"        # NOTE(ganso): trigger maintenance mode"},{"line_number":139,"context_line":"        if instance_access_status \u003d\u003d constants.STATUS_ERROR:"},{"line_number":140,"context_line":"            rules_to_be_removed_from_db \u003d delete_rules"},{"line_number":141,"context_line":"            delete_rules \u003d []"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        try:"},{"line_number":144,"context_line":"            access_keys \u003d None"}],"source_content_type":"text/x-python","patch_set":3,"id":"fa48f94a_12dbecee","line":141,"range":{"start_line":141,"start_character":11,"end_line":141,"end_character":29},"updated":"2016-12-21 17:27:09.000000000","message":"in this case add_rules need to be [] as well","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":154,"context_line":"            except NotImplementedError:"},{"line_number":155,"context_line":"                # NOTE(u_glide): Fallback to legacy allow_access/deny_access"},{"line_number":156,"context_line":"                # for drivers without update_access() method support"},{"line_number":157,"context_line":"                self._update_access_fallback(context, add_rules,  delete_rules,"},{"line_number":158,"context_line":"                                             rules_to_be_removed_from_db,"},{"line_number":159,"context_line":"                                             share_instance,"},{"line_number":160,"context_line":"                                             share_server)"},{"line_number":161,"context_line":""},{"line_number":162,"context_line":"            if access_keys:"},{"line_number":163,"context_line":"                self._validate_access_keys(access_rules_to_be_on_share,"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_69497a8b","line":160,"range":{"start_line":157,"start_character":16,"end_line":160,"end_character":58},"updated":"2016-10-21 11:29:05.000000000","message":"it would be good to have this removed in Ocata, along with this refactor. I have a patch up doing it https://review.openstack.org/#/c/365779/","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":170,"context_line":"                        context, access_id, access_key)"},{"line_number":171,"context_line":""},{"line_number":172,"context_line":"            for rule in add_rules:"},{"line_number":173,"context_line":"                # If the \u0027applying\u0027 state wasn\u0027t changed in the database,"},{"line_number":174,"context_line":"                # we can set these rules to \u0027active\u0027."},{"line_number":175,"context_line":"                instance_rule_mapping \u003d self.db.share_instance_access_get("},{"line_number":176,"context_line":"                    context, rule[\u0027id\u0027], share_instance_id)"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_640e0136","line":173,"range":{"start_line":173,"start_character":18,"end_line":173,"end_character":73},"updated":"2016-10-21 11:29:05.000000000","message":"I am curious as to what situation a rule that had already transitioned to \u0027applying\u0027 could be changed to a different state, assuming no exception have been raised so far","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"a3e4e22a88b462e8f79c4714b1826451aafbbf39","unresolved":false,"context_lines":[{"line_number":274,"context_line":"            r for r in existing_rules_in_db if r[\u0027id\u0027] not in delete_rule_ids"},{"line_number":275,"context_line":"        ]"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        # NOTE(ganso): trigger maintenance mode"},{"line_number":278,"context_line":"        if instance_access_status \u003d\u003d constants.STATUS_ERROR:"},{"line_number":279,"context_line":"            rules_to_be_removed_from_db \u003d delete_rules"},{"line_number":280,"context_line":"            delete_rules \u003d []"},{"line_number":281,"context_line":""},{"line_number":282,"context_line":"        if share_instance[\u0027status\u0027] \u003d\u003d constants.STATUS_MIGRATING:"},{"line_number":283,"context_line":"            # NOTE(ganso): If the share instance is the source in a migration,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_47ec3ef3","line":280,"range":{"start_line":277,"start_character":7,"end_line":280,"end_character":29},"updated":"2017-01-11 21:29:53.000000000","message":"I believe this situation is no longer valid according to the latest spec","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"a3e4e22a88b462e8f79c4714b1826451aafbbf39","unresolved":false,"context_lines":[{"line_number":304,"context_line":"                          share_instance[\u0027id\u0027])"},{"line_number":305,"context_line":"                access_rules_to_be_on_share \u003d add_rules"},{"line_number":306,"context_line":"            add_rules \u003d []"},{"line_number":307,"context_line":"            delete_rules \u003d []"},{"line_number":308,"context_line":""},{"line_number":309,"context_line":"        try:"},{"line_number":310,"context_line":"            driver_rule_updates \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_a7b16a03","line":307,"range":{"start_line":307,"start_character":11,"end_line":307,"end_character":29},"updated":"2017-01-11 21:29:53.000000000","message":"this is causing the problem of stuck rules in migration. You need to set rules_to_be_removed_from_db \u003d delete_rules before this","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"4addf8a6f93225f7c41d84226cd844bd594a9596","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        conditionally_change[access_rules_status],"},{"line_number":84,"context_line":"                }"},{"line_number":85,"context_line":"            except KeyError:"},{"line_number":86,"context_line":"                pass"},{"line_number":87,"context_line":"        if updates:"},{"line_number":88,"context_line":"            share_instance \u003d self.db.share_instance_update("},{"line_number":89,"context_line":"                context, share_instance_id, updates)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a57fde8_2e02423f","line":86,"range":{"start_line":86,"start_character":16,"end_line":86,"end_character":20},"updated":"2017-01-12 11:42:22.000000000","message":"You do not define \"updates\" in this case and get NameError","commit_id":"5e0303e8dfce9f5c3288e70710ee0af6f7705c15"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"9e4e9221f23280b03c1b3d9bfeda272eaafb35f1","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        conditionally_change[access_rules_status],"},{"line_number":84,"context_line":"                }"},{"line_number":85,"context_line":"            except KeyError:"},{"line_number":86,"context_line":"                pass"},{"line_number":87,"context_line":"        if updates:"},{"line_number":88,"context_line":"            share_instance \u003d self.db.share_instance_update("},{"line_number":89,"context_line":"                context, share_instance_id, updates)"}],"source_content_type":"text/x-python","patch_set":11,"id":"9a57fde8_e7493890","line":86,"range":{"start_line":86,"start_character":16,"end_line":86,"end_character":20},"in_reply_to":"9a57fde8_2e02423f","updated":"2017-01-12 14:23:50.000000000","message":"yep, thank you. Fixed that locally. Working on unit testing this file thoroughly to get rid of other gaffes.","commit_id":"5e0303e8dfce9f5c3288e70710ee0af6f7705c15"},{"author":{"_account_id":11047,"name":"Mark Sturdevant","email":"mark.sturdevant@ibm.com","username":"markstur"},"change_message_id":"c49f1c867e367b830f0e866d309e5d8e3e3e9223","unresolved":false,"context_lines":[{"line_number":241,"context_line":"            self.get_and_update_share_instance_access_rules("},{"line_number":242,"context_line":"                context, updates\u003dupdates, share_instance_id\u003dshare_instance_id)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        # Is there a sync in progress? If yes, ignore the sync request."},{"line_number":245,"context_line":"        rule_filter \u003d {"},{"line_number":246,"context_line":"            \u0027state\u0027: (constants.ACCESS_STATE_APPLYING,"},{"line_number":247,"context_line":"                      constants.ACCESS_STATE_DENYING),"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_347dadb1","line":244,"range":{"start_line":244,"start_character":39,"end_line":244,"end_character":71},"updated":"2017-01-17 01:53:28.000000000","message":"ignore which sync? This wording isn\u0027t great. At first I read that you would ignore the one in progress.","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"643807591f67614e8c2b4f26fcbf9b4b64e8117d","unresolved":false,"context_lines":[{"line_number":241,"context_line":"            self.get_and_update_share_instance_access_rules("},{"line_number":242,"context_line":"                context, updates\u003dupdates, share_instance_id\u003dshare_instance_id)"},{"line_number":243,"context_line":""},{"line_number":244,"context_line":"        # Is there a sync in progress? If yes, ignore the sync request."},{"line_number":245,"context_line":"        rule_filter \u003d {"},{"line_number":246,"context_line":"            \u0027state\u0027: (constants.ACCESS_STATE_APPLYING,"},{"line_number":247,"context_line":"                      constants.ACCESS_STATE_DENYING),"}],"source_content_type":"text/x-python","patch_set":16,"id":"7a3c09a3_d789b227","line":244,"range":{"start_line":244,"start_character":39,"end_line":244,"end_character":71},"in_reply_to":"7a3c09a3_347dadb1","updated":"2017-01-17 13:47:35.000000000","message":"my bad. clarified","commit_id":"612dd96e6d799736f72c61242eae21b04d2d11d2"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"2efc9e92869a1163c2aa2d7516d7531d316a5f89","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @locked_access_rules_operation"},{"line_number":57,"context_line":"    def get_and_update_share_instance_access_rules_status("},{"line_number":58,"context_line":"            self, context, status\u003dNone, conditionally_change\u003d{},"},{"line_number":59,"context_line":"            share_instance_id\u003dNone):"},{"line_number":60,"context_line":"        \"\"\"Get and update the access_rules_status of a share instance."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        :param status: Set this parameter only if you want to"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_a5f85ec4","line":59,"range":{"start_line":58,"start_character":40,"end_line":59,"end_character":34},"updated":"2017-01-17 17:28:32.000000000","message":"this","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @locked_access_rules_operation"},{"line_number":57,"context_line":"    def get_and_update_share_instance_access_rules_status("},{"line_number":58,"context_line":"            self, context, status\u003dNone, conditionally_change\u003d{},"},{"line_number":59,"context_line":"            share_instance_id\u003dNone):"},{"line_number":60,"context_line":"        \"\"\"Get and update the access_rules_status of a share instance."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        :param status: Set this parameter only if you want to"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_50013769","line":59,"range":{"start_line":58,"start_character":40,"end_line":59,"end_character":34},"in_reply_to":"7a3c09a3_1ac350ee","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    @locked_access_rules_operation"},{"line_number":57,"context_line":"    def get_and_update_share_instance_access_rules_status("},{"line_number":58,"context_line":"            self, context, status\u003dNone, conditionally_change\u003d{},"},{"line_number":59,"context_line":"            share_instance_id\u003dNone):"},{"line_number":60,"context_line":"        \"\"\"Get and update the access_rules_status of a share instance."},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        :param status: Set this parameter only if you want to"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_1ac350ee","line":59,"range":{"start_line":58,"start_character":40,"end_line":59,"end_character":34},"in_reply_to":"7a3c09a3_a5f85ec4","updated":"2017-01-17 18:01:57.000000000","message":"will fix","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"de5db1cbd74f4a476a7c3036e50f07148aae6479","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            access_rules_status to. If the state is not as expected,"},{"line_number":70,"context_line":"            no transition is performed. Default is {}, which means no state"},{"line_number":71,"context_line":"            transitions will be made."},{"line_number":72,"context_line":"        :returns share_instance: if an update was made."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        if status is not None:"},{"line_number":75,"context_line":"            updates \u003d {\u0027access_rules_status\u0027: status}"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_fa3844a2","line":72,"range":{"start_line":72,"start_character":33,"end_line":72,"end_character":54},"updated":"2017-01-17 17:44:04.000000000","message":"?","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"de5db1cbd74f4a476a7c3036e50f07148aae6479","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            access_rules_status to. If the state is not as expected,"},{"line_number":70,"context_line":"            no transition is performed. Default is {}, which means no state"},{"line_number":71,"context_line":"            transitions will be made."},{"line_number":72,"context_line":"        :returns share_instance: if an update was made."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        if status is not None:"},{"line_number":75,"context_line":"            updates \u003d {\u0027access_rules_status\u0027: status}"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_7ac6149b","line":72,"range":{"start_line":72,"start_character":17,"end_line":72,"end_character":31},"updated":"2017-01-17 17:44:04.000000000","message":"share_instance_id","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"26a7832ce869f21622293a172e7b3aeb616a3dc8","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            access_rules_status to. If the state is not as expected,"},{"line_number":70,"context_line":"            no transition is performed. Default is {}, which means no state"},{"line_number":71,"context_line":"            transitions will be made."},{"line_number":72,"context_line":"        :returns share_instance: if an update was made."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        if status is not None:"},{"line_number":75,"context_line":"            updates \u003d {\u0027access_rules_status\u0027: status}"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_da6708b5","line":72,"range":{"start_line":72,"start_character":17,"end_line":72,"end_character":31},"in_reply_to":"7a3c09a3_7ac6149b","updated":"2017-01-17 17:45:07.000000000","message":"nevermind, this is share_instance, it is missing docstring for share_instance_id.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":69,"context_line":"            access_rules_status to. If the state is not as expected,"},{"line_number":70,"context_line":"            no transition is performed. Default is {}, which means no state"},{"line_number":71,"context_line":"            transitions will be made."},{"line_number":72,"context_line":"        :returns share_instance: if an update was made."},{"line_number":73,"context_line":"        \"\"\""},{"line_number":74,"context_line":"        if status is not None:"},{"line_number":75,"context_line":"            updates \u003d {\u0027access_rules_status\u0027: status}"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_1a6a70ee","line":72,"range":{"start_line":72,"start_character":17,"end_line":72,"end_character":31},"in_reply_to":"7a3c09a3_da6708b5","updated":"2017-01-17 18:01:57.000000000","message":"yes, it doesn\u0027t have all parameters.. some of these are self explanatory.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"de5db1cbd74f4a476a7c3036e50f07148aae6479","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            updates \u003d {\u0027access_rules_status\u0027: status}"},{"line_number":76,"context_line":"        else:"},{"line_number":77,"context_line":"            share_instance \u003d self.db.share_instance_get("},{"line_number":78,"context_line":"                context, share_instance_id)"},{"line_number":79,"context_line":"            access_rules_status \u003d share_instance[\u0027access_rules_status\u0027]"},{"line_number":80,"context_line":"            try:"},{"line_number":81,"context_line":"                updates \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_fad12458","line":78,"range":{"start_line":78,"start_character":25,"end_line":78,"end_character":42},"updated":"2017-01-17 17:44:04.000000000","message":"can be None","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":75,"context_line":"            updates \u003d {\u0027access_rules_status\u0027: status}"},{"line_number":76,"context_line":"        else:"},{"line_number":77,"context_line":"            share_instance \u003d self.db.share_instance_get("},{"line_number":78,"context_line":"                context, share_instance_id)"},{"line_number":79,"context_line":"            access_rules_status \u003d share_instance[\u0027access_rules_status\u0027]"},{"line_number":80,"context_line":"            try:"},{"line_number":81,"context_line":"                updates \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_7a27f4c9","line":78,"range":{"start_line":78,"start_character":25,"end_line":78,"end_character":42},"in_reply_to":"7a3c09a3_fad12458","updated":"2017-01-17 18:01:57.000000000","message":"cannot be None. \n\nThe reason it\u0027s a kwarg in the parameters in this method is to allow it to be parsed easily by the decorator. Passing this as None is hopefully a mistake we\u0027ll never make, because the lock will be named:\n\n locked_access_rules_operation_by_share_instance_None\n\n\nSee similar lock in share/manager","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"2efc9e92869a1163c2aa2d7516d7531d316a5f89","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    @locked_access_rules_operation"},{"line_number":93,"context_line":"    def get_and_update_share_instance_access_rules(self, context,"},{"line_number":94,"context_line":"                                                   filters\u003d{}, updates\u003d{},"},{"line_number":95,"context_line":"                                                   share_instance_id\u003dNone,"},{"line_number":96,"context_line":"                                                   conditionally_change\u003d{}):"},{"line_number":97,"context_line":"        \"\"\"Get and conditionally update all access rules of a share instance."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        :param updates: Set this parameter to a dictionary of key:value"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_85228250","line":96,"range":{"start_line":95,"start_character":51,"end_line":96,"end_character":74},"updated":"2017-01-17 17:28:32.000000000","message":"and this is inconsistent with each other, they have different order.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    @locked_access_rules_operation"},{"line_number":93,"context_line":"    def get_and_update_share_instance_access_rules(self, context,"},{"line_number":94,"context_line":"                                                   filters\u003d{}, updates\u003d{},"},{"line_number":95,"context_line":"                                                   share_instance_id\u003dNone,"},{"line_number":96,"context_line":"                                                   conditionally_change\u003d{}):"},{"line_number":97,"context_line":"        \"\"\"Get and conditionally update all access rules of a share instance."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        :param updates: Set this parameter to a dictionary of key:value"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_fa192489","line":96,"range":{"start_line":95,"start_character":51,"end_line":96,"end_character":74},"in_reply_to":"7a3c09a3_85228250","updated":"2017-01-17 18:01:57.000000000","message":"will fix","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":92,"context_line":"    @locked_access_rules_operation"},{"line_number":93,"context_line":"    def get_and_update_share_instance_access_rules(self, context,"},{"line_number":94,"context_line":"                                                   filters\u003d{}, updates\u003d{},"},{"line_number":95,"context_line":"                                                   share_instance_id\u003dNone,"},{"line_number":96,"context_line":"                                                   conditionally_change\u003d{}):"},{"line_number":97,"context_line":"        \"\"\"Get and conditionally update all access rules of a share instance."},{"line_number":98,"context_line":""},{"line_number":99,"context_line":"        :param updates: Set this parameter to a dictionary of key:value"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_b047bbba","line":96,"range":{"start_line":95,"start_character":51,"end_line":96,"end_character":74},"in_reply_to":"7a3c09a3_fa192489","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        \"\"\""},{"line_number":129,"context_line":"        instance_rules \u003d self.db.share_access_get_all_for_instance("},{"line_number":130,"context_line":"            context, share_instance_id, filters\u003dfilters)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        if instance_rules and (updates or conditionally_change):"},{"line_number":133,"context_line":"            for rule in instance_rules:"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_3724a55e","line":130,"range":{"start_line":130,"start_character":21,"end_line":130,"end_character":38},"updated":"2017-01-17 17:41:48.000000000","message":"it can be None according to the method definition, then I suppose this will fail.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":127,"context_line":""},{"line_number":128,"context_line":"        \"\"\""},{"line_number":129,"context_line":"        instance_rules \u003d self.db.share_access_get_all_for_instance("},{"line_number":130,"context_line":"            context, share_instance_id, filters\u003dfilters)"},{"line_number":131,"context_line":""},{"line_number":132,"context_line":"        if instance_rules and (updates or conditionally_change):"},{"line_number":133,"context_line":"            for rule in instance_rules:"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_3a100c59","line":130,"range":{"start_line":130,"start_character":21,"end_line":130,"end_character":38},"in_reply_to":"7a3c09a3_3724a55e","updated":"2017-01-17 18:01:57.000000000","message":"see above","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        return instance_rules"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    @locked_access_rules_operation"},{"line_number":155,"context_line":"    def get_and_update_share_instance_access_rule(self, context, rule_id,"},{"line_number":156,"context_line":"                                                  updates\u003d{},"},{"line_number":157,"context_line":"                                                  share_instance_id\u003dNone,"},{"line_number":158,"context_line":"                                                  conditionally_change\u003d{}):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_37cf452a","line":155,"range":{"start_line":155,"start_character":8,"end_line":155,"end_character":49},"updated":"2017-01-17 17:41:48.000000000","message":"have you considered merging this with the above method? you could have the list of rule_ids as optional parameter and get the rules from the database only if rule_ids is None","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        return instance_rules"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    @locked_access_rules_operation"},{"line_number":155,"context_line":"    def get_and_update_share_instance_access_rule(self, context, rule_id,"},{"line_number":156,"context_line":"                                                  updates\u003d{},"},{"line_number":157,"context_line":"                                                  share_instance_id\u003dNone,"},{"line_number":158,"context_line":"                                                  conditionally_change\u003d{}):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_ed008c03","line":155,"range":{"start_line":155,"start_character":8,"end_line":155,"end_character":49},"in_reply_to":"7a3c09a3_37cf452a","updated":"2017-01-17 18:01:57.000000000","message":"I\u0027ve considered it and refrained from doing so.. we need a method that can still commit per rule changes...","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":152,"context_line":"        return instance_rules"},{"line_number":153,"context_line":""},{"line_number":154,"context_line":"    @locked_access_rules_operation"},{"line_number":155,"context_line":"    def get_and_update_share_instance_access_rule(self, context, rule_id,"},{"line_number":156,"context_line":"                                                  updates\u003d{},"},{"line_number":157,"context_line":"                                                  share_instance_id\u003dNone,"},{"line_number":158,"context_line":"                                                  conditionally_change\u003d{}):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_906c7f36","line":155,"range":{"start_line":155,"start_character":8,"end_line":155,"end_character":49},"in_reply_to":"7a3c09a3_ed008c03","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                updated_state \u003d conditionally_change[mapping_state]"},{"line_number":187,"context_line":"                updates.update({\u0027state\u0027: updated_state})"},{"line_number":188,"context_line":"            except KeyError:"},{"line_number":189,"context_line":"                msg \u003d (\"The state of the Access rule %(rule_id)s (permitting \""},{"line_number":190,"context_line":"                       \"access to share instance %(si)s) was not updated \""},{"line_number":191,"context_line":"                       \"because it\u0027s state was modified by another operation.\")"},{"line_number":192,"context_line":"                msg_payload \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_378105c4","line":189,"range":{"start_line":189,"start_character":66,"end_line":189,"end_character":76},"updated":"2017-01-17 17:41:48.000000000","message":"\"allowing\" is more appropriate","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                updated_state \u003d conditionally_change[mapping_state]"},{"line_number":187,"context_line":"                updates.update({\u0027state\u0027: updated_state})"},{"line_number":188,"context_line":"            except KeyError:"},{"line_number":189,"context_line":"                msg \u003d (\"The state of the Access rule %(rule_id)s (permitting \""},{"line_number":190,"context_line":"                       \"access to share instance %(si)s) was not updated \""},{"line_number":191,"context_line":"                       \"because it\u0027s state was modified by another operation.\")"},{"line_number":192,"context_line":"                msg_payload \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_d71b61c8","line":189,"range":{"start_line":189,"start_character":41,"end_line":189,"end_character":42},"updated":"2017-01-17 17:41:48.000000000","message":"lowercase","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                updated_state \u003d conditionally_change[mapping_state]"},{"line_number":187,"context_line":"                updates.update({\u0027state\u0027: updated_state})"},{"line_number":188,"context_line":"            except KeyError:"},{"line_number":189,"context_line":"                msg \u003d (\"The state of the Access rule %(rule_id)s (permitting \""},{"line_number":190,"context_line":"                       \"access to share instance %(si)s) was not updated \""},{"line_number":191,"context_line":"                       \"because it\u0027s state was modified by another operation.\")"},{"line_number":192,"context_line":"                msg_payload \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_7028f3e2","line":189,"range":{"start_line":189,"start_character":41,"end_line":189,"end_character":42},"in_reply_to":"7a3c09a3_2d07f418","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":186,"context_line":"                updated_state \u003d conditionally_change[mapping_state]"},{"line_number":187,"context_line":"                updates.update({\u0027state\u0027: updated_state})"},{"line_number":188,"context_line":"            except KeyError:"},{"line_number":189,"context_line":"                msg \u003d (\"The state of the Access rule %(rule_id)s (permitting \""},{"line_number":190,"context_line":"                       \"access to share instance %(si)s) was not updated \""},{"line_number":191,"context_line":"                       \"because it\u0027s state was modified by another operation.\")"},{"line_number":192,"context_line":"                msg_payload \u003d {"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_2d07f418","line":189,"range":{"start_line":189,"start_character":41,"end_line":189,"end_character":42},"in_reply_to":"7a3c09a3_d71b61c8","updated":"2017-01-17 18:01:57.000000000","message":"will change","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            except KeyError:"},{"line_number":189,"context_line":"                msg \u003d (\"The state of the Access rule %(rule_id)s (permitting \""},{"line_number":190,"context_line":"                       \"access to share instance %(si)s) was not updated \""},{"line_number":191,"context_line":"                       \"because it\u0027s state was modified by another operation.\")"},{"line_number":192,"context_line":"                msg_payload \u003d {"},{"line_number":193,"context_line":"                    \u0027si\u0027: share_instance_id,"},{"line_number":194,"context_line":"                    \u0027rule_id\u0027: rule_id,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_9725590a","line":191,"range":{"start_line":191,"start_character":32,"end_line":191,"end_character":35},"updated":"2017-01-17 17:41:48.000000000","message":"its","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            except KeyError:"},{"line_number":189,"context_line":"                msg \u003d (\"The state of the Access rule %(rule_id)s (permitting \""},{"line_number":190,"context_line":"                       \"access to share instance %(si)s) was not updated \""},{"line_number":191,"context_line":"                       \"because it\u0027s state was modified by another operation.\")"},{"line_number":192,"context_line":"                msg_payload \u003d {"},{"line_number":193,"context_line":"                    \u0027si\u0027: share_instance_id,"},{"line_number":194,"context_line":"                    \u0027rule_id\u0027: rule_id,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_d0728710","line":191,"range":{"start_line":191,"start_character":32,"end_line":191,"end_character":35},"in_reply_to":"7a3c09a3_4d0c403b","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":188,"context_line":"            except KeyError:"},{"line_number":189,"context_line":"                msg \u003d (\"The state of the Access rule %(rule_id)s (permitting \""},{"line_number":190,"context_line":"                       \"access to share instance %(si)s) was not updated \""},{"line_number":191,"context_line":"                       \"because it\u0027s state was modified by another operation.\")"},{"line_number":192,"context_line":"                msg_payload \u003d {"},{"line_number":193,"context_line":"                    \u0027si\u0027: share_instance_id,"},{"line_number":194,"context_line":"                    \u0027rule_id\u0027: rule_id,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_4d0c403b","line":191,"range":{"start_line":191,"start_character":32,"end_line":191,"end_character":35},"in_reply_to":"7a3c09a3_9725590a","updated":"2017-01-17 18:01:57.000000000","message":"thanks. will do","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"44f75184569b4ad7ce74079fa1e95ead3bb47566","unresolved":false,"context_lines":[{"line_number":355,"context_line":""},{"line_number":356,"context_line":"            for rule_id, rule_updates in driver_rule_updates.items():"},{"line_number":357,"context_line":"                if \u0027state\u0027 in rule_updates:"},{"line_number":358,"context_line":"                    # Allow updates *only* if the state\u0027s unchanged"},{"line_number":359,"context_line":"                    state \u003d rule_updates.pop(\u0027state\u0027)"},{"line_number":360,"context_line":"                    conditional_state_updates \u003d {"},{"line_number":361,"context_line":"                        constants.ACCESS_STATE_APPLYING: state,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_ab6aec7e","line":358,"range":{"start_line":358,"start_character":20,"end_line":358,"end_character":67},"updated":"2017-01-17 18:40:18.000000000","message":"I am having a hard time understand this comment. Please elaborate. The code is not very complicated to understand but this comment makes me think I am missing something important","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":355,"context_line":""},{"line_number":356,"context_line":"            for rule_id, rule_updates in driver_rule_updates.items():"},{"line_number":357,"context_line":"                if \u0027state\u0027 in rule_updates:"},{"line_number":358,"context_line":"                    # Allow updates *only* if the state\u0027s unchanged"},{"line_number":359,"context_line":"                    state \u003d rule_updates.pop(\u0027state\u0027)"},{"line_number":360,"context_line":"                    conditional_state_updates \u003d {"},{"line_number":361,"context_line":"                        constants.ACCESS_STATE_APPLYING: state,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_0665b5b3","line":358,"range":{"start_line":358,"start_character":20,"end_line":358,"end_character":67},"in_reply_to":"7a3c09a3_ab6aec7e","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"44f75184569b4ad7ce74079fa1e95ead3bb47566","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                    context, filters\u003drule_filter,"},{"line_number":419,"context_line":"                    share_instance_id\u003dshare_instance_id)"},{"line_number":420,"context_line":"            )"},{"line_number":421,"context_line":"            if not any(rules_in_error_state):"},{"line_number":422,"context_line":"                conditionally_change \u003d {"},{"line_number":423,"context_line":"                    constants.SHARE_INSTANCE_RULES_SYNCING:"},{"line_number":424,"context_line":"                        constants.STATUS_ACTIVE,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_4beb4878","line":421,"range":{"start_line":421,"start_character":19,"end_line":421,"end_character":44},"updated":"2017-01-17 18:40:18.000000000","message":"is \"any()\" necessary here? shouldn\u0027t it return None or empty list?","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":418,"context_line":"                    context, filters\u003drule_filter,"},{"line_number":419,"context_line":"                    share_instance_id\u003dshare_instance_id)"},{"line_number":420,"context_line":"            )"},{"line_number":421,"context_line":"            if not any(rules_in_error_state):"},{"line_number":422,"context_line":"                conditionally_change \u003d {"},{"line_number":423,"context_line":"                    constants.SHARE_INSTANCE_RULES_SYNCING:"},{"line_number":424,"context_line":"                        constants.STATUS_ACTIVE,"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_469c1d5d","line":421,"range":{"start_line":421,"start_character":19,"end_line":421,"end_character":44},"in_reply_to":"7a3c09a3_4beb4878","updated":"2017-01-17 23:52:48.000000000","message":"It does return a list, so any() is appropriate here to test list membership semantics... also, among:\n\n if not any(rules_in_error_state)\n\nvs\n\n if not rules_in_error_state\n\nvs the redundant:\n\n if not len(rules_in_error_state) \u003e 0\n\nthe one with any() is more readable and it is a builtin..","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"44f75184569b4ad7ce74079fa1e95ead3bb47566","unresolved":false,"context_lines":[{"line_number":484,"context_line":"                share_server\u003dshare_server"},{"line_number":485,"context_line":"            )"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    def _get_unsynced_access_rules_from_db(self, context, share_instance_id):"},{"line_number":488,"context_line":"        rule_filter \u003d {"},{"line_number":489,"context_line":"            \u0027state\u0027: (constants.ACCESS_STATE_QUEUED_TO_APPLY,"},{"line_number":490,"context_line":"                      constants.ACCESS_STATE_QUEUED_TO_DENY),"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_2b10bc58","line":487,"range":{"start_line":487,"start_character":8,"end_line":487,"end_character":42},"updated":"2017-01-17 18:40:18.000000000","message":"you are not only getting, you are setting them as well. I recommend a name-change. I was not able to figure out where the rules were being changed to APPLYING and DENYING until I read the details of this method.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":484,"context_line":"                share_server\u003dshare_server"},{"line_number":485,"context_line":"            )"},{"line_number":486,"context_line":""},{"line_number":487,"context_line":"    def _get_unsynced_access_rules_from_db(self, context, share_instance_id):"},{"line_number":488,"context_line":"        rule_filter \u003d {"},{"line_number":489,"context_line":"            \u0027state\u0027: (constants.ACCESS_STATE_QUEUED_TO_APPLY,"},{"line_number":490,"context_line":"                      constants.ACCESS_STATE_QUEUED_TO_DENY),"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_a6ab0174","line":487,"range":{"start_line":487,"start_character":8,"end_line":487,"end_character":42},"in_reply_to":"7a3c09a3_2b10bc58","updated":"2017-01-17 23:52:48.000000000","message":"sure thing.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"5bb45d96ec2311bdbff90c23ca11fb46b29b4c2f","unresolved":false,"context_lines":[{"line_number":357,"context_line":""},{"line_number":358,"context_line":"            for rule_id, rule_updates in driver_rule_updates.items():"},{"line_number":359,"context_line":"                if \u0027state\u0027 in rule_updates:"},{"line_number":360,"context_line":"                    # We allow updates *only* if the state is unchanged from"},{"line_number":361,"context_line":"                    # the time this update was initiated. It is possible"},{"line_number":362,"context_line":"                    # that the access rule was denied at the API prior to"},{"line_number":363,"context_line":"                    # the driver reporting that the access rule was added"},{"line_number":364,"context_line":"                    # successfully."},{"line_number":365,"context_line":"                    state \u003d rule_updates.pop(\u0027state\u0027)"},{"line_number":366,"context_line":"                    conditional_state_updates \u003d {"},{"line_number":367,"context_line":"                        constants.ACCESS_STATE_APPLYING: state,"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_9457e856","line":364,"range":{"start_line":360,"start_character":20,"end_line":364,"end_character":35},"updated":"2017-01-18 12:27:24.000000000","message":"thanks. In other words, the way you implement this \"only if\", is by only handling these states below and not QUEUED_TO_DENY and QUEUED_TO_ALLOW, correct? makes sense that we are not looking at those since the rules we expect to handle are supposed to be at APPLYING/DENYING.","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"3cc8595a54686105ea65b5fbf3fc66d0c7f344e4","unresolved":false,"context_lines":[{"line_number":357,"context_line":""},{"line_number":358,"context_line":"            for rule_id, rule_updates in driver_rule_updates.items():"},{"line_number":359,"context_line":"                if \u0027state\u0027 in rule_updates:"},{"line_number":360,"context_line":"                    # We allow updates *only* if the state is unchanged from"},{"line_number":361,"context_line":"                    # the time this update was initiated. It is possible"},{"line_number":362,"context_line":"                    # that the access rule was denied at the API prior to"},{"line_number":363,"context_line":"                    # the driver reporting that the access rule was added"},{"line_number":364,"context_line":"                    # successfully."},{"line_number":365,"context_line":"                    state \u003d rule_updates.pop(\u0027state\u0027)"},{"line_number":366,"context_line":"                    conditional_state_updates \u003d {"},{"line_number":367,"context_line":"                        constants.ACCESS_STATE_APPLYING: state,"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_66b2c74d","line":364,"range":{"start_line":360,"start_character":20,"end_line":364,"end_character":35},"in_reply_to":"7a3c09a3_9457e856","updated":"2017-01-18 14:27:43.000000000","message":"or active.","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"06db914e0d1a52fffee5c0206763435b5357a2a2","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"7a3c09a3_eb665b28","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"updated":"2017-01-19 01:24:44.000000000","message":"I think we can avoid this db get by passing in the share instance instead of the share_instance_id.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"4572cc3ba24dc01f5ad682135cf947f1e102c5b7","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_40badce2","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"in_reply_to":"5a3905b3_25ab2276","updated":"2017-01-19 15:20:37.000000000","message":"Oh. yes, you\u0027re right. Thanks. i\u0027ll fix this up as a separate change.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e45af366eec215f1cf1b8929ffa6b69b55f2941d","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_7aedc48a","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"in_reply_to":"5a3905b3_2b53f1e2","updated":"2017-01-19 16:12:14.000000000","message":"I agree. It\u0027s also good for performance to avoid unnecessary round trips to the database that could be running elsewhere in your cloud.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"9b70b2d9cb15ff7f7678a365a7383d17bfe1dc40","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_2b53f1e2","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"in_reply_to":"5a3905b3_40badce2","updated":"2017-01-19 15:33:24.000000000","message":"Separate change is fine.  I think you know the reason I\u0027m paying attention to this stuff is that overall I\u0027m trying to ensure that whatever view we have of e.g. share-instance is actually up-to-date and a racing thread hasn\u0027t changed things on us.  So when I see a refresh I ask whether it is actually helping with that goal.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"0aa9687b72a7435ea802bfdaaada0c922a80ae08","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_25ab2276","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"in_reply_to":"5a3905b3_51bb98c4","updated":"2017-01-19 15:18:30.000000000","message":"In this method?  You just call into other methods with the share_instance_id and they do their own DB gets with a fresher view of things.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"bcc1813f6fa0943e4255cd4eb4213ff5f6c8f98a","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_bc2aed7c","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"in_reply_to":"5a3905b3_7373584e","updated":"2017-01-19 03:49:00.000000000","message":"Yeah, it\u0027s used in a lot of places.  Quick scan suggested to me that in every case the caller has just fetched the instance before calling into this method with the instance ID.  But we\u0027d need to be careful to make sure that changes are not being made to the instance in that window (if they were, maybe the method that does the changes could itself return a fresh view of the instance).\n\nBut I agree, probably a diversion for this patch set.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0fee3eb1bb4aa3bebfb7427f437b9ef414422ff4","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_51bb98c4","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"in_reply_to":"5a3905b3_87f947cb","updated":"2017-01-19 14:55:59.000000000","message":"Not just that, we pass it down to the driver. We want to be sure we have the most updated share instance.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"050876e6270439c8c98e9238f4c28981c6dc16fc","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_87f947cb","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"in_reply_to":"5a3905b3_bc2aed7c","updated":"2017-01-19 14:20:19.000000000","message":"Actually all the share instance is used for is a debug log of its share_id.  Even though there are lots of callers, I bet all of them could pass the share_id from the share instance or replica in tha calling context.  But it can be a followup patch if this is right.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"a5fa1261d5ee89c7e17baab6b7ef296d3562efb8","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        existing access rules must be denied for a given share instance"},{"line_number":228,"context_line":"        :param share_server: Share server model or None"},{"line_number":229,"context_line":"        \"\"\""},{"line_number":230,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":231,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"},{"line_number":232,"context_line":"        msg_payload \u003d {"},{"line_number":233,"context_line":"            \u0027si\u0027: share_instance_id,"},{"line_number":234,"context_line":"            \u0027shr\u0027: share_instance[\u0027share_id\u0027],"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_7373584e","line":231,"range":{"start_line":230,"start_character":0,"end_line":231,"end_character":61},"in_reply_to":"7a3c09a3_eb665b28","updated":"2017-01-19 02:02:01.000000000","message":"True, but not sure it buys us a lot:\n\nMakes sense for some of these, but looking at all the places where this is invoked, that\u0027s not always the case:\n\n* Used: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@341\n   Share instance get: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@320\n\n* Used: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@1599\n   Share instance get: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@1574\n\n* Used: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@2084\n   Share instance get: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@2042\n\n* Used: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@2232\n   Share instance get: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@2228\n\n* Used: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@2732\n   Share instance get: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@2726\n\n* Used: https://review.openstack.org/#/c/369668/25/manila/share/migration.py@159\n   Share instance get: https://review.openstack.org/#/c/369668/25/manila/share/manager.py@893\n\n* Used:\nhttps://review.openstack.org/#/c/369668/25/manila/share/manager.py@795\n   Share instance get:\nhttps://review.openstack.org/#/c/369668/25/manila/share/manager.py@893","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"050876e6270439c8c98e9238f4c28981c6dc16fc","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                       \"%(si)s belonging to share %(shr)s.\")"},{"line_number":271,"context_line":"                LOG.debug(msg, msg_payload)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    def _update_access_rules(self, context, share_instance_id,"},{"line_number":274,"context_line":"                             share_server\u003dNone):"},{"line_number":275,"context_line":"        # Refresh the share instance model"},{"line_number":276,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":277,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_6ed190fb","line":274,"range":{"start_line":273,"start_character":0,"end_line":274,"end_character":48},"updated":"2017-01-19 14:20:19.000000000","message":"This important method is way too long for quality review and it is getting longer with your patch set - 181 lines, doing way more than one thing.\n\nCan we break it out into submethods - perhaps a method that determines the rule lists for the call into the driver, probably a submethod of that for the migration case, a method that calls into the driver (with legacy fallback) and that returns the driver rule updates, and a method that handles the driver rule updates, and a method that handles resync or finalizes the update?","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0fee3eb1bb4aa3bebfb7427f437b9ef414422ff4","unresolved":false,"context_lines":[{"line_number":270,"context_line":"                       \"%(si)s belonging to share %(shr)s.\")"},{"line_number":271,"context_line":"                LOG.debug(msg, msg_payload)"},{"line_number":272,"context_line":""},{"line_number":273,"context_line":"    def _update_access_rules(self, context, share_instance_id,"},{"line_number":274,"context_line":"                             share_server\u003dNone):"},{"line_number":275,"context_line":"        # Refresh the share instance model"},{"line_number":276,"context_line":"        share_instance \u003d self.db.share_instance_get("},{"line_number":277,"context_line":"            context, share_instance_id, with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_fab9b92f","line":274,"range":{"start_line":273,"start_character":0,"end_line":274,"end_character":48},"in_reply_to":"5a3905b3_6ed190fb","updated":"2017-01-19 14:55:59.000000000","message":"Done","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        conditionally_change[access_rules_status],"},{"line_number":84,"context_line":"                }"},{"line_number":85,"context_line":"            except KeyError:"},{"line_number":86,"context_line":"                updates \u003d {}"},{"line_number":87,"context_line":"        if updates:"},{"line_number":88,"context_line":"            share_instance \u003d self.db.share_instance_update("},{"line_number":89,"context_line":"                context, share_instance_id, updates, with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_da4eb8c0","line":86,"updated":"2017-01-19 17:14:47.000000000","message":"I think we should return the share_instance here, i.e. this method should always provide the freshest view of the updated (or unchanged) share instance rather than making the caller have to figure out whether an update was done in this method or not.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"75fb5163e5644a4b496f125bf958769b05fac811","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        conditionally_change[access_rules_status],"},{"line_number":84,"context_line":"                }"},{"line_number":85,"context_line":"            except KeyError:"},{"line_number":86,"context_line":"                updates \u003d {}"},{"line_number":87,"context_line":"        if updates:"},{"line_number":88,"context_line":"            share_instance \u003d self.db.share_instance_update("},{"line_number":89,"context_line":"                context, share_instance_id, updates, with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_bfc71644","line":86,"in_reply_to":"5a3905b3_845dafeb","updated":"2017-01-19 18:22:17.000000000","message":"I\u0027m thinking of line 283 in _update_access_rules() in this file.  That\u0027s the only place this method is called where the return value is collected and in that place there it is assigned to share_instance without checking that it is not None.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e779db6b9af01f099022f074ccdf62f495b6958c","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        conditionally_change[access_rules_status],"},{"line_number":84,"context_line":"                }"},{"line_number":85,"context_line":"            except KeyError:"},{"line_number":86,"context_line":"                updates \u003d {}"},{"line_number":87,"context_line":"        if updates:"},{"line_number":88,"context_line":"            share_instance \u003d self.db.share_instance_update("},{"line_number":89,"context_line":"                context, share_instance_id, updates, with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_e2b902eb","line":86,"in_reply_to":"5a3905b3_bfc71644","updated":"2017-01-19 19:20:29.000000000","message":"and that\u0027s by design :)","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eaeca88ea93580395582b6b53f1be5a2cb1adfd4","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                        conditionally_change[access_rules_status],"},{"line_number":84,"context_line":"                }"},{"line_number":85,"context_line":"            except KeyError:"},{"line_number":86,"context_line":"                updates \u003d {}"},{"line_number":87,"context_line":"        if updates:"},{"line_number":88,"context_line":"            share_instance \u003d self.db.share_instance_update("},{"line_number":89,"context_line":"                context, share_instance_id, updates, with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_845dafeb","line":86,"in_reply_to":"5a3905b3_da4eb8c0","updated":"2017-01-19 17:41:38.000000000","message":"Hmmm, where do you see this method used in the scenario you describe?\nAlso, if you return the share instance, how can you tell if the update was made? wouldn\u0027t you need to check the status again?","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        share_instance \u003d ("},{"line_number":283,"context_line":"            self.get_and_update_share_instance_access_rules_status("},{"line_number":284,"context_line":"                context, conditionally_change\u003dconditionally_change,"},{"line_number":285,"context_line":"                share_instance_id\u003dshare_instance_id) or share_instance"},{"line_number":286,"context_line":"        )"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        add_rules \u003d []"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_3a309c56","line":285,"range":{"start_line":285,"start_character":53,"end_line":285,"end_character":70},"updated":"2017-01-19 17:14:47.000000000","message":"Note that get_and_update_share_instance_access_rules_status always fetches the share instance.  If we just ensure that it always returns the share instance we can delete the fetch at lines 276-277.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eaeca88ea93580395582b6b53f1be5a2cb1adfd4","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        share_instance \u003d ("},{"line_number":283,"context_line":"            self.get_and_update_share_instance_access_rules_status("},{"line_number":284,"context_line":"                context, conditionally_change\u003dconditionally_change,"},{"line_number":285,"context_line":"                share_instance_id\u003dshare_instance_id) or share_instance"},{"line_number":286,"context_line":"        )"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        add_rules \u003d []"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_443107a2","line":285,"range":{"start_line":285,"start_character":53,"end_line":285,"end_character":70},"in_reply_to":"5a3905b3_3a309c56","updated":"2017-01-19 17:41:38.000000000","message":"No - see above","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"75fb5163e5644a4b496f125bf958769b05fac811","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        share_instance \u003d ("},{"line_number":283,"context_line":"            self.get_and_update_share_instance_access_rules_status("},{"line_number":284,"context_line":"                context, conditionally_change\u003dconditionally_change,"},{"line_number":285,"context_line":"                share_instance_id\u003dshare_instance_id) or share_instance"},{"line_number":286,"context_line":"        )"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        add_rules \u003d []"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_7f5f6e72","line":285,"range":{"start_line":285,"start_character":53,"end_line":285,"end_character":70},"in_reply_to":"5a3905b3_443107a2","updated":"2017-01-19 18:22:17.000000000","message":"Sure, you need it if get_and_update_share_instance_access_rules_status returns None but this is the only place you ever collect a return value from that method and you actually don\u0027t behave any differently if it didn\u0027t do an update: you use the share instance whether it was updated or not; just if it wasn\u0027t updated (by you) you use a slightly older view of it.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e779db6b9af01f099022f074ccdf62f495b6958c","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        share_instance \u003d ("},{"line_number":283,"context_line":"            self.get_and_update_share_instance_access_rules_status("},{"line_number":284,"context_line":"                context, conditionally_change\u003dconditionally_change,"},{"line_number":285,"context_line":"                share_instance_id\u003dshare_instance_id) or share_instance"},{"line_number":286,"context_line":"        )"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        add_rules \u003d []"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_e273c2af","line":285,"range":{"start_line":285,"start_character":53,"end_line":285,"end_character":70},"in_reply_to":"5a3905b3_7f5f6e72","updated":"2017-01-19 19:20:29.000000000","message":"Yes, not caring is deliberate.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"b3b267e9071bdeec99c5b9606ec3404e7561d262","unresolved":false,"context_lines":[{"line_number":282,"context_line":"        share_instance \u003d ("},{"line_number":283,"context_line":"            self.get_and_update_share_instance_access_rules_status("},{"line_number":284,"context_line":"                context, conditionally_change\u003dconditionally_change,"},{"line_number":285,"context_line":"                share_instance_id\u003dshare_instance_id) or share_instance"},{"line_number":286,"context_line":"        )"},{"line_number":287,"context_line":""},{"line_number":288,"context_line":"        add_rules \u003d []"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_08f9cd80","line":285,"range":{"start_line":285,"start_character":53,"end_line":285,"end_character":70},"in_reply_to":"5a3905b3_e273c2af","updated":"2017-01-19 19:23:27.000000000","message":"And the point is that *none* of the callers care, or so far as I can tell, none will need to.  So it could always return a fresh view.\n\nBut I\u0027m not digging in on this point as far as review progress is concerned.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        rules_to_be_removed_from_db \u003d []"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"        # Populate rules to send to the driver"},{"line_number":293,"context_line":"        access_rules_to_be_on_share \u003d self._get_rules_to_send_to_driver("},{"line_number":294,"context_line":"            context, add_rules, delete_rules, share_instance)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        if share_instance[\u0027status\u0027] \u003d\u003d constants.STATUS_MIGRATING:"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_1d791afd","line":293,"range":{"start_line":293,"start_character":43,"end_line":293,"end_character":71},"updated":"2017-01-19 17:14:47.000000000","message":"Can we make this method a function that returns a tuple instead of it taking null list inputs (add_rules and delete_rules) and changing them as side-effects?","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eaeca88ea93580395582b6b53f1be5a2cb1adfd4","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        rules_to_be_removed_from_db \u003d []"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"        # Populate rules to send to the driver"},{"line_number":293,"context_line":"        access_rules_to_be_on_share \u003d self._get_rules_to_send_to_driver("},{"line_number":294,"context_line":"            context, add_rules, delete_rules, share_instance)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        if share_instance[\u0027status\u0027] \u003d\u003d constants.STATUS_MIGRATING:"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_c4e4170f","line":293,"range":{"start_line":293,"start_character":43,"end_line":293,"end_character":71},"in_reply_to":"5a3905b3_1d791afd","updated":"2017-01-19 17:41:38.000000000","message":"? anything wrong with this implementation?","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"141db3543d365bef3fc39709391d32cd166ebc39","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        rules_to_be_removed_from_db \u003d []"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"        # Populate rules to send to the driver"},{"line_number":293,"context_line":"        access_rules_to_be_on_share \u003d self._get_rules_to_send_to_driver("},{"line_number":294,"context_line":"            context, add_rules, delete_rules, share_instance)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        if share_instance[\u0027status\u0027] \u003d\u003d constants.STATUS_MIGRATING:"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_53bc6a05","line":293,"range":{"start_line":293,"start_character":43,"end_line":293,"end_character":71},"in_reply_to":"5a3905b3_7f4dad71","updated":"2017-01-19 20:11:54.000000000","message":"Done","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"75fb5163e5644a4b496f125bf958769b05fac811","unresolved":false,"context_lines":[{"line_number":290,"context_line":"        rules_to_be_removed_from_db \u003d []"},{"line_number":291,"context_line":""},{"line_number":292,"context_line":"        # Populate rules to send to the driver"},{"line_number":293,"context_line":"        access_rules_to_be_on_share \u003d self._get_rules_to_send_to_driver("},{"line_number":294,"context_line":"            context, add_rules, delete_rules, share_instance)"},{"line_number":295,"context_line":""},{"line_number":296,"context_line":"        if share_instance[\u0027status\u0027] \u003d\u003d constants.STATUS_MIGRATING:"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_7f4dad71","line":293,"range":{"start_line":293,"start_character":43,"end_line":293,"end_character":71},"in_reply_to":"5a3905b3_c4e4170f","updated":"2017-01-19 18:22:17.000000000","message":"Yes.  Side effects are lies.\n\nI stole that line from Robert C. Martin, but the idea is that your method promises to do one thing and also does other hidden things.  The reader knows without looking into the implementation of _get_rules_to_send_to_driver that it is delivering access_rules_to_be_on_share.  But the reader has to study the method in question to see that it also updates add_rules and delete_rules.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            delete_rules \u003d []"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            driver_rule_updates \u003d None"},{"line_number":306,"context_line":"            try:"},{"line_number":307,"context_line":"                driver_rule_updates \u003d self.driver.update_access("},{"line_number":308,"context_line":"                    context,"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_537c7372","line":305,"range":{"start_line":305,"start_character":34,"end_line":305,"end_character":38},"updated":"2017-01-19 17:14:47.000000000","message":"{} ?","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"75fb5163e5644a4b496f125bf958769b05fac811","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            delete_rules \u003d []"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            driver_rule_updates \u003d None"},{"line_number":306,"context_line":"            try:"},{"line_number":307,"context_line":"                driver_rule_updates \u003d self.driver.update_access("},{"line_number":308,"context_line":"                    context,"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_7fe92d1a","line":305,"range":{"start_line":305,"start_character":34,"end_line":305,"end_character":38},"in_reply_to":"5a3905b3_448d8730","updated":"2017-01-19 18:22:17.000000000","message":"It leaves driver_rule_updates as None in the legacy fallback case, which is a problem when you get to line 323.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eaeca88ea93580395582b6b53f1be5a2cb1adfd4","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            delete_rules \u003d []"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            driver_rule_updates \u003d None"},{"line_number":306,"context_line":"            try:"},{"line_number":307,"context_line":"                driver_rule_updates \u003d self.driver.update_access("},{"line_number":308,"context_line":"                    context,"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_448d8730","line":305,"range":{"start_line":305,"start_character":34,"end_line":305,"end_character":38},"in_reply_to":"5a3905b3_537c7372","updated":"2017-01-19 17:41:38.000000000","message":"probably, but no effect..","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"141db3543d365bef3fc39709391d32cd166ebc39","unresolved":false,"context_lines":[{"line_number":302,"context_line":"            delete_rules \u003d []"},{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            driver_rule_updates \u003d None"},{"line_number":306,"context_line":"            try:"},{"line_number":307,"context_line":"                driver_rule_updates \u003d self.driver.update_access("},{"line_number":308,"context_line":"                    context,"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_f372b67b","line":305,"range":{"start_line":305,"start_character":34,"end_line":305,"end_character":38},"in_reply_to":"5a3905b3_7fe92d1a","updated":"2017-01-19 20:11:54.000000000","message":"Thanks, i totally missed this.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":311,"context_line":"                    add_rules\u003dadd_rules,"},{"line_number":312,"context_line":"                    delete_rules\u003ddelete_rules,"},{"line_number":313,"context_line":"                    share_server\u003dshare_server"},{"line_number":314,"context_line":"                ) or {}"},{"line_number":315,"context_line":"            except NotImplementedError:"},{"line_number":316,"context_line":"                # NOTE(u_glide): Fallback to legacy allow_access/deny_access"},{"line_number":317,"context_line":"                # for drivers without update_access() method support"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_33bfe7d1","line":314,"range":{"start_line":314,"start_character":18,"end_line":314,"end_character":23},"updated":"2017-01-19 17:14:47.000000000","message":"still need it to be {} for the legacy fallback branch.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eaeca88ea93580395582b6b53f1be5a2cb1adfd4","unresolved":false,"context_lines":[{"line_number":311,"context_line":"                    add_rules\u003dadd_rules,"},{"line_number":312,"context_line":"                    delete_rules\u003ddelete_rules,"},{"line_number":313,"context_line":"                    share_server\u003dshare_server"},{"line_number":314,"context_line":"                ) or {}"},{"line_number":315,"context_line":"            except NotImplementedError:"},{"line_number":316,"context_line":"                # NOTE(u_glide): Fallback to legacy allow_access/deny_access"},{"line_number":317,"context_line":"                # for drivers without update_access() method support"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_64828302","line":314,"range":{"start_line":314,"start_character":18,"end_line":314,"end_character":23},"in_reply_to":"5a3905b3_33bfe7d1","updated":"2017-01-19 17:41:38.000000000","message":"No. I will not make any changes to the fallback approach.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"75fb5163e5644a4b496f125bf958769b05fac811","unresolved":false,"context_lines":[{"line_number":311,"context_line":"                    add_rules\u003dadd_rules,"},{"line_number":312,"context_line":"                    delete_rules\u003ddelete_rules,"},{"line_number":313,"context_line":"                    share_server\u003dshare_server"},{"line_number":314,"context_line":"                ) or {}"},{"line_number":315,"context_line":"            except NotImplementedError:"},{"line_number":316,"context_line":"                # NOTE(u_glide): Fallback to legacy allow_access/deny_access"},{"line_number":317,"context_line":"                # for drivers without update_access() method support"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_1f702972","line":314,"range":{"start_line":314,"start_character":18,"end_line":314,"end_character":23},"in_reply_to":"5a3905b3_64828302","updated":"2017-01-19 18:22:17.000000000","message":"I don\u0027t mean to be suggesting that you change update_access_fallback().  I\u0027m just trying to say that even though you do the safe thing here (\u0027or {}\u0027) that doesn\u0027t suffice to cover the legacy fallback case where with your current code driver_rule_updates will still be None instead of {}, so you\u0027ll then have a problem at line 323.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            driver_rule_updates \u003d None"},{"line_number":306,"context_line":"            try:"},{"line_number":307,"context_line":"                driver_rule_updates \u003d self.driver.update_access("},{"line_number":308,"context_line":"                    context,"},{"line_number":309,"context_line":"                    share_instance,"},{"line_number":310,"context_line":"                    access_rules_to_be_on_share,"},{"line_number":311,"context_line":"                    add_rules\u003dadd_rules,"},{"line_number":312,"context_line":"                    delete_rules\u003ddelete_rules,"},{"line_number":313,"context_line":"                    share_server\u003dshare_server"},{"line_number":314,"context_line":"                ) or {}"},{"line_number":315,"context_line":"            except NotImplementedError:"},{"line_number":316,"context_line":"                # NOTE(u_glide): Fallback to legacy allow_access/deny_access"},{"line_number":317,"context_line":"                # for drivers without update_access() method support"},{"line_number":318,"context_line":"                self._update_access_fallback(context, add_rules,  delete_rules,"},{"line_number":319,"context_line":"                                             rules_to_be_removed_from_db,"},{"line_number":320,"context_line":"                                             share_instance,"},{"line_number":321,"context_line":"                                             share_server)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"            self._process_driver_rule_updates(context, driver_rule_updates,"},{"line_number":324,"context_line":"                                              share_instance_id)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_2ec77a13","line":321,"range":{"start_line":306,"start_character":0,"end_line":321,"end_character":58},"updated":"2017-01-19 17:14:47.000000000","message":"I really wish the nested try-excepts were avoided here by encapsulating the inner one in a try-new-method-and-fall-back-on-old method which returns driver_rule_updates (and  return value can be {} ).","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"75fb5163e5644a4b496f125bf958769b05fac811","unresolved":false,"context_lines":[{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            driver_rule_updates \u003d None"},{"line_number":306,"context_line":"            try:"},{"line_number":307,"context_line":"                driver_rule_updates \u003d self.driver.update_access("},{"line_number":308,"context_line":"                    context,"},{"line_number":309,"context_line":"                    share_instance,"},{"line_number":310,"context_line":"                    access_rules_to_be_on_share,"},{"line_number":311,"context_line":"                    add_rules\u003dadd_rules,"},{"line_number":312,"context_line":"                    delete_rules\u003ddelete_rules,"},{"line_number":313,"context_line":"                    share_server\u003dshare_server"},{"line_number":314,"context_line":"                ) or {}"},{"line_number":315,"context_line":"            except NotImplementedError:"},{"line_number":316,"context_line":"                # NOTE(u_glide): Fallback to legacy allow_access/deny_access"},{"line_number":317,"context_line":"                # for drivers without update_access() method support"},{"line_number":318,"context_line":"                self._update_access_fallback(context, add_rules,  delete_rules,"},{"line_number":319,"context_line":"                                             rules_to_be_removed_from_db,"},{"line_number":320,"context_line":"                                             share_instance,"},{"line_number":321,"context_line":"                                             share_server)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"            self._process_driver_rule_updates(context, driver_rule_updates,"},{"line_number":324,"context_line":"                                              share_instance_id)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_9f17b9a2","line":321,"range":{"start_line":306,"start_character":0,"end_line":321,"end_character":58},"in_reply_to":"5a3905b3_24b6bbdc","updated":"2017-01-19 18:22:17.000000000","message":"def do_driver_rule_updates(context, share_instance, access_rules_to_be_on_share, add_rules\u003dadd_rules, delete_rules\u003ddelete_rules, share_server\u003dshare_server):\n    try:\n      driver_rule_updates \u003d self.driver.update_access(...) or {}\n    except NotImplementedError:\n      self._update_access_fallback(...)\n      driver_rule_updates \u003d {}\n    return driver_rule_updates","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eaeca88ea93580395582b6b53f1be5a2cb1adfd4","unresolved":false,"context_lines":[{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            driver_rule_updates \u003d None"},{"line_number":306,"context_line":"            try:"},{"line_number":307,"context_line":"                driver_rule_updates \u003d self.driver.update_access("},{"line_number":308,"context_line":"                    context,"},{"line_number":309,"context_line":"                    share_instance,"},{"line_number":310,"context_line":"                    access_rules_to_be_on_share,"},{"line_number":311,"context_line":"                    add_rules\u003dadd_rules,"},{"line_number":312,"context_line":"                    delete_rules\u003ddelete_rules,"},{"line_number":313,"context_line":"                    share_server\u003dshare_server"},{"line_number":314,"context_line":"                ) or {}"},{"line_number":315,"context_line":"            except NotImplementedError:"},{"line_number":316,"context_line":"                # NOTE(u_glide): Fallback to legacy allow_access/deny_access"},{"line_number":317,"context_line":"                # for drivers without update_access() method support"},{"line_number":318,"context_line":"                self._update_access_fallback(context, add_rules,  delete_rules,"},{"line_number":319,"context_line":"                                             rules_to_be_removed_from_db,"},{"line_number":320,"context_line":"                                             share_instance,"},{"line_number":321,"context_line":"                                             share_server)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"            self._process_driver_rule_updates(context, driver_rule_updates,"},{"line_number":324,"context_line":"                                              share_instance_id)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_24b6bbdc","line":321,"range":{"start_line":306,"start_character":0,"end_line":321,"end_character":58},"in_reply_to":"5a3905b3_2ec77a13","updated":"2017-01-19 17:41:38.000000000","message":"we\u0027re expecting a specific exception. We\u0027ll be removing these fallback interfaces first thing when Pike opens up.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"141db3543d365bef3fc39709391d32cd166ebc39","unresolved":false,"context_lines":[{"line_number":303,"context_line":""},{"line_number":304,"context_line":"        try:"},{"line_number":305,"context_line":"            driver_rule_updates \u003d None"},{"line_number":306,"context_line":"            try:"},{"line_number":307,"context_line":"                driver_rule_updates \u003d self.driver.update_access("},{"line_number":308,"context_line":"                    context,"},{"line_number":309,"context_line":"                    share_instance,"},{"line_number":310,"context_line":"                    access_rules_to_be_on_share,"},{"line_number":311,"context_line":"                    add_rules\u003dadd_rules,"},{"line_number":312,"context_line":"                    delete_rules\u003ddelete_rules,"},{"line_number":313,"context_line":"                    share_server\u003dshare_server"},{"line_number":314,"context_line":"                ) or {}"},{"line_number":315,"context_line":"            except NotImplementedError:"},{"line_number":316,"context_line":"                # NOTE(u_glide): Fallback to legacy allow_access/deny_access"},{"line_number":317,"context_line":"                # for drivers without update_access() method support"},{"line_number":318,"context_line":"                self._update_access_fallback(context, add_rules,  delete_rules,"},{"line_number":319,"context_line":"                                             rules_to_be_removed_from_db,"},{"line_number":320,"context_line":"                                             share_instance,"},{"line_number":321,"context_line":"                                             share_server)"},{"line_number":322,"context_line":""},{"line_number":323,"context_line":"            self._process_driver_rule_updates(context, driver_rule_updates,"},{"line_number":324,"context_line":"                                              share_instance_id)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_b3266e66","line":321,"range":{"start_line":306,"start_character":0,"end_line":321,"end_character":58},"in_reply_to":"5a3905b3_9f17b9a2","updated":"2017-01-19 20:11:54.000000000","message":"Done","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":349,"context_line":"                conditionally_change\u003dconditionally_change_access_rules_status)"},{"line_number":350,"context_line":"            raise"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        if rules_to_be_removed_from_db:"},{"line_number":353,"context_line":"            delete_rules \u003d rules_to_be_removed_from_db"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        self.delete_share_instance_access_rules("},{"line_number":356,"context_line":"            context, delete_rules, share_instance_id\u003dshare_instance[\u0027id\u0027])"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        share_instance \u003d self.db.share_instance_get(context, share_instance_id,"},{"line_number":359,"context_line":"                                                    with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_2eddba14","line":356,"range":{"start_line":352,"start_character":8,"end_line":356,"end_character":74},"updated":"2017-01-19 17:14:47.000000000","message":"Just pass rules_to_be_removed_from_db directly to self.delete_share_instance_access rules since delete_rules isn\u0027t used otherwise.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"141db3543d365bef3fc39709391d32cd166ebc39","unresolved":false,"context_lines":[{"line_number":349,"context_line":"                conditionally_change\u003dconditionally_change_access_rules_status)"},{"line_number":350,"context_line":"            raise"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        if rules_to_be_removed_from_db:"},{"line_number":353,"context_line":"            delete_rules \u003d rules_to_be_removed_from_db"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        self.delete_share_instance_access_rules("},{"line_number":356,"context_line":"            context, delete_rules, share_instance_id\u003dshare_instance[\u0027id\u0027])"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        share_instance \u003d self.db.share_instance_get(context, share_instance_id,"},{"line_number":359,"context_line":"                                                    with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_6e9b39ca","line":356,"range":{"start_line":352,"start_character":8,"end_line":356,"end_character":74},"in_reply_to":"5a3905b3_24dddb14","updated":"2017-01-19 20:11:54.000000000","message":"The only time rules_to_be_removed_from_db is populated is when the share is migrating and we\u0027re casting all rules to readonly, we don\u0027t want to pass anything in the delete_rules parameter there and just junk the rules out of the db if there are any. In this case delete_rules will be []\n\nIn other cases, rules_to_be_removed_from_db will be []..","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eaeca88ea93580395582b6b53f1be5a2cb1adfd4","unresolved":false,"context_lines":[{"line_number":349,"context_line":"                conditionally_change\u003dconditionally_change_access_rules_status)"},{"line_number":350,"context_line":"            raise"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        if rules_to_be_removed_from_db:"},{"line_number":353,"context_line":"            delete_rules \u003d rules_to_be_removed_from_db"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        self.delete_share_instance_access_rules("},{"line_number":356,"context_line":"            context, delete_rules, share_instance_id\u003dshare_instance[\u0027id\u0027])"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        share_instance \u003d self.db.share_instance_get(context, share_instance_id,"},{"line_number":359,"context_line":"                                                    with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_24dddb14","line":356,"range":{"start_line":352,"start_character":8,"end_line":356,"end_character":74},"in_reply_to":"5a3905b3_2eddba14","updated":"2017-01-19 17:41:38.000000000","message":"Okay","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"8c1efe0c67de5bb51231d365e9756e4ab5375891","unresolved":false,"context_lines":[{"line_number":349,"context_line":"                conditionally_change\u003dconditionally_change_access_rules_status)"},{"line_number":350,"context_line":"            raise"},{"line_number":351,"context_line":""},{"line_number":352,"context_line":"        if rules_to_be_removed_from_db:"},{"line_number":353,"context_line":"            delete_rules \u003d rules_to_be_removed_from_db"},{"line_number":354,"context_line":""},{"line_number":355,"context_line":"        self.delete_share_instance_access_rules("},{"line_number":356,"context_line":"            context, delete_rules, share_instance_id\u003dshare_instance[\u0027id\u0027])"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        share_instance \u003d self.db.share_instance_get(context, share_instance_id,"},{"line_number":359,"context_line":"                                                    with_share_data\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_193f61b5","line":356,"range":{"start_line":352,"start_character":8,"end_line":356,"end_character":74},"in_reply_to":"5a3905b3_6e9b39ca","updated":"2017-01-19 20:24:22.000000000","message":"Right.  I was missing the case here where rules_to_be_removed_from_db was empty/False.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        self.delete_share_instance_access_rules("},{"line_number":356,"context_line":"            context, delete_rules, share_instance_id\u003dshare_instance[\u0027id\u0027])"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        share_instance \u003d self.db.share_instance_get(context, share_instance_id,"},{"line_number":359,"context_line":"                                                    with_share_data\u003dTrue)"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        self._loop_for_refresh_else_update_access_rules_status("},{"line_number":362,"context_line":"            context, share_instance_id, share_server)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_29d05453","line":359,"range":{"start_line":358,"start_character":0,"end_line":359,"end_character":73},"updated":"2017-01-19 17:14:47.000000000","message":"Is this useful?  I only see it used for the msg payload in the LOG message to extract the share_id.  But we\u0027ve already fetched the share instance earlier in this method (lines 276 and 282) and the share id won\u0027t have changed.   We need to get fresh info inside _loop_for_refresh .... but we aren\u0027t passing this share_instance in anyways.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"eaeca88ea93580395582b6b53f1be5a2cb1adfd4","unresolved":false,"context_lines":[{"line_number":355,"context_line":"        self.delete_share_instance_access_rules("},{"line_number":356,"context_line":"            context, delete_rules, share_instance_id\u003dshare_instance[\u0027id\u0027])"},{"line_number":357,"context_line":""},{"line_number":358,"context_line":"        share_instance \u003d self.db.share_instance_get(context, share_instance_id,"},{"line_number":359,"context_line":"                                                    with_share_data\u003dTrue)"},{"line_number":360,"context_line":""},{"line_number":361,"context_line":"        self._loop_for_refresh_else_update_access_rules_status("},{"line_number":362,"context_line":"            context, share_instance_id, share_server)"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_c4c7b743","line":359,"range":{"start_line":358,"start_character":0,"end_line":359,"end_character":73},"in_reply_to":"5a3905b3_29d05453","updated":"2017-01-19 17:41:38.000000000","message":"Will remove","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"cb2975020b31dfb1f599ba5ddf1022389fa16584","unresolved":false,"context_lines":[{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    def _process_driver_rule_updates(self, context, driver_rule_updates,"},{"line_number":409,"context_line":"                                     share_instance_id):"},{"line_number":410,"context_line":"        for rule_id, rule_updates in driver_rule_updates.items():"},{"line_number":411,"context_line":"            if \u0027state\u0027 in rule_updates:"},{"line_number":412,"context_line":"                # We allow updates *only* if the state is unchanged from"},{"line_number":413,"context_line":"                # the time this update was initiated. It is possible"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_d36223d8","line":410,"range":{"start_line":410,"start_character":37,"end_line":410,"end_character":63},"updated":"2017-01-19 17:14:47.000000000","message":"you\u0027ll get an Attribute Error if this is None, which currently it can be.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"75fb5163e5644a4b496f125bf958769b05fac811","unresolved":false,"context_lines":[{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    def _process_driver_rule_updates(self, context, driver_rule_updates,"},{"line_number":409,"context_line":"                                     share_instance_id):"},{"line_number":410,"context_line":"        for rule_id, rule_updates in driver_rule_updates.items():"},{"line_number":411,"context_line":"            if \u0027state\u0027 in rule_updates:"},{"line_number":412,"context_line":"                # We allow updates *only* if the state is unchanged from"},{"line_number":413,"context_line":"                # the time this update was initiated. It is possible"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_df5f8170","line":410,"range":{"start_line":410,"start_character":37,"end_line":410,"end_character":63},"in_reply_to":"5a3905b3_d36223d8","updated":"2017-01-19 18:22:17.000000000","message":"This is why you need driver_rule_updates to be {} rather than None.  Alternatively, you could check for \u0027not None\u0027 before calling this method but that is uglier.","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"141db3543d365bef3fc39709391d32cd166ebc39","unresolved":false,"context_lines":[{"line_number":407,"context_line":""},{"line_number":408,"context_line":"    def _process_driver_rule_updates(self, context, driver_rule_updates,"},{"line_number":409,"context_line":"                                     share_instance_id):"},{"line_number":410,"context_line":"        for rule_id, rule_updates in driver_rule_updates.items():"},{"line_number":411,"context_line":"            if \u0027state\u0027 in rule_updates:"},{"line_number":412,"context_line":"                # We allow updates *only* if the state is unchanged from"},{"line_number":413,"context_line":"                # the time this update was initiated. It is possible"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_4e53dde1","line":410,"range":{"start_line":410,"start_character":37,"end_line":410,"end_character":63},"in_reply_to":"5a3905b3_df5f8170","updated":"2017-01-19 20:11:54.000000000","message":"Done","commit_id":"c50d0c39f229a93f34cb20ef6142702a38c7188b"}],"manila/share/api.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"834295b0ca82356b35f0ef7cf910cae7e2db325a","unresolved":false,"context_lines":[{"line_number":1391,"context_line":"        return access"},{"line_number":1392,"context_line":""},{"line_number":1393,"context_line":"    def allow_access_to_instance(self, context, share_instance, access):"},{"line_number":1394,"context_line":"        policy.check_policy(context, \u0027share\u0027, \u0027allow_access\u0027)"},{"line_number":1395,"context_line":""},{"line_number":1396,"context_line":"        if not share_instance[\u0027host\u0027]:"},{"line_number":1397,"context_line":"            msg \u003d _(\"Invalid share instance host: %s\") % share_instance[\u0027host\u0027]"},{"line_number":1398,"context_line":"            raise exception.InvalidShareInstance(reason\u003dmsg)"},{"line_number":1399,"context_line":""},{"line_number":1400,"context_line":"        status \u003d share_instance[\u0027access_rules_status\u0027]"},{"line_number":1401,"context_line":""},{"line_number":1402,"context_line":"        if status \u003d\u003d constants.STATUS_ERROR:"},{"line_number":1403,"context_line":"            values \u003d {"},{"line_number":1404,"context_line":"                \u0027instance_id\u0027: share_instance[\u0027id\u0027],"},{"line_number":1405,"context_line":"                \u0027status\u0027: status,"},{"line_number":1406,"context_line":"                \u0027valid_status\u0027: constants.STATUS_ACTIVE"},{"line_number":1407,"context_line":"            }"},{"line_number":1408,"context_line":"            msg \u003d _(\"Share instance %(instance_id)s access rules status is: \""},{"line_number":1409,"context_line":"                    \"%(status)s. Please remove any incorrect rules to get it \""},{"line_number":1410,"context_line":"                    \"back to %(valid_status)s.\") % values"},{"line_number":1411,"context_line":""},{"line_number":1412,"context_line":"            raise exception.InvalidShareInstance(reason\u003dmsg)"},{"line_number":1413,"context_line":"        else:"},{"line_number":1414,"context_line":"            if status \u003d\u003d constants.STATUS_ACTIVE:"},{"line_number":1415,"context_line":"                self.db.share_instance_update_access_status("},{"line_number":1416,"context_line":"                    context, share_instance[\u0027id\u0027],"},{"line_number":1417,"context_line":"                    constants.STATUS_OUT_OF_SYNC"},{"line_number":1418,"context_line":"                )"},{"line_number":1419,"context_line":"            elif status \u003d\u003d constants.STATUS_UPDATING:"},{"line_number":1420,"context_line":"                self.db.share_instance_update_access_status("},{"line_number":1421,"context_line":"                    context, share_instance[\u0027id\u0027],"},{"line_number":1422,"context_line":"                    constants.STATUS_UPDATING_MULTIPLE"},{"line_number":1423,"context_line":"                )"},{"line_number":1424,"context_line":""},{"line_number":1425,"context_line":"            self.share_rpcapi.allow_access(context, share_instance, access)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_bb04c0c6","side":"PARENT","line":1422,"range":{"start_line":1394,"start_character":7,"end_line":1422,"end_character":46},"updated":"2016-09-13 23:50:21.000000000","message":"very nice, now we don\u0027t have the case where one instance gets the rule applied and the other one doesn\u0027t","commit_id":"0a2fcb86bd9f128a5ad12e9218e34f0c724cd2e8"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"834295b0ca82356b35f0ef7cf910cae7e2db325a","unresolved":false,"context_lines":[{"line_number":1388,"context_line":"                        instance[\u0027access_rules_status\u0027] \u003d\u003d"},{"line_number":1389,"context_line":"                        constants.STATUS_ERROR)"},{"line_number":1390,"context_line":""},{"line_number":1391,"context_line":"            invalid_instances \u003d [instance for instance in share.instances if"},{"line_number":1392,"context_line":"                                 _is_invalid_share_instance(instance)]"},{"line_number":1393,"context_line":"            if invalid_instances:"},{"line_number":1394,"context_line":"                msg \u003d _(\"New access rules cannot be applied while some \""},{"line_number":1395,"context_line":"                        \"replicas or migration copies of the share \""},{"line_number":1396,"context_line":"                        \"do not have a valid host, are in an invalid state \""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_fbc9e8ad","line":1393,"range":{"start_line":1391,"start_character":12,"end_line":1393,"end_character":33},"updated":"2016-09-13 23:50:21.000000000","message":"you can use \"if any(...):\"","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e6d143838d6f8e496972a7dce1f97ef883375ff7","unresolved":false,"context_lines":[{"line_number":1388,"context_line":"                        instance[\u0027access_rules_status\u0027] \u003d\u003d"},{"line_number":1389,"context_line":"                        constants.STATUS_ERROR)"},{"line_number":1390,"context_line":""},{"line_number":1391,"context_line":"            invalid_instances \u003d [instance for instance in share.instances if"},{"line_number":1392,"context_line":"                                 _is_invalid_share_instance(instance)]"},{"line_number":1393,"context_line":"            if invalid_instances:"},{"line_number":1394,"context_line":"                msg \u003d _(\"New access rules cannot be applied while some \""},{"line_number":1395,"context_line":"                        \"replicas or migration copies of the share \""},{"line_number":1396,"context_line":"                        \"do not have a valid host, are in an invalid state \""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a8bc5a2_470b147d","line":1393,"range":{"start_line":1391,"start_character":12,"end_line":1393,"end_character":33},"in_reply_to":"7a8ec9b2_fbc9e8ad","updated":"2016-10-03 15:00:10.000000000","message":"Done","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"834295b0ca82356b35f0ef7cf910cae7e2db325a","unresolved":false,"context_lines":[{"line_number":1398,"context_line":"                        \"to %s.\") % constants.STATUS_ERROR"},{"line_number":1399,"context_line":"                raise exception.InvalidShareInstance(message\u003dmsg)"},{"line_number":1400,"context_line":"        else:"},{"line_number":1401,"context_line":"            if share.instance[\u0027host\u0027] is None:"},{"line_number":1402,"context_line":"                msg \u003d _(\"New access rules cannot be applied while the share \""},{"line_number":1403,"context_line":"                        \"does not have a valid host.\")"},{"line_number":1404,"context_line":"                raise exception.InvalidShare(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_bb5da0ee","line":1401,"range":{"start_line":1401,"start_character":12,"end_line":1401,"end_character":46},"updated":"2016-09-13 23:50:21.000000000","message":"so, above if you have more than one instance, you don\u0027t allow rule to be applied if instance is \"extending\", but here if you have only one you do, it is not clear to me why","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"18a1a8e44aae23366d56fe33b5a14f76470646ce","unresolved":false,"context_lines":[{"line_number":1398,"context_line":"                        \"to %s.\") % constants.STATUS_ERROR"},{"line_number":1399,"context_line":"                raise exception.InvalidShareInstance(message\u003dmsg)"},{"line_number":1400,"context_line":"        else:"},{"line_number":1401,"context_line":"            if share.instance[\u0027host\u0027] is None:"},{"line_number":1402,"context_line":"                msg \u003d _(\"New access rules cannot be applied while the share \""},{"line_number":1403,"context_line":"                        \"does not have a valid host.\")"},{"line_number":1404,"context_line":"                raise exception.InvalidShare(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_ca30d00a","line":1401,"range":{"start_line":1401,"start_character":12,"end_line":1401,"end_character":46},"in_reply_to":"7a8ec9b2_bb5da0ee","updated":"2016-09-14 00:05:04.000000000","message":"If the share only has one instance, the status check done on line 1349 will suffice","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e6d143838d6f8e496972a7dce1f97ef883375ff7","unresolved":false,"context_lines":[{"line_number":1398,"context_line":"                        \"to %s.\") % constants.STATUS_ERROR"},{"line_number":1399,"context_line":"                raise exception.InvalidShareInstance(message\u003dmsg)"},{"line_number":1400,"context_line":"        else:"},{"line_number":1401,"context_line":"            if share.instance[\u0027host\u0027] is None:"},{"line_number":1402,"context_line":"                msg \u003d _(\"New access rules cannot be applied while the share \""},{"line_number":1403,"context_line":"                        \"does not have a valid host.\")"},{"line_number":1404,"context_line":"                raise exception.InvalidShare(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"3a98d1f4_eba78ba4","line":1401,"range":{"start_line":1401,"start_character":12,"end_line":1401,"end_character":46},"in_reply_to":"7a8ec9b2_ca30d00a","updated":"2016-10-03 15:00:10.000000000","message":"I was wrong.. refactored..","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"834295b0ca82356b35f0ef7cf910cae7e2db325a","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"                return (instance[\u0027host\u0027] is None or instance[\u0027status\u0027] in"},{"line_number":1443,"context_line":"                        constants.TRANSITIONAL_STATUSES)"},{"line_number":1444,"context_line":""},{"line_number":1445,"context_line":"            invalid_instances \u003d [instance for instance in share.instances if"},{"line_number":1446,"context_line":"                                 _is_invalid_share_instance(instance)]"},{"line_number":1447,"context_line":"            if invalid_instances:"},{"line_number":1448,"context_line":"                msg \u003d _(\"Access rules cannot be denied while some replicas \""},{"line_number":1449,"context_line":"                        \"or migration copies of the share do not have a \""}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_8abec8fd","line":1446,"range":{"start_line":1445,"start_character":32,"end_line":1446,"end_character":70},"updated":"2016-09-13 23:50:21.000000000","message":"same as above","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e6d143838d6f8e496972a7dce1f97ef883375ff7","unresolved":false,"context_lines":[{"line_number":1442,"context_line":"                return (instance[\u0027host\u0027] is None or instance[\u0027status\u0027] in"},{"line_number":1443,"context_line":"                        constants.TRANSITIONAL_STATUSES)"},{"line_number":1444,"context_line":""},{"line_number":1445,"context_line":"            invalid_instances \u003d [instance for instance in share.instances if"},{"line_number":1446,"context_line":"                                 _is_invalid_share_instance(instance)]"},{"line_number":1447,"context_line":"            if invalid_instances:"},{"line_number":1448,"context_line":"                msg \u003d _(\"Access rules cannot be denied while some replicas \""},{"line_number":1449,"context_line":"                        \"or migration copies of the share do not have a \""}],"source_content_type":"text/x-python","patch_set":1,"id":"5a8bc5a2_670e508d","line":1446,"range":{"start_line":1445,"start_character":32,"end_line":1446,"end_character":70},"in_reply_to":"7a8ec9b2_8abec8fd","updated":"2016-10-03 15:00:10.000000000","message":"Done","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"834295b0ca82356b35f0ef7cf910cae7e2db325a","unresolved":false,"context_lines":[{"line_number":1450,"context_line":"                        \"valid host, are in a transitional state.\")"},{"line_number":1451,"context_line":"                raise exception.InvalidShareInstance(message\u003dmsg)"},{"line_number":1452,"context_line":"        else:"},{"line_number":1453,"context_line":"            if share.instance[\u0027host\u0027] is None:"},{"line_number":1454,"context_line":"                msg \u003d _(\"Access rules cannot be denied while the share \""},{"line_number":1455,"context_line":"                        \"does not have a valid host.\")"},{"line_number":1456,"context_line":"                raise exception.InvalidShare(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_6ac17c80","line":1453,"range":{"start_line":1453,"start_character":12,"end_line":1453,"end_character":46},"updated":"2016-09-13 23:50:21.000000000","message":"same as above","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e6d143838d6f8e496972a7dce1f97ef883375ff7","unresolved":false,"context_lines":[{"line_number":1450,"context_line":"                        \"valid host, are in a transitional state.\")"},{"line_number":1451,"context_line":"                raise exception.InvalidShareInstance(message\u003dmsg)"},{"line_number":1452,"context_line":"        else:"},{"line_number":1453,"context_line":"            if share.instance[\u0027host\u0027] is None:"},{"line_number":1454,"context_line":"                msg \u003d _(\"Access rules cannot be denied while the share \""},{"line_number":1455,"context_line":"                        \"does not have a valid host.\")"},{"line_number":1456,"context_line":"                raise exception.InvalidShare(message\u003dmsg)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a8bc5a2_87299ce8","line":1453,"range":{"start_line":1453,"start_character":12,"end_line":1453,"end_character":46},"in_reply_to":"7a8ec9b2_6ac17c80","updated":"2016-10-03 15:00:10.000000000","message":"same as above","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":1364,"context_line":"               if _is_invalid_share_instance(instance)):"},{"line_number":1365,"context_line":"            if len(share.instances) \u003e 1:"},{"line_number":1366,"context_line":"                msg \u003d _(\"New access rules cannot be applied while some \""},{"line_number":1367,"context_line":"                        \"replicas or migration copies of the share \""},{"line_number":1368,"context_line":"                        \"do not have a valid host, are in an invalid state \""},{"line_number":1369,"context_line":"                        \"or have their access_rules_status set \""},{"line_number":1370,"context_line":"                        \"to %s.\") % constants.STATUS_ERROR"}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_b3ff8533","line":1367,"range":{"start_line":1367,"start_character":25,"end_line":1367,"end_character":53},"updated":"2016-10-21 11:29:05.000000000","message":"this is bound to become outdated, I would just use \"instances\", but ok.","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2c70df0025fd039b8b674bac1a28ae4bceaa53bb","unresolved":false,"context_lines":[{"line_number":1407,"context_line":"        if any(instance for instance in share.instances if"},{"line_number":1408,"context_line":"               _is_invalid_share_instance(instance)):"},{"line_number":1409,"context_line":"            if len(share.instances) \u003e 1:"},{"line_number":1410,"context_line":"                msg \u003d _(\"Access rules cannot be denied while some replicas \""},{"line_number":1411,"context_line":"                        \"or migration copies of the share do not have a \""},{"line_number":1412,"context_line":"                        \"valid host, or are in an invalid state.\")"},{"line_number":1413,"context_line":"            else:"},{"line_number":1414,"context_line":"                msg \u003d _(\"Access rules cannot be denied while the share \""}],"source_content_type":"text/x-python","patch_set":3,"id":"da6895a0_5312a963","line":1411,"range":{"start_line":1410,"start_character":65,"end_line":1411,"end_character":44},"updated":"2016-10-21 11:29:05.000000000","message":"same as above","commit_id":"5bf009a50807f43034f75912b87ac6188fe5c81a"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"48452cbddd7970a765d038a5fde497d5f6d4ec5a","unresolved":false,"context_lines":[{"line_number":1433,"context_line":"    def _conditionally_transition_share_instance_access_rules_status("},{"line_number":1434,"context_line":"            self, context, share_instance):"},{"line_number":1435,"context_line":"        instance_access_rules_status \u003d share_instance[\u0027access_rules_status\u0027]"},{"line_number":1436,"context_line":"        if (instance_access_rules_status !\u003d"},{"line_number":1437,"context_line":"                constants.SHARE_INSTANCE_RULES_ERROR):"},{"line_number":1438,"context_line":"            self.access_helper.update_share_instance_access_rules_status("},{"line_number":1439,"context_line":"                context, state\u003dconstants.SHARE_INSTANCE_RULES_SYNCING,"},{"line_number":1440,"context_line":"                share_instance_id\u003dshare_instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_20e00f96","line":1437,"range":{"start_line":1436,"start_character":8,"end_line":1437,"end_character":54},"updated":"2017-01-11 19:19:03.000000000","message":"shouldn\u0027t this check be performed within the method below to avoid race conditions?","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0c81951e4e23a56d3ae0d1da8d8aeadc20845115","unresolved":false,"context_lines":[{"line_number":1433,"context_line":"    def _conditionally_transition_share_instance_access_rules_status("},{"line_number":1434,"context_line":"            self, context, share_instance):"},{"line_number":1435,"context_line":"        instance_access_rules_status \u003d share_instance[\u0027access_rules_status\u0027]"},{"line_number":1436,"context_line":"        if (instance_access_rules_status !\u003d"},{"line_number":1437,"context_line":"                constants.SHARE_INSTANCE_RULES_ERROR):"},{"line_number":1438,"context_line":"            self.access_helper.update_share_instance_access_rules_status("},{"line_number":1439,"context_line":"                context, state\u003dconstants.SHARE_INSTANCE_RULES_SYNCING,"},{"line_number":1440,"context_line":"                share_instance_id\u003dshare_instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_3853dfe3","line":1437,"range":{"start_line":1436,"start_character":8,"end_line":1437,"end_character":54},"in_reply_to":"ba5201f7_20e00f96","updated":"2017-01-11 19:47:25.000000000","message":"We could do it in the method below... with a similar logic as we\u0027re using to conditionally transform access rules","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2307d66a51cdf13e911af61fe2482bccada0201e","unresolved":false,"context_lines":[{"line_number":1433,"context_line":"    def _conditionally_transition_share_instance_access_rules_status("},{"line_number":1434,"context_line":"            self, context, share_instance):"},{"line_number":1435,"context_line":"        instance_access_rules_status \u003d share_instance[\u0027access_rules_status\u0027]"},{"line_number":1436,"context_line":"        if (instance_access_rules_status !\u003d"},{"line_number":1437,"context_line":"                constants.SHARE_INSTANCE_RULES_ERROR):"},{"line_number":1438,"context_line":"            self.access_helper.update_share_instance_access_rules_status("},{"line_number":1439,"context_line":"                context, state\u003dconstants.SHARE_INSTANCE_RULES_SYNCING,"},{"line_number":1440,"context_line":"                share_instance_id\u003dshare_instance[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_fbe871e4","line":1437,"range":{"start_line":1436,"start_character":8,"end_line":1437,"end_character":54},"in_reply_to":"ba5201f7_3853dfe3","updated":"2017-01-11 19:54:55.000000000","message":"Yes I think so, so it is more like \"get and set\". In this case right now the \"get\" is unprotected from locks, so it may be taking a decision based on stale data. I know it will not work until we have tooz, but we will be ready.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":8851,"name":"Valeriy Ponomaryov","email":"kiparis.kh@gmail.com","username":"vponomaryov"},"change_message_id":"e0f698c7c31f9a721d3a32ae2af18246eeed92be","unresolved":false,"context_lines":[{"line_number":1396,"context_line":"            msg \u003d _(\"Invalid share access level: %s.\") % access_level"},{"line_number":1397,"context_line":"            raise exception.InvalidShareAccess(reason\u003dmsg)"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"        def _is_invalid_share_instance(instance):"},{"line_number":1400,"context_line":"            return (instance[\u0027host\u0027] is None or"},{"line_number":1401,"context_line":"                    instance[\u0027status\u0027] in constants."},{"line_number":1402,"context_line":"                    INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES)"},{"line_number":1403,"context_line":""},{"line_number":1404,"context_line":"        if any(instance for instance in share.instances"},{"line_number":1405,"context_line":"               if _is_invalid_share_instance(instance)):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_b72df5a9","line":1402,"range":{"start_line":1399,"start_character":7,"end_line":1402,"end_character":76},"updated":"2017-01-17 17:45:49.000000000","message":"It is defined twice as local function, so would be better to define it only once, maybe as static method?","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":1396,"context_line":"            msg \u003d _(\"Invalid share access level: %s.\") % access_level"},{"line_number":1397,"context_line":"            raise exception.InvalidShareAccess(reason\u003dmsg)"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"        def _is_invalid_share_instance(instance):"},{"line_number":1400,"context_line":"            return (instance[\u0027host\u0027] is None or"},{"line_number":1401,"context_line":"                    instance[\u0027status\u0027] in constants."},{"line_number":1402,"context_line":"                    INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES)"},{"line_number":1403,"context_line":""},{"line_number":1404,"context_line":"        if any(instance for instance in share.instances"},{"line_number":1405,"context_line":"               if _is_invalid_share_instance(instance)):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_4b70a8dc","line":1402,"range":{"start_line":1399,"start_character":7,"end_line":1402,"end_character":76},"in_reply_to":"7a3c09a3_2df1b401","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":1396,"context_line":"            msg \u003d _(\"Invalid share access level: %s.\") % access_level"},{"line_number":1397,"context_line":"            raise exception.InvalidShareAccess(reason\u003dmsg)"},{"line_number":1398,"context_line":""},{"line_number":1399,"context_line":"        def _is_invalid_share_instance(instance):"},{"line_number":1400,"context_line":"            return (instance[\u0027host\u0027] is None or"},{"line_number":1401,"context_line":"                    instance[\u0027status\u0027] in constants."},{"line_number":1402,"context_line":"                    INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES)"},{"line_number":1403,"context_line":""},{"line_number":1404,"context_line":"        if any(instance for instance in share.instances"},{"line_number":1405,"context_line":"               if _is_invalid_share_instance(instance)):"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_2df1b401","line":1402,"range":{"start_line":1399,"start_character":7,"end_line":1402,"end_character":76},"in_reply_to":"7a3c09a3_b72df5a9","updated":"2017-01-17 18:01:57.000000000","message":"thanks. will do","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"300d6ee6636bd73b0c950228ad54d4f5ecbf65ea","unresolved":false,"context_lines":[{"line_number":1532,"context_line":"                raise exception.ShareAccessExists(access_type\u003daccess_type,"},{"line_number":1533,"context_line":"                                                  access\u003daccess_to)"},{"line_number":1534,"context_line":""},{"line_number":1535,"context_line":"        if access_level not in constants.ACCESS_LEVELS + (None, ):"},{"line_number":1536,"context_line":"            msg \u003d _(\"Invalid share access level: %s.\") % access_level"},{"line_number":1537,"context_line":"            raise exception.InvalidShareAccess(reason\u003dmsg)"},{"line_number":1538,"context_line":""},{"line_number":1539,"context_line":"        if any(instance for instance in share.instances"},{"line_number":1540,"context_line":"               if self._is_invalid_share_instance(instance)):"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_e2305c58","line":1537,"range":{"start_line":1535,"start_character":0,"end_line":1537,"end_character":58},"updated":"2017-01-18 17:37:43.000000000","message":"Consider moving this before line 1527 since this check, as opposed to the other validation checks here, does not need protection from races.  Note that I am thinking that the other validation checks do need protection ...","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0d6fd79d3868c7c0b4a2f18d8d4aedf3a75c35ed","unresolved":false,"context_lines":[{"line_number":1532,"context_line":"                raise exception.ShareAccessExists(access_type\u003daccess_type,"},{"line_number":1533,"context_line":"                                                  access\u003daccess_to)"},{"line_number":1534,"context_line":""},{"line_number":1535,"context_line":"        if access_level not in constants.ACCESS_LEVELS + (None, ):"},{"line_number":1536,"context_line":"            msg \u003d _(\"Invalid share access level: %s.\") % access_level"},{"line_number":1537,"context_line":"            raise exception.InvalidShareAccess(reason\u003dmsg)"},{"line_number":1538,"context_line":""},{"line_number":1539,"context_line":"        if any(instance for instance in share.instances"},{"line_number":1540,"context_line":"               if self._is_invalid_share_instance(instance)):"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_79709e40","line":1537,"range":{"start_line":1535,"start_character":0,"end_line":1537,"end_character":58},"in_reply_to":"7a3c09a3_e2305c58","updated":"2017-01-18 19:20:56.000000000","message":"+1","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"300d6ee6636bd73b0c950228ad54d4f5ecbf65ea","unresolved":false,"context_lines":[{"line_number":1539,"context_line":"        if any(instance for instance in share.instances"},{"line_number":1540,"context_line":"               if self._is_invalid_share_instance(instance)):"},{"line_number":1541,"context_line":"            msg \u003d _(\"New access rules cannot be applied while the share, \""},{"line_number":1542,"context_line":"                    \"any of its replicas or migration copies does not \""},{"line_number":1543,"context_line":"                    \"have a valid host or is in an invalid state.\")"},{"line_number":1544,"context_line":"            raise exception.InvalidShare(message\u003dmsg)"},{"line_number":1545,"context_line":""},{"line_number":1546,"context_line":"        values \u003d {"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_9d661db9","line":1543,"range":{"start_line":1542,"start_character":61,"end_line":1543,"end_character":27},"updated":"2017-01-18 17:37:43.000000000","message":"suggest \"lacks a valid host ...\"  Nice finesse on not exposing share intances through the share abstraction :D","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0d6fd79d3868c7c0b4a2f18d8d4aedf3a75c35ed","unresolved":false,"context_lines":[{"line_number":1539,"context_line":"        if any(instance for instance in share.instances"},{"line_number":1540,"context_line":"               if self._is_invalid_share_instance(instance)):"},{"line_number":1541,"context_line":"            msg \u003d _(\"New access rules cannot be applied while the share, \""},{"line_number":1542,"context_line":"                    \"any of its replicas or migration copies does not \""},{"line_number":1543,"context_line":"                    \"have a valid host or is in an invalid state.\")"},{"line_number":1544,"context_line":"            raise exception.InvalidShare(message\u003dmsg)"},{"line_number":1545,"context_line":""},{"line_number":1546,"context_line":"        values \u003d {"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_19a6baad","line":1543,"range":{"start_line":1542,"start_character":61,"end_line":1543,"end_character":27},"in_reply_to":"7a3c09a3_9d661db9","updated":"2017-01-18 19:20:56.000000000","message":"thank you.. will fix.","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"300d6ee6636bd73b0c950228ad54d4f5ecbf65ea","unresolved":false,"context_lines":[{"line_number":1580,"context_line":"        if any(instance for instance in share.instances if"},{"line_number":1581,"context_line":"               self._is_invalid_share_instance(instance)):"},{"line_number":1582,"context_line":"            msg \u003d _(\"Access rules cannot be denied while the share, \""},{"line_number":1583,"context_line":"                    \"any of its replicas or migration copies does not \""},{"line_number":1584,"context_line":"                    \"have a valid host or is in an invalid state.\")"},{"line_number":1585,"context_line":"            raise exception.InvalidShare(message\u003dmsg)"},{"line_number":1586,"context_line":""},{"line_number":1587,"context_line":"        for share_instance in share.instances:"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_fd0ac177","line":1584,"range":{"start_line":1583,"start_character":61,"end_line":1584,"end_character":25},"updated":"2017-01-18 17:37:43.000000000","message":"ditto","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0d6fd79d3868c7c0b4a2f18d8d4aedf3a75c35ed","unresolved":false,"context_lines":[{"line_number":1580,"context_line":"        if any(instance for instance in share.instances if"},{"line_number":1581,"context_line":"               self._is_invalid_share_instance(instance)):"},{"line_number":1582,"context_line":"            msg \u003d _(\"Access rules cannot be denied while the share, \""},{"line_number":1583,"context_line":"                    \"any of its replicas or migration copies does not \""},{"line_number":1584,"context_line":"                    \"have a valid host or is in an invalid state.\")"},{"line_number":1585,"context_line":"            raise exception.InvalidShare(message\u003dmsg)"},{"line_number":1586,"context_line":""},{"line_number":1587,"context_line":"        for share_instance in share.instances:"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_99b8eac9","line":1584,"range":{"start_line":1583,"start_character":61,"end_line":1584,"end_character":25},"in_reply_to":"7a3c09a3_fd0ac177","updated":"2017-01-18 19:20:56.000000000","message":"thank you, will fix.","commit_id":"7dd71e345f8af1010a70503525e4eb6c95b95038"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"c7ee904f455bb0ab89605158af8c07ac4b3bcb68","unresolved":false,"context_lines":[{"line_number":1524,"context_line":"    def allow_access(self, ctx, share, access_type, access_to,"},{"line_number":1525,"context_line":"                     access_level\u003dNone):"},{"line_number":1526,"context_line":"        \"\"\"Allow access to share.\"\"\""},{"line_number":1527,"context_line":"        share \u003d self.db.share_get(ctx, share[\u0027id\u0027])"},{"line_number":1528,"context_line":"        share_access_list \u003d self.db.share_access_get_all_by_type_and_access("},{"line_number":1529,"context_line":"            ctx, share[\u0027id\u0027], access_type, access_to)"},{"line_number":1530,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_c9b452d6","line":1527,"range":{"start_line":1527,"start_character":8,"end_line":1527,"end_character":51},"updated":"2017-01-18 22:41:55.000000000","message":"We just got the share at api/v1/shares.py line 419 via a share_api.get()  Any reason to do a fresh DB fetch here rather than just using that for share[\u0027id\u0027] and for share.instances ?  (Implicit here is an observation that the critical section around share fetch and check - which will be addressed as a separate effort - starts earlier than I indicated in my previous remarks since we are checking conditions based on fetching the share (and its instances back in ap1/v1/shares.py).","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"fc79585237dbe39a7a9b26f3b69eec73d71648fc","unresolved":false,"context_lines":[{"line_number":1524,"context_line":"    def allow_access(self, ctx, share, access_type, access_to,"},{"line_number":1525,"context_line":"                     access_level\u003dNone):"},{"line_number":1526,"context_line":"        \"\"\"Allow access to share.\"\"\""},{"line_number":1527,"context_line":"        share \u003d self.db.share_get(ctx, share[\u0027id\u0027])"},{"line_number":1528,"context_line":"        share_access_list \u003d self.db.share_access_get_all_by_type_and_access("},{"line_number":1529,"context_line":"            ctx, share[\u0027id\u0027], access_type, access_to)"},{"line_number":1530,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_bfbad508","line":1527,"range":{"start_line":1527,"start_character":8,"end_line":1527,"end_character":51},"in_reply_to":"7a3c09a3_c9b452d6","updated":"2017-01-18 23:05:57.000000000","message":"We should expect the tiniest of windows between v1/shares.py and here.. We don\u0027t need to refresh the share model in that window. Let me get rid of this","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"c7ee904f455bb0ab89605158af8c07ac4b3bcb68","unresolved":false,"context_lines":[{"line_number":1550,"context_line":"            \u0027access_level\u0027: access_level,"},{"line_number":1551,"context_line":"        }"},{"line_number":1552,"context_line":""},{"line_number":1553,"context_line":"        access \u003d self.db.share_access_create(ctx, values)"},{"line_number":1554,"context_line":""},{"line_number":1555,"context_line":"        for share_instance in share.instances:"},{"line_number":1556,"context_line":"            self.allow_access_to_instance(ctx, share_instance)"},{"line_number":1557,"context_line":""},{"line_number":1558,"context_line":"        # NOTE(tpsilva): refresh share_access model"},{"line_number":1559,"context_line":"        access \u003d self.db.share_access_get(ctx, access[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_1f2a0c2d","line":1556,"range":{"start_line":1553,"start_character":1,"end_line":1556,"end_character":62},"updated":"2017-01-18 22:41:55.000000000","message":"I\u0027m intrigued by the fact that db.share_access_create() is under the covers iterating over all instances and setting up an access map for each, right before we begin our own higher level iteration over share instances to update their access rule\u0027s status.  Not sure what to say about it at the moment, but for me it\u0027s food for thought in the conversation I\u0027m having with you and myself about the share-\u003eshare_instance abstraction and its implications for understanding our general concurrency problems.  It\u0027s easy for a developer not to see that the first iteration is going on.","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"fc79585237dbe39a7a9b26f3b69eec73d71648fc","unresolved":false,"context_lines":[{"line_number":1550,"context_line":"            \u0027access_level\u0027: access_level,"},{"line_number":1551,"context_line":"        }"},{"line_number":1552,"context_line":""},{"line_number":1553,"context_line":"        access \u003d self.db.share_access_create(ctx, values)"},{"line_number":1554,"context_line":""},{"line_number":1555,"context_line":"        for share_instance in share.instances:"},{"line_number":1556,"context_line":"            self.allow_access_to_instance(ctx, share_instance)"},{"line_number":1557,"context_line":""},{"line_number":1558,"context_line":"        # NOTE(tpsilva): refresh share_access model"},{"line_number":1559,"context_line":"        access \u003d self.db.share_access_get(ctx, access[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_4224b4ae","line":1556,"range":{"start_line":1553,"start_character":1,"end_line":1556,"end_character":62},"in_reply_to":"7a3c09a3_1f2a0c2d","updated":"2017-01-18 23:05:57.000000000","message":"It\u0027s by design to keep the slightest of windows between access create calls for all instances of a given share. We don\u0027t want to take that out because you can start here by having 5 instances of a given share and returning without creating the rule mapping for all of them in one database transaction is bound to cause errors elsewhere... \n\nAs noted in our spec and irc/summit discussion (possibly between us), access_rules_status can become a automatically derived field in the future, however, we maintain control over unpredictable state changes by explicitly updating this field ourselves. You will notice that we don\u0027t rely on this field to be in a particular state while making rule changes apart from in the data copy service where:\n\nwe make rule modifications and set the access_rules_status to \u0027syncing\u0027 and monitor the access_rules_status until it transitions to \u0027active\u0027. This is the behavior we expect applications to implement.. I don\u0027t yet feel comfortable about automatically deriving this field. Let\u0027s give this some time to settle and test this thoroughly before we decide to make that change.","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"c7ee904f455bb0ab89605158af8c07ac4b3bcb68","unresolved":false,"context_lines":[{"line_number":1556,"context_line":"            self.allow_access_to_instance(ctx, share_instance)"},{"line_number":1557,"context_line":""},{"line_number":1558,"context_line":"        # NOTE(tpsilva): refresh share_access model"},{"line_number":1559,"context_line":"        access \u003d self.db.share_access_get(ctx, access[\u0027id\u0027])"},{"line_number":1560,"context_line":""},{"line_number":1561,"context_line":"        return access"},{"line_number":1562,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_3adc3674","line":1559,"range":{"start_line":1559,"start_character":7,"end_line":1559,"end_character":60},"updated":"2017-01-18 22:41:55.000000000","message":"We just got the share access model returned to us at line 1533.  Did we really do anything that made it stale so that we need to do a fresh get here?  (I could well be missing something but I\u0027m not seeing it right off.)  We\u0027re just passing the access model up to the view builder anyways, so if somehow there was a change elsewhere in the system due to somebody responding very quickly to the rpc casts triggered by share_rpcapi.update_access (perhaps in combination with this DB access being very slow for some reason) I don\u0027t think we\u0027d want to show those as are result of this api call anyways.","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"fc79585237dbe39a7a9b26f3b69eec73d71648fc","unresolved":false,"context_lines":[{"line_number":1556,"context_line":"            self.allow_access_to_instance(ctx, share_instance)"},{"line_number":1557,"context_line":""},{"line_number":1558,"context_line":"        # NOTE(tpsilva): refresh share_access model"},{"line_number":1559,"context_line":"        access \u003d self.db.share_access_get(ctx, access[\u0027id\u0027])"},{"line_number":1560,"context_line":""},{"line_number":1561,"context_line":"        return access"},{"line_number":1562,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_3f2a652d","line":1559,"range":{"start_line":1559,"start_character":7,"end_line":1559,"end_character":60},"in_reply_to":"7a3c09a3_3adc3674","updated":"2017-01-18 23:05:57.000000000","message":"access_create returns only with the share data, does not set the instance access mapping data.","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"87484e6f5aba5ac7b059547d48482688c76419fc","unresolved":false,"context_lines":[{"line_number":1556,"context_line":"            self.allow_access_to_instance(ctx, share_instance)"},{"line_number":1557,"context_line":""},{"line_number":1558,"context_line":"        # NOTE(tpsilva): refresh share_access model"},{"line_number":1559,"context_line":"        access \u003d self.db.share_access_get(ctx, access[\u0027id\u0027])"},{"line_number":1560,"context_line":""},{"line_number":1561,"context_line":"        return access"},{"line_number":1562,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_d0ceaeb6","line":1559,"range":{"start_line":1559,"start_character":7,"end_line":1559,"end_character":60},"in_reply_to":"7a3c09a3_3f2a652d","updated":"2017-01-18 23:52:59.000000000","message":"As you mentioned on irc, this comment was added in Iff1ec2e3176a46e9f6bd383b38ffc5d838aa8bb8 and with your changes the extra DB get may not be necessary.  As I said there, extra DB gets (1) give false security that we\u0027re avoiding races, and (2) are also a potential source of large and unpredictable delays that can exacerbate race windows on distributed systems.","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ef35103bb877ed70dde3b3ba07a0a2129aa92128","unresolved":false,"context_lines":[{"line_number":1556,"context_line":"            self.allow_access_to_instance(ctx, share_instance)"},{"line_number":1557,"context_line":""},{"line_number":1558,"context_line":"        # NOTE(tpsilva): refresh share_access model"},{"line_number":1559,"context_line":"        access \u003d self.db.share_access_get(ctx, access[\u0027id\u0027])"},{"line_number":1560,"context_line":""},{"line_number":1561,"context_line":"        return access"},{"line_number":1562,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_109e7607","line":1559,"range":{"start_line":1559,"start_character":7,"end_line":1559,"end_character":60},"in_reply_to":"7a3c09a3_d0ceaeb6","updated":"2017-01-19 00:11:47.000000000","message":"Yes, for the record, my reply here was invalid. \n\nThe refresh was probably necessary when the \u0027state\u0027 field did not exist as a column in share instance access mapping and in transitive sense, tpsilva probably wanted to allow the @property state to get computed correctly.","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"c7ee904f455bb0ab89605158af8c07ac4b3bcb68","unresolved":false,"context_lines":[{"line_number":1576,"context_line":""},{"line_number":1577,"context_line":"    def deny_access(self, ctx, share, access):"},{"line_number":1578,"context_line":"        \"\"\"Deny access to share.\"\"\""},{"line_number":1579,"context_line":""},{"line_number":1580,"context_line":"        if any(instance for instance in share.instances if"},{"line_number":1581,"context_line":"               self._is_invalid_share_instance(instance)):"},{"line_number":1582,"context_line":"            msg \u003d _(\"Access rules cannot be denied while the share, \""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_ba5ca6f1","line":1579,"updated":"2017-01-18 22:41:55.000000000","message":"Referring back to my note on line 1527, note that we\u0027re happy to just use the share as fetched back in api/v1/shares.py here without refreshing from the database again.","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"fc79585237dbe39a7a9b26f3b69eec73d71648fc","unresolved":false,"context_lines":[{"line_number":1576,"context_line":""},{"line_number":1577,"context_line":"    def deny_access(self, ctx, share, access):"},{"line_number":1578,"context_line":"        \"\"\"Deny access to share.\"\"\""},{"line_number":1579,"context_line":""},{"line_number":1580,"context_line":"        if any(instance for instance in share.instances if"},{"line_number":1581,"context_line":"               self._is_invalid_share_instance(instance)):"},{"line_number":1582,"context_line":"            msg \u003d _(\"Access rules cannot be denied while the share, \""}],"source_content_type":"text/x-python","patch_set":24,"id":"7a3c09a3_1f25a95b","line":1579,"in_reply_to":"7a3c09a3_ba5ca6f1","updated":"2017-01-18 23:05:57.000000000","message":"+1","commit_id":"5d7a14c7b1194de7462eda8ec41ce67cd85a44d9"}],"manila/share/driver.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"48452cbddd7970a765d038a5fde497d5f6d4ec5a","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"            An access rule can be set to ``error`` state, either explicitly"},{"line_number":667,"context_line":"            via this return parameter or because of an exception raised in"},{"line_number":668,"context_line":"            this method. Such an access rule will no longer be sent to the"},{"line_number":669,"context_line":"            driver on subsequent access rule updates. When users deny that"},{"line_number":670,"context_line":"            rule, manila will purge it from its database. Logging errors in"},{"line_number":671,"context_line":"            the driver is good practice."},{"line_number":672,"context_line":"        \"\"\""},{"line_number":673,"context_line":"        raise NotImplementedError()"},{"line_number":674,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_36e05596","line":671,"range":{"start_line":668,"start_character":25,"end_line":671,"end_character":40},"updated":"2017-01-11 19:19:03.000000000","message":"question: user tries do add a rule, and it fails, driver returns state error or raises an exception (makes no difference for only 1 rule). Then, I understand that this rule will not be included in access_rules anymore, but it will be included in delete_rules when being delete? is the driver ever being invoked when denying an error-ed rule?","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0c81951e4e23a56d3ae0d1da8d8aeadc20845115","unresolved":false,"context_lines":[{"line_number":665,"context_line":""},{"line_number":666,"context_line":"            An access rule can be set to ``error`` state, either explicitly"},{"line_number":667,"context_line":"            via this return parameter or because of an exception raised in"},{"line_number":668,"context_line":"            this method. Such an access rule will no longer be sent to the"},{"line_number":669,"context_line":"            driver on subsequent access rule updates. When users deny that"},{"line_number":670,"context_line":"            rule, manila will purge it from its database. Logging errors in"},{"line_number":671,"context_line":"            the driver is good practice."},{"line_number":672,"context_line":"        \"\"\""},{"line_number":673,"context_line":"        raise NotImplementedError()"},{"line_number":674,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_78de4712","line":671,"range":{"start_line":668,"start_character":25,"end_line":671,"end_character":40},"in_reply_to":"ba5201f7_36e05596","updated":"2017-01-11 19:47:25.000000000","message":"no, the driver will never be called to deny access to an errored rule if the access_rules_status is set to \u0027error\u0027; and that will happen if any rules are in error state.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                    },"},{"line_number":654,"context_line":"            }"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        The top level keys are ``access_id``s that are the \u0027access_id\u0027"},{"line_number":657,"context_line":"        fields of the access rule that needs to be updated. ``access_key``s are"},{"line_number":658,"context_line":"        credentials (str) of the entity granted access. Any rule in the"},{"line_number":659,"context_line":"        ``access_rules`` parameter can be updated."}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_ffde790f","line":656,"range":{"start_line":656,"start_character":59,"end_line":656,"end_character":70},"updated":"2017-01-17 17:41:48.000000000","message":"sounds very redundant to read. I liked the old one better \u0027id\u0027, or just remove","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":653,"context_line":"                    },"},{"line_number":654,"context_line":"            }"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        The top level keys are ``access_id``s that are the \u0027access_id\u0027"},{"line_number":657,"context_line":"        fields of the access rule that needs to be updated. ``access_key``s are"},{"line_number":658,"context_line":"        credentials (str) of the entity granted access. Any rule in the"},{"line_number":659,"context_line":"        ``access_rules`` parameter can be updated."}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_cd5030d7","line":656,"range":{"start_line":656,"start_character":59,"end_line":656,"end_character":70},"in_reply_to":"7a3c09a3_ffde790f","updated":"2017-01-17 18:01:57.000000000","message":"This is necessary to clarify, because the \u0027id\u0027 field cannot be used to return values. i\u0027ll remove the emphasis on the first access_id.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":660,"context_line":""},{"line_number":661,"context_line":"        .. important::"},{"line_number":662,"context_line":""},{"line_number":663,"context_line":"            Raising an exception in this method will force *all* rules in"},{"line_number":664,"context_line":"            \u0027applying\u0027 and \u0027denying\u0027 states to \u0027error\u0027."},{"line_number":665,"context_line":""},{"line_number":666,"context_line":"            An access rule can be set to \u0027error\u0027 state, either explicitly"},{"line_number":667,"context_line":"            via this return parameter or because of an exception raised in"},{"line_number":668,"context_line":"            this method. Such an access rule will no longer be sent to the"},{"line_number":669,"context_line":"            driver on subsequent access rule updates. When users deny that"},{"line_number":670,"context_line":"            rule however, the driver will be asked to deny access to the"},{"line_number":671,"context_line":"            client/s represented by the rule. We expect that a"},{"line_number":672,"context_line":"            rule that was error-ed at the driver should never exist on the"},{"line_number":673,"context_line":"            back end. So, do not fail the deletion request."},{"line_number":674,"context_line":""},{"line_number":675,"context_line":"            Also, it is possible that the driver may receive a request to"},{"line_number":676,"context_line":"            add a rule that is already present on the back end."},{"line_number":677,"context_line":"            This can happen if the share manager service goes down"},{"line_number":678,"context_line":"            while the driver is committing access rule changes. Since we"},{"line_number":679,"context_line":"            cannot determine if the rule was applied successfully by the driver"},{"line_number":680,"context_line":"            before the disruption, we will treat all \u0027applying\u0027 transitional"},{"line_number":681,"context_line":"            rules as new rules and repeat the request."},{"line_number":682,"context_line":"        \"\"\""},{"line_number":683,"context_line":"        raise NotImplementedError()"},{"line_number":684,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_1f32655f","line":681,"range":{"start_line":663,"start_character":12,"end_line":681,"end_character":54},"updated":"2017-01-17 17:41:48.000000000","message":"awesome","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"}],"manila/share/drivers/cephfs/cephfs_native.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":289,"context_line":"        # backend are in sync."},{"line_number":290,"context_line":"        for rule in add_rules:"},{"line_number":291,"context_line":"            access_key \u003d self._allow_access(context, share, rule)"},{"line_number":292,"context_line":"            access_keys.update({rule[\u0027access_id\u0027]: {\u0027access_key\u0027: access_key}})"},{"line_number":293,"context_line":""},{"line_number":294,"context_line":"        for rule in delete_rules:"},{"line_number":295,"context_line":"            self._deny_access(context, share, rule)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_7fb28995","line":292,"range":{"start_line":292,"start_character":38,"end_line":292,"end_character":47},"updated":"2017-01-17 17:41:48.000000000","message":"see comment in driver.py","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"}],"manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"48452cbddd7970a765d038a5fde497d5f6d4ec5a","unresolved":false,"context_lines":[{"line_number":1471,"context_line":"            new_active_replica[\u0027access_rules_status\u0027] \u003d ("},{"line_number":1472,"context_line":"                constants.SHARE_INSTANCE_RULES_SYNCING)"},{"line_number":1473,"context_line":"        else:"},{"line_number":1474,"context_line":"            new_active_replica[\u0027access_rules_status\u0027] \u003d ("},{"line_number":1475,"context_line":"                constants.STATUS_ACTIVE)"},{"line_number":1476,"context_line":""},{"line_number":1477,"context_line":"        new_active_replica[\u0027export_locations\u0027] \u003d self._create_export("},{"line_number":1478,"context_line":"            new_active_replica, share_server, vserver, vserver_client)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_368e95d0","line":1475,"range":{"start_line":1474,"start_character":56,"end_line":1475,"end_character":40},"updated":"2017-01-11 19:19:03.000000000","message":"unrelated change","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0c81951e4e23a56d3ae0d1da8d8aeadc20845115","unresolved":false,"context_lines":[{"line_number":1471,"context_line":"            new_active_replica[\u0027access_rules_status\u0027] \u003d ("},{"line_number":1472,"context_line":"                constants.SHARE_INSTANCE_RULES_SYNCING)"},{"line_number":1473,"context_line":"        else:"},{"line_number":1474,"context_line":"            new_active_replica[\u0027access_rules_status\u0027] \u003d ("},{"line_number":1475,"context_line":"                constants.STATUS_ACTIVE)"},{"line_number":1476,"context_line":""},{"line_number":1477,"context_line":"        new_active_replica[\u0027export_locations\u0027] \u003d self._create_export("},{"line_number":1478,"context_line":"            new_active_replica, share_server, vserver, vserver_client)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_b8326f5c","line":1475,"range":{"start_line":1474,"start_character":56,"end_line":1475,"end_character":40},"in_reply_to":"ba5201f7_368e95d0","updated":"2017-01-11 19:47:25.000000000","message":"+1 will revert. The one one 1472 isn\u0027t though..","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":1471,"context_line":"            new_active_replica[\u0027access_rules_status\u0027] \u003d ("},{"line_number":1472,"context_line":"                constants.SHARE_INSTANCE_RULES_SYNCING)"},{"line_number":1473,"context_line":"        else:"},{"line_number":1474,"context_line":"            new_active_replica[\u0027access_rules_status\u0027] \u003d constants.STATUS_ACTIVE"},{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":"        new_active_replica[\u0027export_locations\u0027] \u003d self._create_export("},{"line_number":1477,"context_line":"            new_active_replica, share_server, vserver, vserver_client)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_4a07c917","line":1474,"range":{"start_line":1474,"start_character":11,"end_line":1474,"end_character":79},"updated":"2017-01-17 17:41:48.000000000","message":"Oh I see now... it is a terrible choice between OCD and \"not doing unrelated changes\". I guess the latter should be prevalent","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":1471,"context_line":"            new_active_replica[\u0027access_rules_status\u0027] \u003d ("},{"line_number":1472,"context_line":"                constants.SHARE_INSTANCE_RULES_SYNCING)"},{"line_number":1473,"context_line":"        else:"},{"line_number":1474,"context_line":"            new_active_replica[\u0027access_rules_status\u0027] \u003d constants.STATUS_ACTIVE"},{"line_number":1475,"context_line":""},{"line_number":1476,"context_line":"        new_active_replica[\u0027export_locations\u0027] \u003d self._create_export("},{"line_number":1477,"context_line":"            new_active_replica, share_server, vserver, vserver_client)"}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_4d7fe065","line":1474,"range":{"start_line":1474,"start_character":11,"end_line":1474,"end_character":79},"in_reply_to":"7a3c09a3_4a07c917","updated":"2017-01-17 18:01:57.000000000","message":"I do suffer from OCD. i\u0027m gritting my teeth about leaving STATUS_ACTIVE as is.","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"}],"manila/share/manager.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"834295b0ca82356b35f0ef7cf910cae7e2db325a","unresolved":false,"context_lines":[{"line_number":2560,"context_line":"        share_instance \u003d self._get_share_instance(context, share_instance_id)"},{"line_number":2561,"context_line":"        share_server \u003d self._get_share_server(context, share_instance)"},{"line_number":2562,"context_line":""},{"line_number":2563,"context_line":"        self.access_helper.update_access_rules("},{"line_number":2564,"context_line":"            context,"},{"line_number":2565,"context_line":"            share_instance_id,"},{"line_number":2566,"context_line":"            share_server\u003dshare_server)"},{"line_number":2567,"context_line":""},{"line_number":2568,"context_line":"    @add_hooks"},{"line_number":2569,"context_line":"    @utils.require_driver_initialized"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_0a355845","line":2566,"range":{"start_line":2563,"start_character":8,"end_line":2566,"end_character":38},"updated":"2016-09-13 23:50:21.000000000","message":"if you are running update_access for every request, I believe there is no way to get rid of the lock","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"fcba637643667a36d2a37aa4dfd83f3ab693e7e0","unresolved":false,"context_lines":[{"line_number":2560,"context_line":"        share_instance \u003d self._get_share_instance(context, share_instance_id)"},{"line_number":2561,"context_line":"        share_server \u003d self._get_share_server(context, share_instance)"},{"line_number":2562,"context_line":""},{"line_number":2563,"context_line":"        self.access_helper.update_access_rules("},{"line_number":2564,"context_line":"            context,"},{"line_number":2565,"context_line":"            share_instance_id,"},{"line_number":2566,"context_line":"            share_server\u003dshare_server)"},{"line_number":2567,"context_line":""},{"line_number":2568,"context_line":"    @add_hooks"},{"line_number":2569,"context_line":"    @utils.require_driver_initialized"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_0f656744","line":2566,"range":{"start_line":2563,"start_character":8,"end_line":2566,"end_character":38},"in_reply_to":"7a8ec9b2_0a355845","updated":"2016-09-14 01:22:23.000000000","message":"I just tested this on HNAS driver. At first it seemed to fix the problem described in the bug entry. All the 100 ip rules were successfully applied. The problem is: the first requested applied all 100 rules through the refresh mechanism from the first request. But then, all the other 99 requests reapplied the rules (most likely through recovery mode because they would retrieve no rules with \u0027NEW\u0027 status, so \u0027add_rules\u0027 would be empty, and my driver is always in recovery mode anyway, but for other drivers this would be terrible, aka remove everything and add back again) once the lock was released, so I kept watching it invoke the driver\u0027s update_access 99 additional times in m-shr log to do basically nothing. So I guess there is still the lock vs refresh to be adjusted.","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"40955d219051750bb982fc6d078dbad570462615","unresolved":false,"context_lines":[{"line_number":2560,"context_line":"        share_instance \u003d self._get_share_instance(context, share_instance_id)"},{"line_number":2561,"context_line":"        share_server \u003d self._get_share_server(context, share_instance)"},{"line_number":2562,"context_line":""},{"line_number":2563,"context_line":"        self.access_helper.update_access_rules("},{"line_number":2564,"context_line":"            context,"},{"line_number":2565,"context_line":"            share_instance_id,"},{"line_number":2566,"context_line":"            share_server\u003dshare_server)"},{"line_number":2567,"context_line":""},{"line_number":2568,"context_line":"    @add_hooks"},{"line_number":2569,"context_line":"    @utils.require_driver_initialized"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_ee76603d","line":2566,"range":{"start_line":2563,"start_character":8,"end_line":2566,"end_character":38},"in_reply_to":"7a8ec9b2_0f656744","updated":"2016-09-14 12:27:10.000000000","message":"No driver should remove everything and add back; i.e, cause a temporary disruption in IO; Which driver is currently doing this?","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"95ab0fa3dca0c3754940ee71606c628d06bec5d9","unresolved":false,"context_lines":[{"line_number":2560,"context_line":"        share_instance \u003d self._get_share_instance(context, share_instance_id)"},{"line_number":2561,"context_line":"        share_server \u003d self._get_share_server(context, share_instance)"},{"line_number":2562,"context_line":""},{"line_number":2563,"context_line":"        self.access_helper.update_access_rules("},{"line_number":2564,"context_line":"            context,"},{"line_number":2565,"context_line":"            share_instance_id,"},{"line_number":2566,"context_line":"            share_server\u003dshare_server)"},{"line_number":2567,"context_line":""},{"line_number":2568,"context_line":"    @add_hooks"},{"line_number":2569,"context_line":"    @utils.require_driver_initialized"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_36afb095","line":2566,"range":{"start_line":2563,"start_character":8,"end_line":2566,"end_character":38},"in_reply_to":"7a8ec9b2_1c82d894","updated":"2016-09-14 16:27:33.000000000","message":"I am thinking of a few possible ways to fix this:\n\n1) have the manager decide to not invoke the driver if all rules are active, and there are no rules to remove. This possibly breaks migration as it currently is, unless we fix it by changing all rules of the given instance to \u0027new\u0027 when migration wants to apply the rules. I am not sure if there is a scenario where we would not be invoking the driver while we actually should.\n\n1.5) I liked your change that removed the add_rules parameter from access_allow. Previously when we had locks, we also had problems when API requests changing order, because they were carrying the objects they wanted to add/remove. This is not a problem anymore because we have locks, but since you already removed the rules to be added, if we add the status \"deleting\" and get rid of the delete_rules parameter, we could achieve the behavior where update_access is simply a refresh that is triggered and reads the database to decide what to do.\n\n2) Maybe we could just convert this to a periodic task? This will fix the 99 executions of update_access but will introduce some delay to apply the rules. This will be a producer-consumer behavior.\n\n2.5) If we go with #2, it is possible that we don\u0027t need locks anymore, since we will be also removing parallelism.\n\nWhat do you think?","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"e6d143838d6f8e496972a7dce1f97ef883375ff7","unresolved":false,"context_lines":[{"line_number":2560,"context_line":"        share_instance \u003d self._get_share_instance(context, share_instance_id)"},{"line_number":2561,"context_line":"        share_server \u003d self._get_share_server(context, share_instance)"},{"line_number":2562,"context_line":""},{"line_number":2563,"context_line":"        self.access_helper.update_access_rules("},{"line_number":2564,"context_line":"            context,"},{"line_number":2565,"context_line":"            share_instance_id,"},{"line_number":2566,"context_line":"            share_server\u003dshare_server)"},{"line_number":2567,"context_line":""},{"line_number":2568,"context_line":"    @add_hooks"},{"line_number":2569,"context_line":"    @utils.require_driver_initialized"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a8bc5a2_c7764405","line":2566,"range":{"start_line":2563,"start_character":8,"end_line":2566,"end_character":38},"in_reply_to":"7a8ec9b2_36afb095","updated":"2016-10-03 15:00:10.000000000","message":"As discussed, changes include:\n\n* removing allow and deny access in favor of update_access\n* Handling the concurrency in the access helper by keeping tracking of share instances being updated;\n* Invoking the driver in a loop based off \u0027state\u0027 saved within the database\n* Allowing for a way to queue up any \u0027resync\u0027 requests -\u003e this is in case you need the periodic task that you mentioned. I think it will be useful in the following pattern:\n\n   * Ask update_access to resync rules\n   * If it returns, wait for share_instance\u0027s access_rules_status\n      to be sure that everything is in_sync.","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"e4ee3a5cb9180aa26fd3295eab329a7ec6a99436","unresolved":false,"context_lines":[{"line_number":2560,"context_line":"        share_instance \u003d self._get_share_instance(context, share_instance_id)"},{"line_number":2561,"context_line":"        share_server \u003d self._get_share_server(context, share_instance)"},{"line_number":2562,"context_line":""},{"line_number":2563,"context_line":"        self.access_helper.update_access_rules("},{"line_number":2564,"context_line":"            context,"},{"line_number":2565,"context_line":"            share_instance_id,"},{"line_number":2566,"context_line":"            share_server\u003dshare_server)"},{"line_number":2567,"context_line":""},{"line_number":2568,"context_line":"    @add_hooks"},{"line_number":2569,"context_line":"    @utils.require_driver_initialized"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a8ec9b2_1c82d894","line":2566,"range":{"start_line":2563,"start_character":8,"end_line":2566,"end_character":38},"in_reply_to":"7a8ec9b2_ee76603d","updated":"2016-09-14 15:50:26.000000000","message":"I don\u0027t remember which ones exactly. I have just taken a quick look, quobyte, tegile and huawei do this.","commit_id":"661350135822225e8e3ce855f807d66b1221f8a5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"48452cbddd7970a765d038a5fde497d5f6d4ec5a","unresolved":false,"context_lines":[{"line_number":333,"context_line":"            if share_instance[\u0027access_rules_status\u0027] !\u003d ("},{"line_number":334,"context_line":"                    constants.STATUS_ACTIVE):"},{"line_number":335,"context_line":"                try:"},{"line_number":336,"context_line":"                    # Cast any existing \u0027applying\u0027 rules to \u0027new\u0027"},{"line_number":337,"context_line":"                    self.access_helper.reset_applying_rules("},{"line_number":338,"context_line":"                        ctxt, share_instance[\u0027id\u0027])"},{"line_number":339,"context_line":"                    self.access_helper.update_access_rules("},{"line_number":340,"context_line":"                        ctxt, share_instance[\u0027id\u0027], share_server\u003dshare_server)"},{"line_number":341,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_f13897d5","line":338,"range":{"start_line":336,"start_character":20,"end_line":338,"end_character":51},"updated":"2017-01-11 19:19:03.000000000","message":"I believe this may race with RPC requests being read from rabbit when restarting a service","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2307d66a51cdf13e911af61fe2482bccada0201e","unresolved":false,"context_lines":[{"line_number":333,"context_line":"            if share_instance[\u0027access_rules_status\u0027] !\u003d ("},{"line_number":334,"context_line":"                    constants.STATUS_ACTIVE):"},{"line_number":335,"context_line":"                try:"},{"line_number":336,"context_line":"                    # Cast any existing \u0027applying\u0027 rules to \u0027new\u0027"},{"line_number":337,"context_line":"                    self.access_helper.reset_applying_rules("},{"line_number":338,"context_line":"                        ctxt, share_instance[\u0027id\u0027])"},{"line_number":339,"context_line":"                    self.access_helper.update_access_rules("},{"line_number":340,"context_line":"                        ctxt, share_instance[\u0027id\u0027], share_server\u003dshare_server)"},{"line_number":341,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_dbb64df8","line":338,"range":{"start_line":336,"start_character":20,"end_line":338,"end_character":51},"in_reply_to":"ba5201f7_9b53e516","updated":"2017-01-11 19:54:55.000000000","message":"I am not sure :\\","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0c81951e4e23a56d3ae0d1da8d8aeadc20845115","unresolved":false,"context_lines":[{"line_number":333,"context_line":"            if share_instance[\u0027access_rules_status\u0027] !\u003d ("},{"line_number":334,"context_line":"                    constants.STATUS_ACTIVE):"},{"line_number":335,"context_line":"                try:"},{"line_number":336,"context_line":"                    # Cast any existing \u0027applying\u0027 rules to \u0027new\u0027"},{"line_number":337,"context_line":"                    self.access_helper.reset_applying_rules("},{"line_number":338,"context_line":"                        ctxt, share_instance[\u0027id\u0027])"},{"line_number":339,"context_line":"                    self.access_helper.update_access_rules("},{"line_number":340,"context_line":"                        ctxt, share_instance[\u0027id\u0027], share_server\u003dshare_server)"},{"line_number":341,"context_line":"                except Exception:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_9b53e516","line":338,"range":{"start_line":336,"start_character":20,"end_line":338,"end_character":51},"in_reply_to":"ba5201f7_f13897d5","updated":"2017-01-11 19:47:25.000000000","message":"Isn\u0027t the listener only fired off after init_host?","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"48452cbddd7970a765d038a5fde497d5f6d4ec5a","unresolved":false,"context_lines":[{"line_number":778,"context_line":""},{"line_number":779,"context_line":"    def _cast_access_rules_to_readonly(self, context, src_share_instance,"},{"line_number":780,"context_line":"                                       share_server):"},{"line_number":781,"context_line":"        # Set all \u0027applying\u0027 or \u0027active\u0027 rules to \u0027queued_to_apply\u0027. since the"},{"line_number":782,"context_line":"        # share has a status set to \u0027migrating\u0027, this will render rules"},{"line_number":783,"context_line":"        # read/only."},{"line_number":784,"context_line":"        acceptable_past_states \u003d (constants.ACCESS_STATE_APPLYING,"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_71d66750","line":781,"range":{"start_line":781,"start_character":69,"end_line":781,"end_character":70},"updated":"2017-01-11 19:19:03.000000000","message":"Uppercase","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"a5a3ae6002f3d8bca18808bb8c03338bc6c9f54e","unresolved":false,"context_lines":[{"line_number":795,"context_line":"            context, src_share_instance[\u0027id\u0027],"},{"line_number":796,"context_line":"            share_server\u003dshare_server)"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"        utils.wait_for_access_update("},{"line_number":799,"context_line":"            context, self.db, src_share_instance,"},{"line_number":800,"context_line":"            self.migration_wait_access_rules_timeout)"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"    @periodic_task.periodic_task("},{"line_number":803,"context_line":"        spacing\u003dCONF.migration_driver_continue_update_interval)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_15c98459","line":800,"range":{"start_line":798,"start_character":9,"end_line":800,"end_character":53},"updated":"2017-01-11 19:18:56.000000000","message":"isn\u0027t the above call supposed to be synchronous?","commit_id":"fb4f814778941c2d52d40477fc680234580eaff8"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"7287285b87809f15187189ac361ded175713fbb5","unresolved":false,"context_lines":[{"line_number":795,"context_line":"            context, src_share_instance[\u0027id\u0027],"},{"line_number":796,"context_line":"            share_server\u003dshare_server)"},{"line_number":797,"context_line":""},{"line_number":798,"context_line":"        utils.wait_for_access_update("},{"line_number":799,"context_line":"            context, self.db, src_share_instance,"},{"line_number":800,"context_line":"            self.migration_wait_access_rules_timeout)"},{"line_number":801,"context_line":""},{"line_number":802,"context_line":"    @periodic_task.periodic_task("},{"line_number":803,"context_line":"        spacing\u003dCONF.migration_driver_continue_update_interval)"}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_38491f61","line":800,"range":{"start_line":798,"start_character":9,"end_line":800,"end_character":53},"in_reply_to":"ba5201f7_15c98459","updated":"2017-01-11 19:26:32.000000000","message":"It won\u0027t be if the above call hits the driver right away, but don\u0027t you want to be sure that there were no access rules in flight?","commit_id":"fb4f814778941c2d52d40477fc680234580eaff8"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"89b852fa4cea760371d785dd0c27690650b327d9","unresolved":false,"context_lines":[{"line_number":973,"context_line":"                    \"backend during migration of share %s.\") % share[\u0027id\u0027]"},{"line_number":974,"context_line":"            LOG.exception(msg)"},{"line_number":975,"context_line":"            helper.cleanup_access_rules(src_share_instance, share_server,"},{"line_number":976,"context_line":"                                        self.access_helper)"},{"line_number":977,"context_line":"            raise exception.ShareMigrationFailed(reason\u003dmsg)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"        ignore_list \u003d self.driver.configuration.safe_get("}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_aa53cd69","line":976,"range":{"start_line":976,"start_character":40,"end_line":976,"end_character":58},"updated":"2017-01-17 17:41:48.000000000","message":"wasn\u0027t it going to be passed in the constructor? I think it would be cleaner, the helper instance is associated with this method, so it still makes sense","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"dcb220d3faa373b663b6cc5f067615e46ddc90ae","unresolved":false,"context_lines":[{"line_number":973,"context_line":"                    \"backend during migration of share %s.\") % share[\u0027id\u0027]"},{"line_number":974,"context_line":"            LOG.exception(msg)"},{"line_number":975,"context_line":"            helper.cleanup_access_rules(src_share_instance, share_server,"},{"line_number":976,"context_line":"                                        self.access_helper)"},{"line_number":977,"context_line":"            raise exception.ShareMigrationFailed(reason\u003dmsg)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"        ignore_list \u003d self.driver.configuration.safe_get("}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_90d0fff7","line":976,"range":{"start_line":976,"start_character":40,"end_line":976,"end_character":58},"in_reply_to":"7a3c09a3_108faf2a","updated":"2017-01-17 18:17:18.000000000","message":"all the invocations will be much cleaner. Performance and Memory-usage wise it should not make any difference, as only the reference is copied","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"ed0accd77a11052501e6f30683ea216dca47d8b3","unresolved":false,"context_lines":[{"line_number":973,"context_line":"                    \"backend during migration of share %s.\") % share[\u0027id\u0027]"},{"line_number":974,"context_line":"            LOG.exception(msg)"},{"line_number":975,"context_line":"            helper.cleanup_access_rules(src_share_instance, share_server,"},{"line_number":976,"context_line":"                                        self.access_helper)"},{"line_number":977,"context_line":"            raise exception.ShareMigrationFailed(reason\u003dmsg)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"        ignore_list \u003d self.driver.configuration.safe_get("}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_263f1188","line":976,"range":{"start_line":976,"start_character":40,"end_line":976,"end_character":58},"in_reply_to":"7a3c09a3_90d0fff7","updated":"2017-01-17 23:52:48.000000000","message":"Done","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"091281243db96b936b88105183d7182db39e9a12","unresolved":false,"context_lines":[{"line_number":973,"context_line":"                    \"backend during migration of share %s.\") % share[\u0027id\u0027]"},{"line_number":974,"context_line":"            LOG.exception(msg)"},{"line_number":975,"context_line":"            helper.cleanup_access_rules(src_share_instance, share_server,"},{"line_number":976,"context_line":"                                        self.access_helper)"},{"line_number":977,"context_line":"            raise exception.ShareMigrationFailed(reason\u003dmsg)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"        ignore_list \u003d self.driver.configuration.safe_get("}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_cdb4b036","line":976,"range":{"start_line":976,"start_character":40,"end_line":976,"end_character":58},"in_reply_to":"7a3c09a3_aa53cd69","updated":"2017-01-17 18:01:57.000000000","message":"only one method is using it...","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"0916c8f87fd3ee7687e09fbb822faf480bc68c81","unresolved":false,"context_lines":[{"line_number":973,"context_line":"                    \"backend during migration of share %s.\") % share[\u0027id\u0027]"},{"line_number":974,"context_line":"            LOG.exception(msg)"},{"line_number":975,"context_line":"            helper.cleanup_access_rules(src_share_instance, share_server,"},{"line_number":976,"context_line":"                                        self.access_helper)"},{"line_number":977,"context_line":"            raise exception.ShareMigrationFailed(reason\u003dmsg)"},{"line_number":978,"context_line":""},{"line_number":979,"context_line":"        ignore_list \u003d self.driver.configuration.safe_get("}],"source_content_type":"text/x-python","patch_set":18,"id":"7a3c09a3_108faf2a","line":976,"range":{"start_line":976,"start_character":40,"end_line":976,"end_character":58},"in_reply_to":"7a3c09a3_cdb4b036","updated":"2017-01-17 18:15:09.000000000","message":"so?","commit_id":"2bcd5fdd1c9245bbec9c9a7155434217a7dbe4e5"},{"author":{"_account_id":9003,"name":"Tom Barron","email":"tpb@dyncloud.net","username":"tbarron"},"change_message_id":"06db914e0d1a52fffee5c0206763435b5357a2a2","unresolved":false,"context_lines":[{"line_number":2731,"context_line":""},{"line_number":2732,"context_line":"        self.access_helper.update_access_rules("},{"line_number":2733,"context_line":"            context,"},{"line_number":2734,"context_line":"            share_instance_id,"},{"line_number":2735,"context_line":"            share_server\u003dshare_server)"},{"line_number":2736,"context_line":""},{"line_number":2737,"context_line":"    @periodic_task.periodic_task(spacing\u003dCONF.periodic_interval)"}],"source_content_type":"text/x-python","patch_set":25,"id":"7a3c09a3_abfff3a8","line":2734,"range":{"start_line":2734,"start_character":12,"end_line":2734,"end_character":29},"updated":"2017-01-19 01:24:44.000000000","message":"If we instead pass the share_instance obtained at line 2726 we can avoid having to refetch the share instance at the start of access_helper.update_access_rules.\n\nupdate_access_rules() is invoked in a number of other places but it looks like they all fetch the share instance or replica right before calling this method too.","commit_id":"ea49b65f8076f3b585623b0bd224b9db162b54d1"}],"manila/share/migration.py":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"48452cbddd7970a765d038a5fde497d5f6d4ec5a","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        self.db.share_instance_access_copy(self.context, self.share[\u0027id\u0027],"},{"line_number":166,"context_line":"                                           new_share_instance[\u0027id\u0027])"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        self.api.allow_access_to_instance(self.context, new_share_instance)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        utils.wait_for_access_update("}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_154a6426","line":167,"updated":"2017-01-11 19:19:03.000000000","message":"I don\u0027t remember why but possibly there was a reason to refresh the share instance before invoking self.api.allow_access_to_instance(self.context, new_share_instance)","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"0c81951e4e23a56d3ae0d1da8d8aeadc20845115","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        self.db.share_instance_access_copy(self.context, self.share[\u0027id\u0027],"},{"line_number":166,"context_line":"                                           new_share_instance[\u0027id\u0027])"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        self.api.allow_access_to_instance(self.context, new_share_instance)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        utils.wait_for_access_update("}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_7beac110","line":167,"in_reply_to":"ba5201f7_154a6426","updated":"2017-01-11 19:47:25.000000000","message":"Not sure we need to.. \n\nallow_access_to_instance cares about one thing, the share_instance\u0027s ID.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"2307d66a51cdf13e911af61fe2482bccada0201e","unresolved":false,"context_lines":[{"line_number":164,"context_line":""},{"line_number":165,"context_line":"        self.db.share_instance_access_copy(self.context, self.share[\u0027id\u0027],"},{"line_number":166,"context_line":"                                           new_share_instance[\u0027id\u0027])"},{"line_number":167,"context_line":""},{"line_number":168,"context_line":"        self.api.allow_access_to_instance(self.context, new_share_instance)"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"        utils.wait_for_access_update("}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_9be3e5d8","line":167,"in_reply_to":"ba5201f7_7beac110","updated":"2017-01-11 19:54:55.000000000","message":"well it is not refreshing anywhere from here to update_access and it is checking access_rules_status in the \"_conditionally...\" method, but only if status is error, which is highly unlikely to be for this method invoked through here. This is ok, but I think that if we move that check to inside a locked method, we should refresh it there, unrelated to this method here.","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"},{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"a5a3ae6002f3d8bca18808bb8c03338bc6c9f54e","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        self.access_helper.update_access_rules("},{"line_number":161,"context_line":"            self.context, share_instance[\u0027id\u0027], share_server\u003dshare_server)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        utils.wait_for_access_update("},{"line_number":164,"context_line":"            self.context, self.db, share_instance,"},{"line_number":165,"context_line":"            self.migration_wait_access_rules_timeout)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def apply_new_access_rules(self, new_share_instance):"},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_7561f038","line":165,"range":{"start_line":163,"start_character":8,"end_line":165,"end_character":53},"updated":"2017-01-11 19:18:56.000000000","message":"same here, above call should be synchronous, why is this necessary?","commit_id":"fb4f814778941c2d52d40477fc680234580eaff8"},{"author":{"_account_id":16643,"name":"Goutham Pacha Ravi","email":"gouthampravi@gmail.com","username":"gouthamr"},"change_message_id":"7287285b87809f15187189ac361ded175713fbb5","unresolved":false,"context_lines":[{"line_number":160,"context_line":"        self.access_helper.update_access_rules("},{"line_number":161,"context_line":"            self.context, share_instance[\u0027id\u0027], share_server\u003dshare_server)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        utils.wait_for_access_update("},{"line_number":164,"context_line":"            self.context, self.db, share_instance,"},{"line_number":165,"context_line":"            self.migration_wait_access_rules_timeout)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":"    def apply_new_access_rules(self, new_share_instance):"},{"line_number":168,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"ba5201f7_987a0b87","line":165,"range":{"start_line":163,"start_character":8,"end_line":165,"end_character":53},"in_reply_to":"ba5201f7_7561f038","updated":"2017-01-11 19:26:32.000000000","message":"same","commit_id":"fb4f814778941c2d52d40477fc680234580eaff8"}],"releasenotes/notes/bug-1626249-reintroduce-per-share-instance-access-rule-state-7c08a91373b21557.yaml":[{"author":{"_account_id":14567,"name":"Rodrigo Barbieri","email":"rodrigo.barbieri2010@gmail.com","username":"ganso"},"change_message_id":"48452cbddd7970a765d038a5fde497d5f6d4ec5a","unresolved":false,"context_lines":[{"line_number":3,"context_line":"  - New micro-states (\u0027applying\u0027, \u0027denying\u0027), appear in the \u0027state\u0027 field"},{"line_number":4,"context_line":"    of access rules list API. These transitional states signify the state of"},{"line_number":5,"context_line":"    an access rule while its application or denial is being processed"},{"line_number":6,"context_line":"    asynchrously."},{"line_number":7,"context_line":"  - Access rules can be added regardless of the \u0027access_rules_status\u0027"},{"line_number":8,"context_line":"    of the share or any of its replicas."},{"line_number":9,"context_line":"fixes:"}],"source_content_type":"text/x-yaml","patch_set":8,"id":"ba5201f7_120b5a2b","line":6,"range":{"start_line":6,"start_character":4,"end_line":6,"end_character":16},"updated":"2017-01-11 19:19:03.000000000","message":"asynchronously","commit_id":"bf4929253df013e07ab9fde19ad03dbe764a536f"}]}