)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"63613a2d70ea12de264ae650554ef1f5bfaf4fcf","unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Michael Davies \u003cmichael@the-davies.net\u003e"},{"line_number":5,"context_line":"CommitDate: 2014-07-15 06:50:58 +0930"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Ironic nova driver to cache ironic client calls"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"When the nova ironic driver makes ironic client calls, it"},{"line_number":10,"context_line":"currently gets a brand new client for each call.  This patch"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"baada198_664ca277","line":7,"updated":"2014-07-16 23:41:51.000000000","message":"nit - its not call it is caching, its the client object itself. E.g.\n\n\"Cache Ironic client in the Nova driver\"","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"}],"ironic/nova/virt/ironic/client_wrapper.py":[{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9f74a0202a3b4532e6e2d76d53bbe5461a4831a9","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        get_token \u003d True"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        # Check to see if the token is still valid"},{"line_number":74,"context_line":"        if IronicClientWrapper._cli is not None:"},{"line_number":75,"context_line":"            try:"},{"line_number":76,"context_line":"                token \u003d IronicClientWrapper._cli.http_client.auth_ref[\u0027token\u0027]"},{"line_number":77,"context_line":"                if self._is_token_valid(token):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_dfd256d1","line":74,"updated":"2014-07-03 09:55:32.000000000","message":"_cli is a private attribute (prefixed with \u0027_\u0027) should not be accesses from outside","commit_id":"369a589a1e34fbb5399273a071317841b7606b8a"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"040a5230b66543d5d07fed9ff01149e52e7dc812","unresolved":false,"context_lines":[{"line_number":71,"context_line":"        get_token \u003d True"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        # Check to see if the token is still valid"},{"line_number":74,"context_line":"        if IronicClientWrapper._cli is not None:"},{"line_number":75,"context_line":"            try:"},{"line_number":76,"context_line":"                token \u003d IronicClientWrapper._cli.http_client.auth_ref[\u0027token\u0027]"},{"line_number":77,"context_line":"                if self._is_token_valid(token):"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_a8bd6bb9","line":74,"in_reply_to":"baada198_dfd256d1","updated":"2014-07-03 10:38:16.000000000","message":"It\u0027s not accessed from the outside, only from this internal method.  The reason it\u0027s referenced from the class is because it\u0027s a class static variable, across instances.","commit_id":"369a589a1e34fbb5399273a071317841b7606b8a"},{"author":{"_account_id":6773,"name":"Lucas Alvares Gomes","email":"lucasagomes@gmail.com","username":"lucasagomes"},"change_message_id":"9f74a0202a3b4532e6e2d76d53bbe5461a4831a9","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        # Check to see if the token is still valid"},{"line_number":74,"context_line":"        if IronicClientWrapper._cli is not None:"},{"line_number":75,"context_line":"            try:"},{"line_number":76,"context_line":"                token \u003d IronicClientWrapper._cli.http_client.auth_ref[\u0027token\u0027]"},{"line_number":77,"context_line":"                if self._is_token_valid(token):"},{"line_number":78,"context_line":"                    LOG.debug(\"Using existing authentication token\")"},{"line_number":79,"context_line":"                    get_token \u003d False"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_3a0fe02f","line":76,"updated":"2014-07-03 09:55:32.000000000","message":"If we require the client to expose the new \"auth_ref\" attribute, apart from having the change merged into the python-ironicclient we also need to release a new version of the client containing the change and pin the version in the Ironic (and nova) requirements.\n\nI think that we may work on a compatible layer here, if the client have the \"auth_ref\" attribute use it to cache the token etc... If not, just get a token everytime we call _get_client - as before.\n\nDependencies across different projects are hard ones :(","commit_id":"369a589a1e34fbb5399273a071317841b7606b8a"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"040a5230b66543d5d07fed9ff01149e52e7dc812","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        # Check to see if the token is still valid"},{"line_number":74,"context_line":"        if IronicClientWrapper._cli is not None:"},{"line_number":75,"context_line":"            try:"},{"line_number":76,"context_line":"                token \u003d IronicClientWrapper._cli.http_client.auth_ref[\u0027token\u0027]"},{"line_number":77,"context_line":"                if self._is_token_valid(token):"},{"line_number":78,"context_line":"                    LOG.debug(\"Using existing authentication token\")"},{"line_number":79,"context_line":"                    get_token \u003d False"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_a819ebd8","line":76,"in_reply_to":"baada198_3a0fe02f","updated":"2014-07-03 10:38:16.000000000","message":"I\u0027ll look at this to see how feasible it might be.  The question is to hold a cached value across get_client() calls  without breaking the abstractions in ironicclient today - investigating...","commit_id":"369a589a1e34fbb5399273a071317841b7606b8a"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"64840a6c3934ae27b6c39804122453a7a070b930","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        # Check to see if the token is still valid"},{"line_number":74,"context_line":"        if IronicClientWrapper._cli is not None:"},{"line_number":75,"context_line":"            try:"},{"line_number":76,"context_line":"                token \u003d IronicClientWrapper._cli.http_client.auth_ref[\u0027token\u0027]"},{"line_number":77,"context_line":"                if self._is_token_valid(token):"},{"line_number":78,"context_line":"                    LOG.debug(\"Using existing authentication token\")"},{"line_number":79,"context_line":"                    get_token \u003d False"}],"source_content_type":"text/x-python","patch_set":4,"id":"baada198_6847c8bc","line":76,"in_reply_to":"baada198_a819ebd8","updated":"2014-07-03 14:49:46.000000000","message":"I\u0027ve decoupled the patches and hence the versions of ironic and ironicclient.  If an older version of the ironicclient is used, caching simply isn\u0027t available. Hope you find this approach an acceptable compromise :)","commit_id":"369a589a1e34fbb5399273a071317841b7606b8a"},{"author":{"_account_id":2889,"name":"Aeva Black","email":"aeva.online@gmail.com","username":"tenbrae"},"change_message_id":"fd27d18f441d2cee4e112e4435872fc70e303db8","unresolved":false,"context_lines":[{"line_number":55,"context_line":"        else:"},{"line_number":56,"context_line":"            return False"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    def _get_client(self):"},{"line_number":59,"context_line":"        auth_token \u003d CONF.ironic.admin_auth_token"},{"line_number":60,"context_line":"        if auth_token is None:"},{"line_number":61,"context_line":"            kwargs \u003d {\u0027os_username\u0027: CONF.ironic.admin_username,"}],"source_content_type":"text/x-python","patch_set":6,"id":"baada198_9bf40b5c","line":58,"updated":"2014-07-09 17:51:24.000000000","message":"I think you need a mutex around the test-or-set logic here to prevent multiple greenthreads from racing and generating more than one token. Nova provides this through the @utils.synchronized decorator\n\nAlso, using a metaclass property might be a cleaner way to guard a variable. Here\u0027s an (untested) example of what I mean:\n\n  from nova import utils\n\n  def get_kwargs():\n    if CONF.ironic.admin_auth_token is none:\n      return {...}\n    else:\n      return {...}\n\n  def is_token_valid(token):\n    return ...\n\n  class ClientCacher(type):\n    @property\n    @utils.synchronized(\u0027cache-ironic-client\u0027)\n    def client(class):\n      get_token \u003d True\n      if class._cli is not None:\n        # check validity of cached token\n        # catch errors\n      if get_token:\n        try: # to get new token\n          kwargs \u003d get_kwargs()\n          class._cli \u003d ...\n        except Unauthorized:\n          ...\n      return class._cli\n\n  class IronicClientWrapper(object):\n    __metaclass__ \u003d ClientCacher\n\n    _cli \u003d None\n\n    def call(...):\n      # accessing self.client is guarded and will generate new token if needed\n      client \u003d self.client\n      # do stuff with client","commit_id":"608628bdfb3c6d3ed24042aaa185f49539cecba1"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"1fbfe9e7abcd5292ecd8eb860804e9225950592e","unresolved":false,"context_lines":[{"line_number":48,"context_line":"    else:"},{"line_number":49,"context_line":"        return False"},{"line_number":50,"context_line":""},{"line_number":51,"context_line":"def get_kwargs():"},{"line_number":52,"context_line":"    auth_token \u003d CONF.ironic.admin_auth_token"},{"line_number":53,"context_line":"    if auth_token is None:"},{"line_number":54,"context_line":"        kwargs \u003d {\u0027os_username\u0027: CONF.ironic.admin_username,"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_64920f4d","line":51,"updated":"2014-07-15 22:50:23.000000000","message":"nit: missing docstring","commit_id":"64c09d3a658c45cfce15ddbac73106ce3837eac5"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"1fbfe9e7abcd5292ecd8eb860804e9225950592e","unresolved":false,"context_lines":[{"line_number":64,"context_line":""},{"line_number":65,"context_line":""},{"line_number":66,"context_line":"class ClientCacher(type):"},{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    # Note(mrda): Before using this cached client, you should check the"},{"line_number":69,"context_line":"    # expiry time in the token."},{"line_number":70,"context_line":"    _cli \u003d None"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_84ae4380","line":67,"updated":"2014-07-15 22:50:23.000000000","message":"nit: missing docstring","commit_id":"64c09d3a658c45cfce15ddbac73106ce3837eac5"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"1fbfe9e7abcd5292ecd8eb860804e9225950592e","unresolved":false,"context_lines":[{"line_number":78,"context_line":""},{"line_number":79,"context_line":"    @property"},{"line_number":80,"context_line":"    @utils.synchronized(\u0027cache-ironic-client\u0027)"},{"line_number":81,"context_line":"    def client(cls):"},{"line_number":82,"context_line":"        get_token \u003d True"},{"line_number":83,"context_line":"        if ClientCacher._cli is not None:"},{"line_number":84,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_44b42bd4","line":81,"updated":"2014-07-15 22:50:23.000000000","message":"nit: missing docstring","commit_id":"64c09d3a658c45cfce15ddbac73106ce3837eac5"},{"author":{"_account_id":3099,"name":"David Shrewsbury","email":"dshrewsb@redhat.com","username":"dshrews"},"change_message_id":"f85217938e908441b3e472333f487be22f688fd1","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"class IronicClientWrapper(object):"},{"line_number":115,"context_line":"    __metaclass__ \u003d ClientCacher"},{"line_number":116,"context_line":"    \"\"\"Ironic client wrapper class that encapsulates retry logic.\"\"\""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _multi_getattr(self, obj, attr):"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_c96a161b","line":115,"updated":"2014-07-15 22:26:23.000000000","message":"I don\u0027t think this works with python3.\n\nhttp://legacy.python.org/dev/peps/pep-3115/\n\nhttps://pythonhosted.org/six/#six.add_metaclass","commit_id":"64c09d3a658c45cfce15ddbac73106ce3837eac5"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"1fbfe9e7abcd5292ecd8eb860804e9225950592e","unresolved":false,"context_lines":[{"line_number":112,"context_line":""},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"class IronicClientWrapper(object):"},{"line_number":115,"context_line":"    __metaclass__ \u003d ClientCacher"},{"line_number":116,"context_line":"    \"\"\"Ironic client wrapper class that encapsulates retry logic.\"\"\""},{"line_number":117,"context_line":""},{"line_number":118,"context_line":"    def _multi_getattr(self, obj, attr):"}],"source_content_type":"text/x-python","patch_set":8,"id":"baada198_44d18b2e","line":115,"in_reply_to":"baada198_c96a161b","updated":"2014-07-15 22:50:23.000000000","message":"Thanks David for the review - I\u0027ll change this to @six.add_metaclass so it\u0027s python 2 and python 3 compatible.  I had it this way during test, but on the rebase/merge back I lost that.","commit_id":"64c09d3a658c45cfce15ddbac73106ce3837eac5"},{"author":{"_account_id":5805,"name":"Chris Krelle","email":"nobodycam@gmail.com","username":"nobodycam"},"change_message_id":"db560b3d3549025373d47901397d696b29da262a","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    \"\"\"Check the supplied token\u0027s expiry date."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    If the supplied token\u0027s expiry date is more than 30 seconds in the future"},{"line_number":40,"context_line":"    then it\u0027s deemed to be valid."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    :param token: The token to check"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_c6b64ef9","line":40,"updated":"2014-07-17 00:12:29.000000000","message":"I don\u0027 t think this is a valid check as to the validity of the token.","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"4a199c29fe9ebb486e98be81ed79f0f49a7922b7","unresolved":false,"context_lines":[{"line_number":37,"context_line":"    \"\"\"Check the supplied token\u0027s expiry date."},{"line_number":38,"context_line":""},{"line_number":39,"context_line":"    If the supplied token\u0027s expiry date is more than 30 seconds in the future"},{"line_number":40,"context_line":"    then it\u0027s deemed to be valid."},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    :param token: The token to check"},{"line_number":43,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_1aa975dd","line":40,"in_reply_to":"baada198_c6b64ef9","updated":"2014-07-17 03:15:24.000000000","message":"So it\u0027s token expiry that we care about in this scenario.  Should this be resolved by renaming this method to has_token_expired ?","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"63613a2d70ea12de264ae650554ef1f5bfaf4fcf","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def get_kwargs():"},{"line_number":55,"context_line":"    \"\"\" Return a dictionary needed for authenticating Ironic Client. \"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    auth_token \u003d CONF.ironic.admin_auth_token"},{"line_number":58,"context_line":"    if auth_token is None:"},{"line_number":59,"context_line":"        kwargs \u003d {\u0027os_username\u0027: CONF.ironic.admin_username,"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_864ff669","line":56,"updated":"2014-07-16 23:41:51.000000000","message":"This blank line isn\u0027t needed (and is against PEP-257).","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"4a199c29fe9ebb486e98be81ed79f0f49a7922b7","unresolved":false,"context_lines":[{"line_number":53,"context_line":""},{"line_number":54,"context_line":"def get_kwargs():"},{"line_number":55,"context_line":"    \"\"\" Return a dictionary needed for authenticating Ironic Client. \"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    auth_token \u003d CONF.ironic.admin_auth_token"},{"line_number":58,"context_line":"    if auth_token is None:"},{"line_number":59,"context_line":"        kwargs \u003d {\u0027os_username\u0027: CONF.ironic.admin_username,"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_9aebc511","line":56,"in_reply_to":"baada198_864ff669","updated":"2014-07-17 03:15:24.000000000","message":"Thanks for the review lifeless.\n\nI don\u0027t think we should hold a different standard to what our tooling enforces - otherwise it\u0027s \"Do pep8, and then check against PEP-257, and then...\" which doesn\u0027t make things easy for developers.  So if this should be enforced, we should update our pep8 config appropriately.\n\nHaving said that I have removed the blank line :(","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"63613a2d70ea12de264ae650554ef1f5bfaf4fcf","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    expires, the client is automatically re-obtained without any additional"},{"line_number":77,"context_line":"    steps by the caller."},{"line_number":78,"context_line":"    \"\"\""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    # Note(mrda): Before using this cached client, you should check the"},{"line_number":81,"context_line":"    # expiry time in the token."},{"line_number":82,"context_line":"    _cli \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_a656ba04","line":79,"updated":"2014-07-16 23:41:51.000000000","message":"Same here.","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"4a199c29fe9ebb486e98be81ed79f0f49a7922b7","unresolved":false,"context_lines":[{"line_number":76,"context_line":"    expires, the client is automatically re-obtained without any additional"},{"line_number":77,"context_line":"    steps by the caller."},{"line_number":78,"context_line":"    \"\"\""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    # Note(mrda): Before using this cached client, you should check the"},{"line_number":81,"context_line":"    # expiry time in the token."},{"line_number":82,"context_line":"    _cli \u003d None"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_baf24904","line":79,"in_reply_to":"baada198_a656ba04","updated":"2014-07-17 03:15:24.000000000","message":"Done","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"},{"author":{"_account_id":4190,"name":"lifeless","email":"robertc@robertcollins.net","username":"lifeless"},"change_message_id":"63613a2d70ea12de264ae650554ef1f5bfaf4fcf","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    \"\"\""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    # Note(mrda): Before using this cached client, you should check the"},{"line_number":81,"context_line":"    # expiry time in the token."},{"line_number":82,"context_line":"    _cli \u003d None"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def __new__(cls, name, parents, dct):"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_c659ee36","line":81,"updated":"2014-07-16 23:41:51.000000000","message":"This is a pointless warning - tokens can expire early (e.g. memcache restarts). We have to handle an invalidtoken error everywhere in the client.","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"},{"author":{"_account_id":8125,"name":"Michael Davies","email":"michael@the-davies.net","username":"mrda"},"change_message_id":"4a199c29fe9ebb486e98be81ed79f0f49a7922b7","unresolved":false,"context_lines":[{"line_number":78,"context_line":"    \"\"\""},{"line_number":79,"context_line":""},{"line_number":80,"context_line":"    # Note(mrda): Before using this cached client, you should check the"},{"line_number":81,"context_line":"    # expiry time in the token."},{"line_number":82,"context_line":"    _cli \u003d None"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def __new__(cls, name, parents, dct):"}],"source_content_type":"text/x-python","patch_set":9,"id":"baada198_daf53dee","line":81,"in_reply_to":"baada198_c659ee36","updated":"2014-07-17 03:15:24.000000000","message":"I\u0027ll remove the comment.","commit_id":"81820665d784edbcd437ec69dc2f4f6e8a9c7732"}]}