)]}'
{"openstack/network/v2/_proxy.py":[{"author":{"_account_id":8257,"name":"Brian Curtin","email":"brian@python.org","username":"briancurtin"},"change_message_id":"b6f664b072c03987b4d48f512d24e94589042bce","unresolved":false,"context_lines":[{"line_number":2930,"context_line":""},{"line_number":2931,"context_line":"    @staticmethod"},{"line_number":2932,"context_line":"    def _check_tag_support(resource):"},{"line_number":2933,"context_line":"        if not hasattr(resource, \u0027tags\u0027):"},{"line_number":2934,"context_line":"            raise exceptions.InvalidRequest("},{"line_number":2935,"context_line":"                \u0027%s resource does not support tag\u0027 %"},{"line_number":2936,"context_line":"                resource.__class__.__name__)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_2d9aa413","line":2933,"updated":"2017-05-05 16:05:09.000000000","message":"If we\u0027re looking for the absence or existence of an attribute, I\u0027d rather we do ``if not getattr(resource, \u0027tags\u0027, False)```. \n\nIt\u0027s probably unlikely that the usual issues with hasattr could affect us here, but I\u0027d rather be safe. See https://hynek.me/articles/hasattr/ for some details.","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b6cb922d5f772b8846914c7afb592b20df24a8da","unresolved":false,"context_lines":[{"line_number":2930,"context_line":""},{"line_number":2931,"context_line":"    @staticmethod"},{"line_number":2932,"context_line":"    def _check_tag_support(resource):"},{"line_number":2933,"context_line":"        if not hasattr(resource, \u0027tags\u0027):"},{"line_number":2934,"context_line":"            raise exceptions.InvalidRequest("},{"line_number":2935,"context_line":"                \u0027%s resource does not support tag\u0027 %"},{"line_number":2936,"context_line":"                resource.__class__.__name__)"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_63b9d4e7","line":2933,"in_reply_to":"3f044301_2d9aa413","updated":"2017-05-05 17:59:29.000000000","message":"Thanks for the useful information. I heard hasattr() should be avoided but I have no good source.\n\nIn this case, ``if not getattr(resource, \u0027tags\u0027, False)`` does not work because tags can be an empty list. In addition, there is a discussion whether None and an empty list should be handled equally in the bug in neutron. Currently the networking API returns only an empty list, but there are some cases where None is returned if nothing is specified. From this, I think I should catch AttributeError.","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"},{"author":{"_account_id":8257,"name":"Brian Curtin","email":"brian@python.org","username":"briancurtin"},"change_message_id":"b6f664b072c03987b4d48f512d24e94589042bce","unresolved":false,"context_lines":[{"line_number":2948,"context_line":"        self._check_tag_support(resource)"},{"line_number":2949,"context_line":"        return resource.add_tag(self._session, tag)"},{"line_number":2950,"context_line":""},{"line_number":2951,"context_line":"    def remove_tag(self, resource, tag\u003dNone, all\u003dFalse):"},{"line_number":2952,"context_line":"        \"\"\"Delete a tag from a specified resource"},{"line_number":2953,"context_line":""},{"line_number":2954,"context_line":"        :param resource:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_edbcbc6a","line":2951,"updated":"2017-05-05 16:05:09.000000000","message":"What if we made this remove_tags(resource, tags), and then if we get a list of one tag we call resource.remove_tag and if we have more than one we call resource.remove_all_tags?","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"384e2fdfd315076d3e559202ee04c5805c191b0e","unresolved":false,"context_lines":[{"line_number":2948,"context_line":"        self._check_tag_support(resource)"},{"line_number":2949,"context_line":"        return resource.add_tag(self._session, tag)"},{"line_number":2950,"context_line":""},{"line_number":2951,"context_line":"    def remove_tag(self, resource, tag\u003dNone, all\u003dFalse):"},{"line_number":2952,"context_line":"        \"\"\"Delete a tag from a specified resource"},{"line_number":2953,"context_line":""},{"line_number":2954,"context_line":"        :param resource:"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff0f0b1f_6f8d0a7a","line":2951,"in_reply_to":"3f044301_2323cc07","updated":"2017-05-21 06:04:38.000000000","message":"After thinking more, I started to think it is enough to provide only set_tags(). The API supports replace tags, add a tag, delete a tag, delete all tags, but we can do all operations by \"replace tags\" operation.\n\nIn addition, normal attributes only support \u0027replace\u0027 operation (PUT operation), so I think we can do the same thing for \u0027tags\u0027.","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b6cb922d5f772b8846914c7afb592b20df24a8da","unresolved":false,"context_lines":[{"line_number":2948,"context_line":"        self._check_tag_support(resource)"},{"line_number":2949,"context_line":"        return resource.add_tag(self._session, tag)"},{"line_number":2950,"context_line":""},{"line_number":2951,"context_line":"    def remove_tag(self, resource, tag\u003dNone, all\u003dFalse):"},{"line_number":2952,"context_line":"        \"\"\"Delete a tag from a specified resource"},{"line_number":2953,"context_line":""},{"line_number":2954,"context_line":"        :param resource:"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_2323cc07","line":2951,"in_reply_to":"3f044301_edbcbc6a","updated":"2017-05-05 17:59:29.000000000","message":"As you see the tags API definition [1], you will notice that the API supports removal of all tags or removal of one tag. We cannot remove multiple tags.\n\nWhen I wrote the code, I wondered what is a good approach as OpenStack SDK.\n\nThe current implementation is just a mirror of the tags API.\n\nOf course we can support removal of multiple tags. There are several options: (1) calling the API of deleting a single tag, or (2) fetch the current tags, calculate the new tags and set it as new tags.\n\nWhat is the good direction as implementation of SDK? I think it is related to the SDK concept and I haven\u0027t understood it well.\n\n[1] http://specs.openstack.org/openstack/api-wg/guidelines/tags.html#tags-resource-urls","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"}],"openstack/network/v2/tag.py":[{"author":{"_account_id":8257,"name":"Brian Curtin","email":"brian@python.org","username":"briancurtin"},"change_message_id":"b6f664b072c03987b4d48f512d24e94589042bce","unresolved":false,"context_lines":[{"line_number":16,"context_line":"class TagMixin(object):"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    _tag_query_parameters \u003d {"},{"line_number":19,"context_line":"        \u0027tags\u0027: \u0027tags\u0027,"},{"line_number":20,"context_line":"        \u0027tags_any\u0027: \u0027tags-any\u0027,"},{"line_number":21,"context_line":"        \u0027not_tags\u0027: \u0027not-tags\u0027,"},{"line_number":22,"context_line":"        \u0027not_tags_any\u0027: \u0027not-tags-any\u0027,"},{"line_number":23,"context_line":"    }"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def set_tags(self, session, tags):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_e32ccb48","line":22,"range":{"start_line":19,"start_character":8,"end_line":22,"end_character":39},"updated":"2017-05-05 16:05:09.000000000","message":"I\u0027ve seen these params before and they read very oddly. What if we changed the two any options to any_tags and not_any_tags? Does that still read properly with what\u0027s being done?","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"384e2fdfd315076d3e559202ee04c5805c191b0e","unresolved":false,"context_lines":[{"line_number":16,"context_line":"class TagMixin(object):"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    _tag_query_parameters \u003d {"},{"line_number":19,"context_line":"        \u0027tags\u0027: \u0027tags\u0027,"},{"line_number":20,"context_line":"        \u0027tags_any\u0027: \u0027tags-any\u0027,"},{"line_number":21,"context_line":"        \u0027not_tags\u0027: \u0027not-tags\u0027,"},{"line_number":22,"context_line":"        \u0027not_tags_any\u0027: \u0027not-tags-any\u0027,"},{"line_number":23,"context_line":"    }"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def set_tags(self, session, tags):"}],"source_content_type":"text/x-python","patch_set":6,"id":"ff0f0b1f_afc1e282","line":22,"range":{"start_line":19,"start_character":8,"end_line":22,"end_character":39},"in_reply_to":"3f044301_83eaa072","updated":"2017-05-21 06:04:38.000000000","message":"Done","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b6cb922d5f772b8846914c7afb592b20df24a8da","unresolved":false,"context_lines":[{"line_number":16,"context_line":"class TagMixin(object):"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"    _tag_query_parameters \u003d {"},{"line_number":19,"context_line":"        \u0027tags\u0027: \u0027tags\u0027,"},{"line_number":20,"context_line":"        \u0027tags_any\u0027: \u0027tags-any\u0027,"},{"line_number":21,"context_line":"        \u0027not_tags\u0027: \u0027not-tags\u0027,"},{"line_number":22,"context_line":"        \u0027not_tags_any\u0027: \u0027not-tags-any\u0027,"},{"line_number":23,"context_line":"    }"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def set_tags(self, session, tags):"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_83eaa072","line":22,"range":{"start_line":19,"start_character":8,"end_line":22,"end_character":39},"in_reply_to":"3f044301_e32ccb48","updated":"2017-05-05 17:59:29.000000000","message":"I am fine with \u0027any_tags\u0027 and \u0027not_any_tags\u0027. The only reason of the current choice comes from the API definition and I just replaces a hyphen to underscore to follow the convention of other query parameter.\n\nThese are just query parameters and they are not read properties.","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"},{"author":{"_account_id":8257,"name":"Brian Curtin","email":"brian@python.org","username":"briancurtin"},"change_message_id":"b6f664b072c03987b4d48f512d24e94589042bce","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        \u0027not_tags_any\u0027: \u0027not-tags-any\u0027,"},{"line_number":23,"context_line":"    }"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def set_tags(self, session, tags):"},{"line_number":26,"context_line":"        url \u003d utils.urljoin(self.base_path, self.id, \u0027tags\u0027)"},{"line_number":27,"context_line":"        session.put(url, endpoint_filter\u003dself.service, json\u003d{\u0027tags\u0027: tags})"},{"line_number":28,"context_line":"        self._body.attributes.update({\u0027tags\u0027: tags})"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_5d2597aa","line":25,"updated":"2017-05-05 16:05:09.000000000","message":"Operationally, what\u0027s the difference between the set and add calls? Can we get the job done with just one of them? \n\nIs there anything different between calling add_tag(\"blah\") and set_tags([\"blah\"]), where just one tag is being sent?","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"b6cb922d5f772b8846914c7afb592b20df24a8da","unresolved":false,"context_lines":[{"line_number":22,"context_line":"        \u0027not_tags_any\u0027: \u0027not-tags-any\u0027,"},{"line_number":23,"context_line":"    }"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    def set_tags(self, session, tags):"},{"line_number":26,"context_line":"        url \u003d utils.urljoin(self.base_path, self.id, \u0027tags\u0027)"},{"line_number":27,"context_line":"        session.put(url, endpoint_filter\u003dself.service, json\u003d{\u0027tags\u0027: tags})"},{"line_number":28,"context_line":"        self._body.attributes.update({\u0027tags\u0027: tags})"}],"source_content_type":"text/x-python","patch_set":6,"id":"3f044301_ee302beb","line":25,"in_reply_to":"3f044301_5d2597aa","updated":"2017-05-05 17:59:29.000000000","message":"The different is how the existing tags are handled. Assume a resource has tags [\u0027red\u0027, \u0027green\u0027]. add_tag(\u0027blue\u0027) results in tags [\u0027red\u0027, \u0027green\u0027, \u0027blue\u0027], but set_tags([\u0027blue\u0027]) replaces the existing tags and results in [\u0027blue\u0027].\n\nThese methods are just a mirror of the current API definition.\n\nWhen I wrote the code, I first tried to support tags as a normal property, i.e. something like network.tags and if an SDK user update network.tags the tag API is called to sync new tags with the API. This might be more reasonable from the perspective of SDK users. However, I cannot find a convenient way to do special handling of updated properties in openstack SDK so far. The current implementation is a kind of compromise (and hopefully we can explore better way to hide the detail of back-end APIs).","commit_id":"085a688f3497c2562e6aa9244e2863cde56a7122"}]}
