)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1916,"name":"Guang Yee","email":"gyee@suse.com","username":"guang-yee"},"change_message_id":"fc60409baa27b467a55cd1ab8a37cdcbb1336d00","unresolved":false,"context_lines":[{"line_number":12,"context_line":"Added pool test using existing FakeLdap as connector class."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: If516a0d308a7f3be88df5583a30739a935076173"},{"line_number":15,"context_line":"Closes-Bug: #1320997"},{"line_number":16,"context_line":"bp: ldap-connection-pooling"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1ae5cdf2_1972e849","line":15,"updated":"2014-06-19 16:55:29.000000000","message":"DocImpact","commit_id":"69aee1f948c75a163d0774c24feca6cdcdd527a1"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"1dbe1e8ce1172f3134b2d8fec3933fecffd5c079","unresolved":false,"context_lines":[{"line_number":12,"context_line":"Added pool test using existing FakeLdap as connector class."},{"line_number":13,"context_line":""},{"line_number":14,"context_line":"Change-Id: If516a0d308a7f3be88df5583a30739a935076173"},{"line_number":15,"context_line":"Closes-Bug: #1320997"},{"line_number":16,"context_line":"bp: ldap-connection-pooling"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":7,"id":"1ae5cdf2_097a9576","line":15,"in_reply_to":"1ae5cdf2_1972e849","updated":"2014-06-19 18:54:36.000000000","message":"Will address in next patch.","commit_id":"69aee1f948c75a163d0774c24feca6cdcdd527a1"}],"doc/source/configuration.rst":[{"author":{"_account_id":1916,"name":"Guang Yee","email":"gyee@suse.com","username":"guang-yee"},"change_message_id":"fc60409baa27b467a55cd1ab8a37cdcbb1336d00","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"  [ldappool]"},{"line_number":1537,"context_line":"  # Enable ldap connection pooling. (boolean value)"},{"line_number":1538,"context_line":"  use_pool\u003dfalse"},{"line_number":1539,"context_line":"  "},{"line_number":1540,"context_line":"  # Pool size. Default size is 10 (integer value)"},{"line_number":1541,"context_line":"  pool_size\u003d10"},{"line_number":1542,"context_line":"  "}],"source_content_type":"text/x-rst","patch_set":7,"id":"1ae5cdf2_5988f021","line":1539,"updated":"2014-06-19 16:55:29.000000000","message":"white spaces","commit_id":"69aee1f948c75a163d0774c24feca6cdcdd527a1"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"1dbe1e8ce1172f3134b2d8fec3933fecffd5c079","unresolved":false,"context_lines":[{"line_number":1536,"context_line":"  [ldappool]"},{"line_number":1537,"context_line":"  # Enable ldap connection pooling. (boolean value)"},{"line_number":1538,"context_line":"  use_pool\u003dfalse"},{"line_number":1539,"context_line":"  "},{"line_number":1540,"context_line":"  # Pool size. Default size is 10 (integer value)"},{"line_number":1541,"context_line":"  pool_size\u003d10"},{"line_number":1542,"context_line":"  "}],"source_content_type":"text/x-rst","patch_set":7,"id":"1ae5cdf2_a988c16b","line":1539,"in_reply_to":"1ae5cdf2_5988f021","updated":"2014-06-19 18:54:36.000000000","message":"Will address in next patch.","commit_id":"69aee1f948c75a163d0774c24feca6cdcdd527a1"}],"etc/keystone.conf.sample":[{"author":{"_account_id":1916,"name":"Guang Yee","email":"gyee@suse.com","username":"guang-yee"},"change_message_id":"fc60409baa27b467a55cd1ab8a37cdcbb1336d00","unresolved":false,"context_lines":[{"line_number":1074,"context_line":"#tls_req_cert\u003ddemand"},{"line_number":1075,"context_line":""},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"[ldappool]"},{"line_number":1078,"context_line":""},{"line_number":1079,"context_line":"#"},{"line_number":1080,"context_line":"# Options defined in keystone"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"1ae5cdf2_79cccc35","line":1077,"updated":"2014-06-19 16:55:29.000000000","message":"this section should be auto-generated by \n\ntools/config/generate_sample.sh\n\nyou may want to push a generated file instead.","commit_id":"69aee1f948c75a163d0774c24feca6cdcdd527a1"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"1dbe1e8ce1172f3134b2d8fec3933fecffd5c079","unresolved":false,"context_lines":[{"line_number":1074,"context_line":"#tls_req_cert\u003ddemand"},{"line_number":1075,"context_line":""},{"line_number":1076,"context_line":""},{"line_number":1077,"context_line":"[ldappool]"},{"line_number":1078,"context_line":""},{"line_number":1079,"context_line":"#"},{"line_number":1080,"context_line":"# Options defined in keystone"}],"source_content_type":"application/octet-stream","patch_set":7,"id":"1ae5cdf2_09f1b500","line":1077,"in_reply_to":"1ae5cdf2_79cccc35","updated":"2014-06-19 18:54:36.000000000","message":"Yes, it was generated by script.","commit_id":"69aee1f948c75a163d0774c24feca6cdcdd527a1"}],"keystone/common/config.py":[{"author":{"_account_id":2218,"name":"Adam Young","email":"adam@younglogic.com","username":"ayoung"},"change_message_id":"98d26ede17c74540bc555b512020344c29fbd0b6","unresolved":false,"context_lines":[{"line_number":642,"context_line":"                   help\u003d\u0027Valid options for tls_req_cert are demand, never, \u0027"},{"line_number":643,"context_line":"                        \u0027and allow.\u0027),"},{"line_number":644,"context_line":"    ],"},{"line_number":645,"context_line":"    \u0027ldappool\u0027: ["},{"line_number":646,"context_line":"        cfg.BoolOpt(\u0027use_pool\u0027, default\u003dFalse,"},{"line_number":647,"context_line":"                    help\u003d\u0027Enable ldap connection pooling.\u0027),"},{"line_number":648,"context_line":"        cfg.IntOpt(\u0027pool_size\u0027, default\u003d10,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ae5cdf2_9b1c0779","line":645,"updated":"2014-06-20 17:29:35.000000000","message":"Why aren\u0027t these just under ldap?","commit_id":"9f3cd209ce3224609a873f2ec235427a7927e747"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"79e0727d257c6719bd452862f73259b1450c98b3","unresolved":false,"context_lines":[{"line_number":642,"context_line":"                   help\u003d\u0027Valid options for tls_req_cert are demand, never, \u0027"},{"line_number":643,"context_line":"                        \u0027and allow.\u0027),"},{"line_number":644,"context_line":"    ],"},{"line_number":645,"context_line":"    \u0027ldappool\u0027: ["},{"line_number":646,"context_line":"        cfg.BoolOpt(\u0027use_pool\u0027, default\u003dFalse,"},{"line_number":647,"context_line":"                    help\u003d\u0027Enable ldap connection pooling.\u0027),"},{"line_number":648,"context_line":"        cfg.IntOpt(\u0027pool_size\u0027, default\u003d10,"}],"source_content_type":"text/x-python","patch_set":8,"id":"1ae5cdf2_f09e0553","line":645,"in_reply_to":"1ae5cdf2_9b1c0779","updated":"2014-06-20 19:28:36.000000000","message":"I kept it separate to clarify pooling attributes from LDAP config section which is already too big. Also, sometimes its easier to search and make changes in a separate section.\n\nLooks like preference is to have within ldap section. I will make that change in next patch.","commit_id":"9f3cd209ce3224609a873f2ec235427a7927e747"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"e7526b5ddd2c21c65a4132381202a9868219c35d","unresolved":false,"context_lines":[{"line_number":647,"context_line":"                   help\u003d\u0027Pool size. Default size is 10\u0027),"},{"line_number":648,"context_line":"        cfg.IntOpt(\u0027pool_retry_max\u0027, default\u003d3,"},{"line_number":649,"context_line":"                   help\u003d\u0027Maximum count of reconnect trials. \u0027"},{"line_number":650,"context_line":"                        \u0027Default is 3\u0027),"},{"line_number":651,"context_line":"        cfg.FloatOpt(\u0027pool_retry_delay\u0027, default\u003d0.1,"},{"line_number":652,"context_line":"                     help\u003d\u0027Time span in seconds to wait between two \u0027"},{"line_number":653,"context_line":"                          \u0027reconnect trials. Default is 0.1\u0027),"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_96c5d53e","line":650,"updated":"2014-06-27 20:19:05.000000000","message":"Looking over the other config options, we don\u0027t specify \"Default\" values. Suggest removing the default specification(s)","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"80b39822baff73d830c6086ac0599a7318685392","unresolved":false,"context_lines":[{"line_number":647,"context_line":"                   help\u003d\u0027Pool size. Default size is 10\u0027),"},{"line_number":648,"context_line":"        cfg.IntOpt(\u0027pool_retry_max\u0027, default\u003d3,"},{"line_number":649,"context_line":"                   help\u003d\u0027Maximum count of reconnect trials. \u0027"},{"line_number":650,"context_line":"                        \u0027Default is 3\u0027),"},{"line_number":651,"context_line":"        cfg.FloatOpt(\u0027pool_retry_delay\u0027, default\u003d0.1,"},{"line_number":652,"context_line":"                     help\u003d\u0027Time span in seconds to wait between two \u0027"},{"line_number":653,"context_line":"                          \u0027reconnect trials. Default is 0.1\u0027),"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_1ed1d7fa","line":650,"in_reply_to":"1ae5cdf2_96c5d53e","updated":"2014-06-27 22:01:14.000000000","message":"Okay..will address in next patch.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"b1cd81d2fecc48d15ebbf6e970c2afdf479c8802","unresolved":false,"context_lines":[{"line_number":683,"context_line":"                          \u0027reconnect trials.\u0027),"},{"line_number":684,"context_line":"        cfg.IntOpt(\u0027pool_connection_timeout\u0027, default\u003d-1,"},{"line_number":685,"context_line":"                   help\u003d\u0027Connector timeout in seconds. Value -1 indicates \u0027"},{"line_number":686,"context_line":"                        \u0027indefinite wait for response\u0027),"},{"line_number":687,"context_line":"        cfg.IntOpt(\u0027pool_connection_lifetime\u0027, default\u003d600,"},{"line_number":688,"context_line":"                   help\u003d\u0027Connection lifetime in seconds.\u0027),"},{"line_number":689,"context_line":"    ],"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_f278d77f","line":686,"updated":"2014-06-28 07:00:53.000000000","message":"nit - missing full stop at end of 2nd sentence","commit_id":"0068442051badb8b5b55c62481c664a5ee3afedb"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"cbba68d40e14f1aa0ab2e15ca83f4a7e5c6c0b9a","unresolved":false,"context_lines":[{"line_number":683,"context_line":"                          \u0027reconnect trials.\u0027),"},{"line_number":684,"context_line":"        cfg.IntOpt(\u0027pool_connection_timeout\u0027, default\u003d-1,"},{"line_number":685,"context_line":"                   help\u003d\u0027Connector timeout in seconds. Value -1 indicates \u0027"},{"line_number":686,"context_line":"                        \u0027indefinite wait for response\u0027),"},{"line_number":687,"context_line":"        cfg.IntOpt(\u0027pool_connection_lifetime\u0027, default\u003d600,"},{"line_number":688,"context_line":"                   help\u003d\u0027Connection lifetime in seconds.\u0027),"},{"line_number":689,"context_line":"    ],"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_48c55c3b","line":686,"in_reply_to":"1ae5cdf2_f278d77f","updated":"2014-06-28 12:46:07.000000000","message":"Will address in next patch.","commit_id":"0068442051badb8b5b55c62481c664a5ee3afedb"}],"keystone/common/ldap/core.py":[{"author":{"_account_id":8116,"name":"John Dennis","email":"jdennis@redhat.com","username":"jdennis"},"change_message_id":"391cc987aa349a368c7858223ffdc6be47307667","unresolved":false,"context_lines":[{"line_number":547,"context_line":"    def delete_ext_s(self, dn, serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":548,"context_line":"        return self.conn.delete_ext_s(dn, serverctrls, clientctrls)"},{"line_number":549,"context_line":""},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"def _common_ldap_initialization(url, page_size\u003d0,"},{"line_number":552,"context_line":"                                alias_dereferencing\u003dNone, use_tls\u003dFalse,"},{"line_number":553,"context_line":"                                tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_9b95f9e5","line":550,"updated":"2014-06-27 20:45:47.000000000","message":"The LDAP options being set here are library/process global options. What happens when two different connect calls are made which have different options? You\u0027re going to overwrite the previous options and change behavior with unpredictable results. Note that it is now possible to set TLS options on a connection basis instead of globally.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"80b39822baff73d830c6086ac0599a7318685392","unresolved":false,"context_lines":[{"line_number":547,"context_line":"    def delete_ext_s(self, dn, serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":548,"context_line":"        return self.conn.delete_ext_s(dn, serverctrls, clientctrls)"},{"line_number":549,"context_line":""},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"def _common_ldap_initialization(url, page_size\u003d0,"},{"line_number":552,"context_line":"                                alias_dereferencing\u003dNone, use_tls\u003dFalse,"},{"line_number":553,"context_line":"                                tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_64424e3c","line":550,"in_reply_to":"1ae5cdf2_9b95f9e5","updated":"2014-06-27 22:01:14.000000000","message":"Existing ldap core code sets the options similarly which is global/ module level e.g. line #483, # 505 in existing code (left pane)\n\nTLS setting is so far configured at keystone deployment level as per values defined in keystone.conf. Once there are multiple LDAP configurations with different TLS attributes, I think existing code can be enhanced to support that as it stores the connection manager per ldap connect URL.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":2903,"name":"Morgan Fainberg","email":"morgan.fainberg@gmail.com","username":"mdrnstm"},"change_message_id":"e7526b5ddd2c21c65a4132381202a9868219c35d","unresolved":false,"context_lines":[{"line_number":650,"context_line":"        if chase_referrals is not None:"},{"line_number":651,"context_line":"            self.set_option(ldap.OPT_REFERRALS, int(chase_referrals))"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        global CONNECTION_POOLS"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"        if url not in CONNECTION_POOLS:"},{"line_number":656,"context_line":"            CONNECTION_POOLS[url] \u003d ldappool.ConnectionManager("}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_154b7a2b","line":653,"updated":"2014-06-27 20:19:05.000000000","message":"(NIT): the global identifier here is probably not needed because you\u0027re not binding the name \u0027CONNECTION_POOLS\u0027 locally, you\u0027re setting values in the dictionary.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"80b39822baff73d830c6086ac0599a7318685392","unresolved":false,"context_lines":[{"line_number":650,"context_line":"        if chase_referrals is not None:"},{"line_number":651,"context_line":"            self.set_option(ldap.OPT_REFERRALS, int(chase_referrals))"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        global CONNECTION_POOLS"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"        if url not in CONNECTION_POOLS:"},{"line_number":656,"context_line":"            CONNECTION_POOLS[url] \u003d ldappool.ConnectionManager("}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_3b2065af","line":653,"in_reply_to":"1ae5cdf2_154b7a2b","updated":"2014-06-27 22:01:14.000000000","message":"I was trying to be explicit here to signify that its a global variable usage. I can address it in next rebase/ patch if its a concern.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":8116,"name":"John Dennis","email":"jdennis@redhat.com","username":"jdennis"},"change_message_id":"391cc987aa349a368c7858223ffdc6be47307667","unresolved":false,"context_lines":[{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    def get_option(self, option):"},{"line_number":671,"context_line":"        return self.conn_options.get(option)"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"    def _apply_options(self, conn):"},{"line_number":674,"context_line":"        for option, invalue in six.iteritems(self.conn_options):"},{"line_number":675,"context_line":"            conn.set_option(option, invalue)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_7ba10d39","line":672,"updated":"2014-06-27 20:45:47.000000000","message":"This seems very inefficient. You\u0027re setting and resetting every connection option on every ldap call.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"80b39822baff73d830c6086ac0599a7318685392","unresolved":false,"context_lines":[{"line_number":669,"context_line":""},{"line_number":670,"context_line":"    def get_option(self, option):"},{"line_number":671,"context_line":"        return self.conn_options.get(option)"},{"line_number":672,"context_line":""},{"line_number":673,"context_line":"    def _apply_options(self, conn):"},{"line_number":674,"context_line":"        for option, invalue in six.iteritems(self.conn_options):"},{"line_number":675,"context_line":"            conn.set_option(option, invalue)"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_64fa4ebb","line":672,"in_reply_to":"1ae5cdf2_7ba10d39","updated":"2014-06-27 22:01:14.000000000","message":"I will try to optimize this in next patch. Probably I can verify if option exists and value is same or not before setting it.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":8116,"name":"John Dennis","email":"jdennis@redhat.com","username":"jdennis"},"change_message_id":"391cc987aa349a368c7858223ffdc6be47307667","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        # of the operations"},{"line_number":683,"context_line":"        self.user \u003d who"},{"line_number":684,"context_line":"        self.password \u003d cred"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"    def unbind_s(self):"},{"line_number":687,"context_line":"        # this is a no op as connection exit would do an unbind anyway"},{"line_number":688,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_5b2b7194","line":685,"updated":"2014-06-27 20:45:47.000000000","message":"A connection should support switching between identities. unbind should be supported and one needs to verify each time the connection is used it\u0027s bound to the expected identity.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"80b39822baff73d830c6086ac0599a7318685392","unresolved":false,"context_lines":[{"line_number":682,"context_line":"        # of the operations"},{"line_number":683,"context_line":"        self.user \u003d who"},{"line_number":684,"context_line":"        self.password \u003d cred"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"    def unbind_s(self):"},{"line_number":687,"context_line":"        # this is a no op as connection exit would do an unbind anyway"},{"line_number":688,"context_line":"        pass"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_09e7e312","line":685,"in_reply_to":"1ae5cdf2_5b2b7194","updated":"2014-06-27 22:01:14.000000000","message":"When connection is obtained from pool, identify is matched first. https://github.com/mozilla-services/ldappool/blob/master/ldappool/__init__.py#L238\n\nFor unbind: Please see response below for Nathan Kinder comment.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":9098,"name":"Nathan Kinder","email":"nkinder@redhat.com","username":"nkinder"},"change_message_id":"b915a9728c491d094718d41d9dfd0d5f811432b5","unresolved":false,"context_lines":[{"line_number":684,"context_line":"        self.password \u003d cred"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"    def unbind_s(self):"},{"line_number":687,"context_line":"        # this is a no op as connection exit would do an unbind anyway"},{"line_number":688,"context_line":"        pass"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    def add_s(self, dn, modlist):"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_98797ffe","line":687,"updated":"2014-06-27 20:04:57.000000000","message":"I am a bit concerned that we don\u0027t have unbind implemented.  I assume that this means that a connection is left bound as the most recently authenticated user.  I don\u0027t like the idea of leaving a connection bound as an end user, as it\u0027s hare to say what that connection will be reused for next.  It would be much better to unbind and assume an anonymous identity.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"80b39822baff73d830c6086ac0599a7318685392","unresolved":false,"context_lines":[{"line_number":684,"context_line":"        self.password \u003d cred"},{"line_number":685,"context_line":""},{"line_number":686,"context_line":"    def unbind_s(self):"},{"line_number":687,"context_line":"        # this is a no op as connection exit would do an unbind anyway"},{"line_number":688,"context_line":"        pass"},{"line_number":689,"context_line":""},{"line_number":690,"context_line":"    def add_s(self, dn, modlist):"}],"source_content_type":"text/x-python","patch_set":9,"id":"1ae5cdf2_3e6db399","line":687,"in_reply_to":"1ae5cdf2_98797ffe","updated":"2014-06-27 22:01:14.000000000","message":"Ldappool always does unbind internally as soon as it comes out of with statement (connection generator). Please see https://github.com/mozilla-services/ldappool/blob/master/ldappool/__init__.py#L315\n\nMay be I can improve on the comment clarifying this.","commit_id":"adfc32861c60eea02884ed3ddfad502f5cc9605e"},{"author":{"_account_id":5707,"name":"Henry Nash","email":"henryn@linux.vnet.ibm.com","username":"henry-nash"},"change_message_id":"b1cd81d2fecc48d15ebbf6e970c2afdf479c8802","unresolved":false,"context_lines":[{"line_number":650,"context_line":"        if chase_referrals is not None:"},{"line_number":651,"context_line":"            self.set_option(ldap.OPT_REFERRALS, int(chase_referrals))"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        if url not in CONNECTION_POOLS:"},{"line_number":654,"context_line":"            CONNECTION_POOLS[url] \u003d ldappool.ConnectionManager("},{"line_number":655,"context_line":"                url,"},{"line_number":656,"context_line":"                size\u003dCONF.ldap.pool_size,"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_d2d3db5c","line":653,"updated":"2014-06-28 07:00:53.000000000","message":"So re-using a connection from the pool presumably creates a more likely (albeit still small) probability that the connection will expire during the processing of transaction?  Did you consider whether it was worth creating a new connection if the lifetime here was below some threshold?","commit_id":"0068442051badb8b5b55c62481c664a5ee3afedb"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"cbba68d40e14f1aa0ab2e15ca83f4a7e5c6c0b9a","unresolved":false,"context_lines":[{"line_number":650,"context_line":"        if chase_referrals is not None:"},{"line_number":651,"context_line":"            self.set_option(ldap.OPT_REFERRALS, int(chase_referrals))"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        if url not in CONNECTION_POOLS:"},{"line_number":654,"context_line":"            CONNECTION_POOLS[url] \u003d ldappool.ConnectionManager("},{"line_number":655,"context_line":"                url,"},{"line_number":656,"context_line":"                size\u003dCONF.ldap.pool_size,"}],"source_content_type":"text/x-python","patch_set":11,"id":"1ae5cdf2_c8b96ccc","line":653,"in_reply_to":"1ae5cdf2_d2d3db5c","updated":"2014-06-28 12:46:07.000000000","message":"ldappool internally retries on connection failures or server restarts. https://github.com/mozilla-services/ldappool/blob/master/ldappool/__init__.py#L212\n\nAlso ldappool does connection lifetime check before returning connection. If existing connection is past lifetime, it unbinds and removes that connection from pool and return new one.","commit_id":"0068442051badb8b5b55c62481c664a5ee3afedb"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"CONF \u003d config.CONF"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"LDAP_VALUES \u003d {\u0027TRUE\u0027: True, \u0027FALSE\u0027: False}"},{"line_number":35,"context_line":"CONTROL_TREEDELETE \u003d \u00271.2.840.113556.1.4.805\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_6b442d47","line":32,"updated":"2014-07-18 10:30:58.000000000","message":"You shouldn\u0027t use global CONF object anywhere here. You should pass all necessary parameters from conf argument to BaseLdap.__init__ down to the handler.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"CONF \u003d config.CONF"},{"line_number":33,"context_line":""},{"line_number":34,"context_line":"LDAP_VALUES \u003d {\u0027TRUE\u0027: True, \u0027FALSE\u0027: False}"},{"line_number":35,"context_line":"CONTROL_TREEDELETE \u003d \u00271.2.840.113556.1.4.805\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_077aa4b9","line":32,"in_reply_to":"baada198_6b442d47","updated":"2014-07-19 01:36:58.000000000","message":"Will address in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        return self.conn.delete_ext_s(dn, serverctrls, clientctrls)"},{"line_number":549,"context_line":""},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"def _common_ldap_initialization(url, page_size\u003d0,"},{"line_number":552,"context_line":"                                alias_dereferencing\u003dNone, use_tls\u003dFalse,"},{"line_number":553,"context_line":"                                tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"},{"line_number":554,"context_line":"                                tls_req_cert\u003dNone,"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_ebff5d64","line":551,"range":{"start_line":551,"start_character":37,"end_line":551,"end_character":46},"updated":"2014-07-18 10:30:58.000000000","message":"Unused argument","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":548,"context_line":"        return self.conn.delete_ext_s(dn, serverctrls, clientctrls)"},{"line_number":549,"context_line":""},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"def _common_ldap_initialization(url, page_size\u003d0,"},{"line_number":552,"context_line":"                                alias_dereferencing\u003dNone, use_tls\u003dFalse,"},{"line_number":553,"context_line":"                                tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"},{"line_number":554,"context_line":"                                tls_req_cert\u003dNone,"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_c75a6c4f","line":551,"range":{"start_line":551,"start_character":37,"end_line":551,"end_character":46},"in_reply_to":"baada198_ebff5d64","updated":"2014-07-19 01:36:58.000000000","message":"Will remove in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":549,"context_line":""},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"def _common_ldap_initialization(url, page_size\u003d0,"},{"line_number":552,"context_line":"                                alias_dereferencing\u003dNone, use_tls\u003dFalse,"},{"line_number":553,"context_line":"                                tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"},{"line_number":554,"context_line":"                                tls_req_cert\u003dNone,"},{"line_number":555,"context_line":"                                chase_referrals\u003dNone, debug_level\u003dNone):"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_a879eb94","line":552,"range":{"start_line":552,"start_character":32,"end_line":552,"end_character":51},"updated":"2014-07-18 10:30:58.000000000","message":"Unused argument","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":549,"context_line":""},{"line_number":550,"context_line":""},{"line_number":551,"context_line":"def _common_ldap_initialization(url, page_size\u003d0,"},{"line_number":552,"context_line":"                                alias_dereferencing\u003dNone, use_tls\u003dFalse,"},{"line_number":553,"context_line":"                                tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"},{"line_number":554,"context_line":"                                tls_req_cert\u003dNone,"},{"line_number":555,"context_line":"                                chase_referrals\u003dNone, debug_level\u003dNone):"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_a75d784a","line":552,"range":{"start_line":552,"start_character":32,"end_line":552,"end_character":51},"in_reply_to":"baada198_a879eb94","updated":"2014-07-19 01:36:58.000000000","message":"Will remove in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":552,"context_line":"                                alias_dereferencing\u003dNone, use_tls\u003dFalse,"},{"line_number":553,"context_line":"                                tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"},{"line_number":554,"context_line":"                                tls_req_cert\u003dNone,"},{"line_number":555,"context_line":"                                chase_referrals\u003dNone, debug_level\u003dNone):"},{"line_number":556,"context_line":"    \u0027\u0027\u0027Method for common ldap initialization between PythonLDAPHandler and"},{"line_number":557,"context_line":"    PooledLDAPHandler."},{"line_number":558,"context_line":"    \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_e86f63c2","line":555,"range":{"start_line":555,"start_character":32,"end_line":555,"end_character":47},"updated":"2014-07-18 10:30:58.000000000","message":"Unused argument","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":552,"context_line":"                                alias_dereferencing\u003dNone, use_tls\u003dFalse,"},{"line_number":553,"context_line":"                                tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"},{"line_number":554,"context_line":"                                tls_req_cert\u003dNone,"},{"line_number":555,"context_line":"                                chase_referrals\u003dNone, debug_level\u003dNone):"},{"line_number":556,"context_line":"    \u0027\u0027\u0027Method for common ldap initialization between PythonLDAPHandler and"},{"line_number":557,"context_line":"    PooledLDAPHandler."},{"line_number":558,"context_line":"    \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_8745d470","line":555,"range":{"start_line":555,"start_character":32,"end_line":555,"end_character":47},"in_reply_to":"baada198_e86f63c2","updated":"2014-07-19 01:36:58.000000000","message":"Will remove in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":619,"context_line":"    \u0027\u0027\u0027"},{"line_number":620,"context_line":""},{"line_number":621,"context_line":"    # Added here to allow override for testing"},{"line_number":622,"context_line":"    CONNECTOR_CLS \u003d ldappool.StateConnector"},{"line_number":623,"context_line":""},{"line_number":624,"context_line":"    def __init__(self, conn\u003dNone):"},{"line_number":625,"context_line":"        super(PooledLDAPHandler, self).__init__(conn\u003dconn)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_3a1fe071","line":622,"range":{"start_line":622,"start_character":4,"end_line":622,"end_character":17},"updated":"2014-07-18 10:30:58.000000000","message":"All caps doesn\u0027t fit here. It\u0027s just a class variable. You can name it Connector to represent that it should be a class.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":619,"context_line":"    \u0027\u0027\u0027"},{"line_number":620,"context_line":""},{"line_number":621,"context_line":"    # Added here to allow override for testing"},{"line_number":622,"context_line":"    CONNECTOR_CLS \u003d ldappool.StateConnector"},{"line_number":623,"context_line":""},{"line_number":624,"context_line":"    def __init__(self, conn\u003dNone):"},{"line_number":625,"context_line":"        super(PooledLDAPHandler, self).__init__(conn\u003dconn)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_2731680b","line":622,"range":{"start_line":622,"start_character":4,"end_line":622,"end_character":17},"in_reply_to":"baada198_3a1fe071","updated":"2014-07-19 01:36:58.000000000","message":"Renaming it to \u0027Connector\u0027 variable in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":623,"context_line":""},{"line_number":624,"context_line":"    def __init__(self, conn\u003dNone):"},{"line_number":625,"context_line":"        super(PooledLDAPHandler, self).__init__(conn\u003dconn)"},{"line_number":626,"context_line":"        self.who \u003d \u0027\u0027"},{"line_number":627,"context_line":"        self.cred \u003d \u0027\u0027"},{"line_number":628,"context_line":"        self.conn_options \u003d {}  # connection specific options"},{"line_number":629,"context_line":""},{"line_number":630,"context_line":"    def connect(self, url, page_size\u003d0, alias_dereferencing\u003dNone,"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_28931ba4","line":627,"range":{"start_line":626,"start_character":8,"end_line":627,"end_character":22},"updated":"2014-07-18 10:30:58.000000000","message":"Why not None?","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":640,"context_line":"                                    chase_referrals\u003dchase_referrals,"},{"line_number":641,"context_line":"                                    debug_level\u003ddebug_level)"},{"line_number":642,"context_line":""},{"line_number":643,"context_line":"        self.url \u003d url"},{"line_number":644,"context_line":"        self.page_size \u003d page_size"},{"line_number":645,"context_line":""},{"line_number":646,"context_line":"        # Following two options are not added in common initialization as they"},{"line_number":647,"context_line":"        # need to follow a sequence in PythonLDAPHandler code."},{"line_number":648,"context_line":"        if alias_dereferencing is not None:"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_c878df80","line":645,"range":{"start_line":643,"start_character":0,"end_line":645,"end_character":0},"updated":"2014-07-18 10:30:58.000000000","message":"You should set these to None in __init__.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":657,"context_line":"                retry_max\u003dCONF.ldap.pool_retry_max,"},{"line_number":658,"context_line":"                retry_delay\u003dCONF.ldap.pool_retry_delay,"},{"line_number":659,"context_line":"                timeout\u003dCONF.ldap.pool_connection_timeout,"},{"line_number":660,"context_line":"                connector_cls\u003dPooledLDAPHandler.CONNECTOR_CLS,"},{"line_number":661,"context_line":"                use_pool\u003dCONF.ldap.use_pool,"},{"line_number":662,"context_line":"                use_tls\u003duse_tls,"},{"line_number":663,"context_line":"                max_lifetime\u003dCONF.ldap.pool_connection_lifetime)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_1a5dbc48","line":660,"range":{"start_line":660,"start_character":30,"end_line":660,"end_character":47},"updated":"2014-07-18 10:30:58.000000000","message":"You should use self here.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":657,"context_line":"                retry_max\u003dCONF.ldap.pool_retry_max,"},{"line_number":658,"context_line":"                retry_delay\u003dCONF.ldap.pool_retry_delay,"},{"line_number":659,"context_line":"                timeout\u003dCONF.ldap.pool_connection_timeout,"},{"line_number":660,"context_line":"                connector_cls\u003dPooledLDAPHandler.CONNECTOR_CLS,"},{"line_number":661,"context_line":"                use_pool\u003dCONF.ldap.use_pool,"},{"line_number":662,"context_line":"                use_tls\u003duse_tls,"},{"line_number":663,"context_line":"                max_lifetime\u003dCONF.ldap.pool_connection_lifetime)"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_6737e0fe","line":660,"range":{"start_line":660,"start_character":30,"end_line":660,"end_character":47},"in_reply_to":"baada198_1a5dbc48","updated":"2014-07-19 01:36:58.000000000","message":"Will address in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":658,"context_line":"                retry_delay\u003dCONF.ldap.pool_retry_delay,"},{"line_number":659,"context_line":"                timeout\u003dCONF.ldap.pool_connection_timeout,"},{"line_number":660,"context_line":"                connector_cls\u003dPooledLDAPHandler.CONNECTOR_CLS,"},{"line_number":661,"context_line":"                use_pool\u003dCONF.ldap.use_pool,"},{"line_number":662,"context_line":"                use_tls\u003duse_tls,"},{"line_number":663,"context_line":"                max_lifetime\u003dCONF.ldap.pool_connection_lifetime)"},{"line_number":664,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_0bf9f1c7","line":661,"range":{"start_line":661,"start_character":16,"end_line":661,"end_character":43},"updated":"2014-07-18 10:30:58.000000000","message":"We wouldn\u0027t get here if use_pool would\u0027ve False. This argument is unneeded.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":658,"context_line":"                retry_delay\u003dCONF.ldap.pool_retry_delay,"},{"line_number":659,"context_line":"                timeout\u003dCONF.ldap.pool_connection_timeout,"},{"line_number":660,"context_line":"                connector_cls\u003dPooledLDAPHandler.CONNECTOR_CLS,"},{"line_number":661,"context_line":"                use_pool\u003dCONF.ldap.use_pool,"},{"line_number":662,"context_line":"                use_tls\u003duse_tls,"},{"line_number":663,"context_line":"                max_lifetime\u003dCONF.ldap.pool_connection_lifetime)"},{"line_number":664,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_ca0fd32c","line":661,"range":{"start_line":661,"start_character":16,"end_line":661,"end_character":43},"in_reply_to":"baada198_0bf9f1c7","updated":"2014-07-19 01:36:58.000000000","message":"I agree. Will remove in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        # if option was not specified explictly, then use connection default"},{"line_number":671,"context_line":"        # value for that option if there."},{"line_number":672,"context_line":"        if value is None:"},{"line_number":673,"context_line":"            with CONNECTION_POOLS[self.url].connection(self.who,"},{"line_number":674,"context_line":"                                                       self.cred) as conn:"},{"line_number":675,"context_line":"                value \u003d conn.get_option(option)"},{"line_number":676,"context_line":"        return value"},{"line_number":677,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_c8fa9f76","line":674,"range":{"start_line":673,"start_character":17,"end_line":674,"end_character":65},"updated":"2014-07-18 10:30:58.000000000","message":"This call repeats a lot in the code. You should create a method for it like this:\n\n    def _get_connection(self, who\u003dNone, cred\u003dNone):\n\nWhere who and cred would default to self.who and self.cred.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":670,"context_line":"        # if option was not specified explictly, then use connection default"},{"line_number":671,"context_line":"        # value for that option if there."},{"line_number":672,"context_line":"        if value is None:"},{"line_number":673,"context_line":"            with CONNECTION_POOLS[self.url].connection(self.who,"},{"line_number":674,"context_line":"                                                       self.cred) as conn:"},{"line_number":675,"context_line":"                value \u003d conn.get_option(option)"},{"line_number":676,"context_line":"        return value"},{"line_number":677,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_2a268fba","line":674,"range":{"start_line":673,"start_character":17,"end_line":674,"end_character":65},"in_reply_to":"baada198_c8fa9f76","updated":"2014-07-19 01:36:58.000000000","message":"Will address in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":686,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":687,"context_line":"        with CONNECTION_POOLS[self.url].connection(who, cred) as conn:"},{"line_number":688,"context_line":"            self._apply_options(conn)"},{"line_number":689,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":690,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_77ffaa02","line":687,"updated":"2014-07-18 10:30:58.000000000","message":"This won\u0027t work for following scenario:\n\n- user connects, authenticates;\n- user changes password;\n- user connects again using old password.\n\nldappool will reuse existing connection with old credentials then. I guess there should be some guard to force password verification.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":684,"context_line":""},{"line_number":685,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":686,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":687,"context_line":"        with CONNECTION_POOLS[self.url].connection(who, cred) as conn:"},{"line_number":688,"context_line":"            self._apply_options(conn)"},{"line_number":689,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":690,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_4a790392","line":687,"in_reply_to":"baada198_77ffaa02","updated":"2014-07-19 01:36:58.000000000","message":"If LDAP connection password needs to be changed, keystone configuration needs to be updated and possibly server needs to restart to make configuration change effective. So pool dynamic reconfig is not needed here and supported elsewhere in keystone.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":698,"context_line":"        pass"},{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def add_s(self, dn, modlist):"},{"line_number":701,"context_line":"        with CONNECTION_POOLS[self.url].connection("},{"line_number":702,"context_line":"                self.who, self.cred) as conn:"},{"line_number":703,"context_line":"            self._apply_options(conn)"},{"line_number":704,"context_line":"            return conn.add_s(dn, modlist)"},{"line_number":705,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_4845afb2","line":702,"range":{"start_line":701,"start_character":8,"end_line":702,"end_character":45},"updated":"2014-07-18 10:30:58.000000000","message":"And since there are a lot of methods that run entirely in (the same) context manager and run the same _apply_options, you should move it to a decorator that would pass conn to the method.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":723,"context_line":"                                   serverctrls, clientctrls,"},{"line_number":724,"context_line":"                                   timeout, sizelimit)"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"    def result3(self, msgid\u003dldap.RES_ANY, all\u003d1, timeout\u003dNone,"},{"line_number":727,"context_line":"                resp_ctrl_classes\u003dNone):"},{"line_number":728,"context_line":"        with CONNECTION_POOLS[self.url].connection("},{"line_number":729,"context_line":"                self.who, self.cred) as conn:"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_ee0dab90","line":726,"range":{"start_line":726,"start_character":8,"end_line":726,"end_character":15},"updated":"2014-07-18 10:30:58.000000000","message":"I don\u0027t think this would work if we run search_ext in one connection and then result3 in another one.\n\nYou should probably encode smth into msgid returned by search_ext. For example, you can do smth like this:\n\n    class MsgId(list): pass\n\n    def search_ext(...):\n        conn_mgr \u003d conn_pool.connection(...)\n        conn \u003d conn_mgr.__enter__()\n        try:\n            msgid \u003d conn.search_ext(...)\n        except:\n            conn_mgr.__exit__(*sys.exc_info())\n            raise\n        res \u003d MsgId((conn, msgid))\n        weakref.ref(res, functools.partial(conn_mgr.__exit__, None, None, None))\n        return res\n\n    def result3(...):\n        conn, msgid \u003d msgid\n        return conn.result3(msgid, ...)\n\nThis will release connection once last reference to MsgId object is freed (basically when the method that called it finishes).","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":723,"context_line":"                                   serverctrls, clientctrls,"},{"line_number":724,"context_line":"                                   timeout, sizelimit)"},{"line_number":725,"context_line":""},{"line_number":726,"context_line":"    def result3(self, msgid\u003dldap.RES_ANY, all\u003d1, timeout\u003dNone,"},{"line_number":727,"context_line":"                resp_ctrl_classes\u003dNone):"},{"line_number":728,"context_line":"        with CONNECTION_POOLS[self.url].connection("},{"line_number":729,"context_line":"                self.who, self.cred) as conn:"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_0a5b8b22","line":726,"range":{"start_line":726,"start_character":8,"end_line":726,"end_character":15},"in_reply_to":"baada198_ee0dab90","updated":"2014-07-19 01:36:58.000000000","message":"Will address in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":800,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":801,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":802,"context_line":"        LOG.debug(\"LDAP bind: who\u003d%s\", who)"},{"line_number":803,"context_line":"        who_utf8 \u003d utf8_encode(who)"},{"line_number":804,"context_line":"        cred_utf8 \u003d utf8_encode(cred)"},{"line_number":805,"context_line":"        return self.conn.simple_bind_s(who_utf8, cred_utf8,"},{"line_number":806,"context_line":"                                       serverctrls, clientctrls)"},{"line_number":807,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_68a6f3ac","line":804,"range":{"start_line":803,"start_character":8,"end_line":804,"end_character":37},"updated":"2014-07-18 10:30:58.000000000","message":"Note that who and cred are encoded here. You should probably do the same.\n\nUPD: Or better use KeystoneLDAPHandler(conn) everywhere so that all these processing steps happens for all methods.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":800,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":801,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":802,"context_line":"        LOG.debug(\"LDAP bind: who\u003d%s\", who)"},{"line_number":803,"context_line":"        who_utf8 \u003d utf8_encode(who)"},{"line_number":804,"context_line":"        cred_utf8 \u003d utf8_encode(cred)"},{"line_number":805,"context_line":"        return self.conn.simple_bind_s(who_utf8, cred_utf8,"},{"line_number":806,"context_line":"                                       serverctrls, clientctrls)"},{"line_number":807,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_9a8610b8","line":804,"range":{"start_line":803,"start_character":8,"end_line":804,"end_character":37},"in_reply_to":"baada198_68a6f3ac","updated":"2014-07-19 01:36:58.000000000","message":"Next line calls same method on self.conn instance with encoded who and cred parameters and that object in case of pooling is PooledLdapHandler. So I think we don\u0027t need to do that again.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":1236,"context_line":"            if k \u003d\u003d \u0027id\u0027 or k in self.attribute_ignore:"},{"line_number":1237,"context_line":"                continue"},{"line_number":1238,"context_line":"            if v is not None:"},{"line_number":1239,"context_line":"                attr_type \u003d self.attribute_mapping.get(k)"},{"line_number":1240,"context_line":"                if attr_type is not None:"},{"line_number":1241,"context_line":"                    attrs.append((attr_type, [v]))"},{"line_number":1242,"context_line":"                extra_attrs \u003d [attr for attr, name"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_ef8f9074","line":1239,"updated":"2014-07-18 10:30:58.000000000","message":"Is this change relevant?","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":1236,"context_line":"            if k \u003d\u003d \u0027id\u0027 or k in self.attribute_ignore:"},{"line_number":1237,"context_line":"                continue"},{"line_number":1238,"context_line":"            if v is not None:"},{"line_number":1239,"context_line":"                attr_type \u003d self.attribute_mapping.get(k)"},{"line_number":1240,"context_line":"                if attr_type is not None:"},{"line_number":1241,"context_line":"                    attrs.append((attr_type, [v]))"},{"line_number":1242,"context_line":"                extra_attrs \u003d [attr for attr, name"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_aa953f89","line":1239,"in_reply_to":"baada198_ef8f9074","updated":"2014-07-19 01:36:58.000000000","message":"Existing ldap live tests are broken in master. When I run them against local instance of openLdap, there are number of failures. So I am going to add defect against them and also will address them in separate branch.\nThis is one of the needed change, so I can run using these tests.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":632,"context_line":""},{"line_number":633,"context_line":"    def __init__(self, conn\u003dNone):"},{"line_number":634,"context_line":"        super(PooledLDAPHandler, self).__init__(conn\u003dconn)"},{"line_number":635,"context_line":"        self.who \u003d \u0027\u0027"},{"line_number":636,"context_line":"        self.cred \u003d \u0027\u0027"},{"line_number":637,"context_line":"        self.conn_options \u003d {}  # connection specific options"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"    def connect(self, url, page_size\u003d0, alias_dereferencing\u003dNone,"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_3edded14","line":636,"range":{"start_line":635,"start_character":8,"end_line":636,"end_character":22},"updated":"2014-07-19 09:04:51.000000000","message":"Not addressed from previous patchset:\n\nWhy not None?","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"6fe21f56d2dd400976a72e308239061942949f24","unresolved":false,"context_lines":[{"line_number":632,"context_line":""},{"line_number":633,"context_line":"    def __init__(self, conn\u003dNone):"},{"line_number":634,"context_line":"        super(PooledLDAPHandler, self).__init__(conn\u003dconn)"},{"line_number":635,"context_line":"        self.who \u003d \u0027\u0027"},{"line_number":636,"context_line":"        self.cred \u003d \u0027\u0027"},{"line_number":637,"context_line":"        self.conn_options \u003d {}  # connection specific options"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"    def connect(self, url, page_size\u003d0, alias_dereferencing\u003dNone,"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_4288ed21","line":636,"range":{"start_line":635,"start_character":8,"end_line":636,"end_character":22},"in_reply_to":"baada198_011289b3","updated":"2014-07-21 15:19:36.000000000","message":"I don\u0027t see how it\u0027ll fail there. But anyway I think it\u0027d be a good thing for it to fail as it would clearly show programming error since these attributes should be initialized before ldappool is called.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":632,"context_line":""},{"line_number":633,"context_line":"    def __init__(self, conn\u003dNone):"},{"line_number":634,"context_line":"        super(PooledLDAPHandler, self).__init__(conn\u003dconn)"},{"line_number":635,"context_line":"        self.who \u003d \u0027\u0027"},{"line_number":636,"context_line":"        self.cred \u003d \u0027\u0027"},{"line_number":637,"context_line":"        self.conn_options \u003d {}  # connection specific options"},{"line_number":638,"context_line":""},{"line_number":639,"context_line":"    def connect(self, url, page_size\u003d0, alias_dereferencing\u003dNone,"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_011289b3","line":636,"range":{"start_line":635,"start_character":8,"end_line":636,"end_character":22},"in_reply_to":"baada198_3edded14","updated":"2014-07-19 20:13:40.000000000","message":"This is because of LDAP pool code. It always expects not None password. If somebody passes None, then its fails at \nhttps://github.com/mozilla-services/ldappool/blob/master/ldappool/__init__.py#L143","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                                    tls_req_cert\u003dtls_req_cert,"},{"line_number":651,"context_line":"                                    debug_level\u003ddebug_level)"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        self.url \u003d url"},{"line_number":654,"context_line":"        self.page_size \u003d page_size"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        # Following two options are not added in common initialization as they"},{"line_number":657,"context_line":"        # need to follow a sequence in PythonLDAPHandler code."}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_7ee3e5d7","line":654,"range":{"start_line":653,"start_character":8,"end_line":654,"end_character":34},"updated":"2014-07-19 09:04:51.000000000","message":"Not addressed from previous patchset:\n\nYou should  set these to None in __init__.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":650,"context_line":"                                    tls_req_cert\u003dtls_req_cert,"},{"line_number":651,"context_line":"                                    debug_level\u003ddebug_level)"},{"line_number":652,"context_line":""},{"line_number":653,"context_line":"        self.url \u003d url"},{"line_number":654,"context_line":"        self.page_size \u003d page_size"},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"        # Following two options are not added in common initialization as they"},{"line_number":657,"context_line":"        # need to follow a sequence in PythonLDAPHandler code."}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_ab737ee5","line":654,"range":{"start_line":653,"start_character":8,"end_line":654,"end_character":34},"in_reply_to":"baada198_7ee3e5d7","updated":"2014-07-19 20:13:40.000000000","message":"Will address in next patch.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":701,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":702,"context_line":"        with self._get_pool_connection(who, cred) as conn:"},{"line_number":703,"context_line":"            self._apply_options(conn)"},{"line_number":704,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":705,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_9ee8b9b4","line":702,"updated":"2014-07-19 09:04:51.000000000","message":"Continuing discussion from previous patchset:\n\nPlease take a look here: http://git.openstack.org/cgit/openstack/keystone/tree/keystone/identity/backends/ldap.py#n66\n\nWhen user authenticates in Keystone, ones password is verified by doing bind on  a LDAP connection.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"6fe21f56d2dd400976a72e308239061942949f24","unresolved":false,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":701,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":702,"context_line":"        with self._get_pool_connection(who, cred) as conn:"},{"line_number":703,"context_line":"            self._apply_options(conn)"},{"line_number":704,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":705,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_a26e29fe","line":702,"in_reply_to":"baada198_81d059f8","updated":"2014-07-21 15:19:36.000000000","message":"I can\u0027t find a test that covers scenario I\u0027ve proposed. The problem is not with user that logins with new password but with user that logins with old password. Here\u0027s a pseudocode of what I\u0027m talking about:\n\n    authenticate(\u0027user1\u0027, \u0027password\u0027)\n    # A connection with (\u0027user1\u0027, \u0027password\u0027) is added to pool\n    change_password(\u0027user1\u0027, \u0027password\u0027, \u0027newpassword\u0027)\n    # Old password shouldn\u0027t work anymore\n    authenticate(\u0027user1\u0027, \u0027password\u0027)\n    # Connection with (\u0027user1\u0027, \u0027password\u0027) is found in the pool\n    # and authentication succeeds.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0d11fa27d8509e11732fcfae5f0802b9b0add8bb","unresolved":false,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":701,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":702,"context_line":"        with self._get_pool_connection(who, cred) as conn:"},{"line_number":703,"context_line":"            self._apply_options(conn)"},{"line_number":704,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":705,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_fd689422","line":702,"in_reply_to":"baada198_9619528a","updated":"2014-07-22 03:37:01.000000000","message":"As per IRC discussion, I agree with taking option #2 but with pooling either turned off by default or connections\u0027 lifetime made rather short.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":701,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":702,"context_line":"        with self._get_pool_connection(who, cred) as conn:"},{"line_number":703,"context_line":"            self._apply_options(conn)"},{"line_number":704,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":705,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_81d059f8","line":702,"in_reply_to":"baada198_9ee8b9b4","updated":"2014-07-19 20:13:40.000000000","message":"That is correct. If I recall, the concern you raised earlier was around password change. Ldap pool does match of existing connection creds and if not found, creates new bind for user cred data.\n\nNow there is no way to know whether user password is changed or is invalid. So in this case, if there is user password is modified, then no existing connection match found, so new bind is established.\n\nThere are unit test around authenticate method which executes in both pool versions..fake ldap and live ldap tests","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"1e42f11ded6bcb1abef6a419b6bdc47ae2203e39","unresolved":false,"context_lines":[{"line_number":699,"context_line":""},{"line_number":700,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":701,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":702,"context_line":"        with self._get_pool_connection(who, cred) as conn:"},{"line_number":703,"context_line":"            self._apply_options(conn)"},{"line_number":704,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":705,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_9619528a","line":702,"in_reply_to":"baada198_a26e29fe","updated":"2014-07-21 17:39:55.000000000","message":"You are right. With recent password change, there can be issue if end_user is passing old password to authenticate and that can pass. On top of my mind, I think two approaches to handle this.\n# 1: Do not use pool for that end_user authentication logic where end user username and password is used for ldap bind to authenticate it.\n# 2: Create a separate pool for end_user auth ldap call which has lower connection lifetime (and may be different size) to handle above password change auth issue.\n\nFor #1: In keystone, authentication is one of frequently used request  so overall pool effectiveness is going to be reduced especially if request happens to be repetitive within a short span.\n\nFor #2: Having separate pool for user_auth allows to have shorter connection lifetime and pool size. This way, a deployer can choose optimum size and connection lifetime. Also it can choose not to use pool for end_user ldap bind if it *always* want to bind via new connection.\n\nTo me #2 seems like a good compromise in security and performance. What do you think?","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":713,"context_line":"        pass"},{"line_number":714,"context_line":""},{"line_number":715,"context_line":"    def add_s(self, dn, modlist):"},{"line_number":716,"context_line":"        with self._get_pool_connection() as conn:"},{"line_number":717,"context_line":"            self._apply_options(conn)"},{"line_number":718,"context_line":"            return conn.add_s(dn, modlist)"},{"line_number":719,"context_line":""},{"line_number":720,"context_line":"    def search_s(self, base, scope,"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_bed01df8","line":717,"range":{"start_line":716,"start_character":8,"end_line":717,"end_character":37},"updated":"2014-07-19 09:04:51.000000000","message":"Not addressed from previous patchset:\n\nAnd since there are a lot of methods that run entirely in (the same) context manager and run the same _apply_options, you should move it to a decorator that would pass conn to the method.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":713,"context_line":"        pass"},{"line_number":714,"context_line":""},{"line_number":715,"context_line":"    def add_s(self, dn, modlist):"},{"line_number":716,"context_line":"        with self._get_pool_connection() as conn:"},{"line_number":717,"context_line":"            self._apply_options(conn)"},{"line_number":718,"context_line":"            return conn.add_s(dn, modlist)"},{"line_number":719,"context_line":""},{"line_number":720,"context_line":"    def search_s(self, base, scope,"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_c4441fba","line":717,"range":{"start_line":716,"start_character":8,"end_line":717,"end_character":37},"in_reply_to":"baada198_bed01df8","updated":"2014-07-19 20:13:40.000000000","message":"Will add decorator in next patch.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        which requested msgId and used it in result3 exits."},{"line_number":770,"context_line":"        \u0027\u0027\u0027"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        if msgid is None:"},{"line_number":773,"context_line":"            msgId \u003d ldap.RES_ANY"},{"line_number":774,"context_line":"        else:"},{"line_number":775,"context_line":"            conn, msgId \u003d msgid"},{"line_number":776,"context_line":"        return conn.result3(msgId, all, timeout)"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_fed615f1","line":773,"range":{"start_line":772,"start_character":8,"end_line":773,"end_character":32},"updated":"2014-07-19 09:04:51.000000000","message":"This branch won\u0027t work since there\u0027s no conn for line #776. I think we can safely make msgid argument mandatory.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":769,"context_line":"        which requested msgId and used it in result3 exits."},{"line_number":770,"context_line":"        \u0027\u0027\u0027"},{"line_number":771,"context_line":""},{"line_number":772,"context_line":"        if msgid is None:"},{"line_number":773,"context_line":"            msgId \u003d ldap.RES_ANY"},{"line_number":774,"context_line":"        else:"},{"line_number":775,"context_line":"            conn, msgId \u003d msgid"},{"line_number":776,"context_line":"        return conn.result3(msgId, all, timeout)"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_eb79f6c5","line":773,"range":{"start_line":772,"start_character":8,"end_line":773,"end_character":32},"in_reply_to":"baada198_fed615f1","updated":"2014-07-19 20:13:40.000000000","message":"Agreed. Made the change.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":852,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":853,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":854,"context_line":"        LOG.debug(\"LDAP bind: who\u003d%s\", who)"},{"line_number":855,"context_line":"        who_utf8 \u003d utf8_encode(who)"},{"line_number":856,"context_line":"        cred_utf8 \u003d utf8_encode(cred)"},{"line_number":857,"context_line":"        return self.conn.simple_bind_s(who_utf8, cred_utf8,"},{"line_number":858,"context_line":"                                       serverctrls, clientctrls)"},{"line_number":859,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_3e040d7d","line":856,"range":{"start_line":855,"start_character":7,"end_line":856,"end_character":37},"updated":"2014-07-19 09:04:51.000000000","message":"Continuing discussion from previous patchset:\n\nWhen ldap.use_pool\u003dTrue is specified in config, with current implementation KeystoneLDAPHandler is not used at all. So I suggest modifying _get_pool_connection to return connections wrapped in KeystoneLDAPHandler to properly handle all corner cases that are already handled here.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":852,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":853,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":854,"context_line":"        LOG.debug(\"LDAP bind: who\u003d%s\", who)"},{"line_number":855,"context_line":"        who_utf8 \u003d utf8_encode(who)"},{"line_number":856,"context_line":"        cred_utf8 \u003d utf8_encode(cred)"},{"line_number":857,"context_line":"        return self.conn.simple_bind_s(who_utf8, cred_utf8,"},{"line_number":858,"context_line":"                                       serverctrls, clientctrls)"},{"line_number":859,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_4e40305f","line":856,"range":{"start_line":855,"start_character":7,"end_line":856,"end_character":37},"in_reply_to":"baada198_3e040d7d","updated":"2014-07-19 20:13:40.000000000","message":"In case of pool, KeystoneLDAPHandler class is used but not PythonLDAPHandler class methods. I have added a test to assert that utf8_encode method is called with username and cred.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"6fe21f56d2dd400976a72e308239061942949f24","unresolved":false,"context_lines":[{"line_number":852,"context_line":"    def simple_bind_s(self, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":853,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":854,"context_line":"        LOG.debug(\"LDAP bind: who\u003d%s\", who)"},{"line_number":855,"context_line":"        who_utf8 \u003d utf8_encode(who)"},{"line_number":856,"context_line":"        cred_utf8 \u003d utf8_encode(cred)"},{"line_number":857,"context_line":"        return self.conn.simple_bind_s(who_utf8, cred_utf8,"},{"line_number":858,"context_line":"                                       serverctrls, clientctrls)"},{"line_number":859,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_a23c69e2","line":856,"range":{"start_line":855,"start_character":7,"end_line":856,"end_character":37},"in_reply_to":"baada198_4e40305f","updated":"2014-07-21 15:19:36.000000000","message":"Oh, right. No need to test for that. Sorry.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":620,"context_line":""},{"line_number":621,"context_line":"    This adds connection object to decorated API as next argument after self."},{"line_number":622,"context_line":"    \u0027\u0027\u0027"},{"line_number":623,"context_line":"    def wrapper(*args, **kwargs):"},{"line_number":624,"context_line":"        self_arg \u003d args[0]"},{"line_number":625,"context_line":"        # assert isinstance(self_arg, PooledLDAPHandler)"},{"line_number":626,"context_line":"        if kwargs.get(\u0027who\u0027):"},{"line_number":627,"context_line":"            who \u003d kwargs.get(\u0027who\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_e14d713c","line":624,"range":{"start_line":623,"start_character":4,"end_line":624,"end_character":26},"updated":"2014-07-22 03:44:29.000000000","message":"You should explicitly state self as the first argument:\n\n  def wrapper(self, *args, **kwargs):","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"47628ec559a66373fffd48a5047f0af39fb887d4","unresolved":false,"context_lines":[{"line_number":620,"context_line":""},{"line_number":621,"context_line":"    This adds connection object to decorated API as next argument after self."},{"line_number":622,"context_line":"    \u0027\u0027\u0027"},{"line_number":623,"context_line":"    def wrapper(*args, **kwargs):"},{"line_number":624,"context_line":"        self_arg \u003d args[0]"},{"line_number":625,"context_line":"        # assert isinstance(self_arg, PooledLDAPHandler)"},{"line_number":626,"context_line":"        if kwargs.get(\u0027who\u0027):"},{"line_number":627,"context_line":"            who \u003d kwargs.get(\u0027who\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_986b26b8","line":624,"range":{"start_line":623,"start_character":4,"end_line":624,"end_character":26},"in_reply_to":"baada198_e14d713c","updated":"2014-07-22 04:13:33.000000000","message":"Okay. Will make change in next patch.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":623,"context_line":"    def wrapper(*args, **kwargs):"},{"line_number":624,"context_line":"        self_arg \u003d args[0]"},{"line_number":625,"context_line":"        # assert isinstance(self_arg, PooledLDAPHandler)"},{"line_number":626,"context_line":"        if kwargs.get(\u0027who\u0027):"},{"line_number":627,"context_line":"            who \u003d kwargs.get(\u0027who\u0027)"},{"line_number":628,"context_line":"        else:"},{"line_number":629,"context_line":"            who \u003d self_arg.who"},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"        if kwargs.get(\u0027cred\u0027):"},{"line_number":632,"context_line":"            cred \u003d kwargs.get(\u0027cred\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_e1fbb1bb","line":629,"range":{"start_line":626,"start_character":8,"end_line":629,"end_character":30},"updated":"2014-07-22 03:44:29.000000000","message":"You can do this instead:\n\n  who \u003d kwargs.get(\u0027who\u0027, self.who)","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"47628ec559a66373fffd48a5047f0af39fb887d4","unresolved":false,"context_lines":[{"line_number":623,"context_line":"    def wrapper(*args, **kwargs):"},{"line_number":624,"context_line":"        self_arg \u003d args[0]"},{"line_number":625,"context_line":"        # assert isinstance(self_arg, PooledLDAPHandler)"},{"line_number":626,"context_line":"        if kwargs.get(\u0027who\u0027):"},{"line_number":627,"context_line":"            who \u003d kwargs.get(\u0027who\u0027)"},{"line_number":628,"context_line":"        else:"},{"line_number":629,"context_line":"            who \u003d self_arg.who"},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"        if kwargs.get(\u0027cred\u0027):"},{"line_number":632,"context_line":"            cred \u003d kwargs.get(\u0027cred\u0027)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_7878128f","line":629,"range":{"start_line":626,"start_character":8,"end_line":629,"end_character":30},"in_reply_to":"baada198_e1fbb1bb","updated":"2014-07-22 04:13:33.000000000","message":"Will make change in next patch.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":628,"context_line":"        else:"},{"line_number":629,"context_line":"            who \u003d self_arg.who"},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"        if kwargs.get(\u0027cred\u0027):"},{"line_number":632,"context_line":"            cred \u003d kwargs.get(\u0027cred\u0027)"},{"line_number":633,"context_line":"        else:"},{"line_number":634,"context_line":"            cred \u003d self_arg.cred"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"        with CONNECTION_POOLS[self_arg.url].connection(who, cred) as conn:"},{"line_number":637,"context_line":"            self_arg._apply_options(conn)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_a11c5914","line":634,"range":{"start_line":631,"start_character":8,"end_line":634,"end_character":32},"updated":"2014-07-22 03:44:29.000000000","message":"ditto","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"47628ec559a66373fffd48a5047f0af39fb887d4","unresolved":false,"context_lines":[{"line_number":628,"context_line":"        else:"},{"line_number":629,"context_line":"            who \u003d self_arg.who"},{"line_number":630,"context_line":""},{"line_number":631,"context_line":"        if kwargs.get(\u0027cred\u0027):"},{"line_number":632,"context_line":"            cred \u003d kwargs.get(\u0027cred\u0027)"},{"line_number":633,"context_line":"        else:"},{"line_number":634,"context_line":"            cred \u003d self_arg.cred"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"        with CONNECTION_POOLS[self_arg.url].connection(who, cred) as conn:"},{"line_number":637,"context_line":"            self_arg._apply_options(conn)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_58750e96","line":634,"range":{"start_line":631,"start_character":8,"end_line":634,"end_character":32},"in_reply_to":"baada198_a11c5914","updated":"2014-07-22 04:13:33.000000000","message":"Okay. Will make change in next patch.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":633,"context_line":"        else:"},{"line_number":634,"context_line":"            cred \u003d self_arg.cred"},{"line_number":635,"context_line":""},{"line_number":636,"context_line":"        with CONNECTION_POOLS[self_arg.url].connection(who, cred) as conn:"},{"line_number":637,"context_line":"            self_arg._apply_options(conn)"},{"line_number":638,"context_line":"            args_list \u003d list(args)"},{"line_number":639,"context_line":"            args_list.insert(1, conn)"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_0106e585","line":636,"updated":"2014-07-22 03:44:29.000000000","message":"Overall I think this decorator should use self.who and self.cred for all connections. simple_bind_s would be an exception.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":638,"context_line":"            args_list \u003d list(args)"},{"line_number":639,"context_line":"            args_list.insert(1, conn)"},{"line_number":640,"context_line":"            args \u003d tuple(args_list)"},{"line_number":641,"context_line":"            return func(*args, **kwargs)"},{"line_number":642,"context_line":"    return wrapper"},{"line_number":643,"context_line":""},{"line_number":644,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_8173d57e","line":641,"range":{"start_line":641,"start_character":19,"end_line":641,"end_character":40},"updated":"2014-07-22 03:44:29.000000000","message":"You can format the call without list-tuple dance:\n\n  return func(self, conn, *args, **kwargs)","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"47628ec559a66373fffd48a5047f0af39fb887d4","unresolved":false,"context_lines":[{"line_number":638,"context_line":"            args_list \u003d list(args)"},{"line_number":639,"context_line":"            args_list.insert(1, conn)"},{"line_number":640,"context_line":"            args \u003d tuple(args_list)"},{"line_number":641,"context_line":"            return func(*args, **kwargs)"},{"line_number":642,"context_line":"    return wrapper"},{"line_number":643,"context_line":""},{"line_number":644,"context_line":""}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_d8683ebc","line":641,"range":{"start_line":641,"start_character":19,"end_line":641,"end_character":40},"in_reply_to":"baada198_8173d57e","updated":"2014-07-22 04:13:33.000000000","message":"Okay. Will make change in next patch.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":791,"context_line":"        which requested msgId and used it in result3 exits."},{"line_number":792,"context_line":"        \u0027\u0027\u0027"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"        conn, msgId \u003d msgid"},{"line_number":795,"context_line":"        return conn.result3(msgId, all, timeout)"},{"line_number":796,"context_line":""},{"line_number":797,"context_line":"    @use_conn_pool"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_01a345b2","line":794,"range":{"start_line":794,"start_character":14,"end_line":794,"end_character":19},"updated":"2014-07-22 03:44:29.000000000","message":"Please don\u0027t use camelcase for variable names.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"47628ec559a66373fffd48a5047f0af39fb887d4","unresolved":false,"context_lines":[{"line_number":791,"context_line":"        which requested msgId and used it in result3 exits."},{"line_number":792,"context_line":"        \u0027\u0027\u0027"},{"line_number":793,"context_line":""},{"line_number":794,"context_line":"        conn, msgId \u003d msgid"},{"line_number":795,"context_line":"        return conn.result3(msgId, all, timeout)"},{"line_number":796,"context_line":""},{"line_number":797,"context_line":"    @use_conn_pool"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_f86342da","line":794,"range":{"start_line":794,"start_character":14,"end_line":794,"end_character":19},"in_reply_to":"baada198_01a345b2","updated":"2014-07-22 04:13:33.000000000","message":"Okay. Will make change in next patch.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":870,"context_line":"        LOG.debug(\"LDAP bind: who\u003d%s\", who)"},{"line_number":871,"context_line":"        who_utf8 \u003d utf8_encode(who)"},{"line_number":872,"context_line":"        cred_utf8 \u003d utf8_encode(cred)"},{"line_number":873,"context_line":"        return self.conn.simple_bind_s(who\u003dwho_utf8, cred\u003dcred_utf8,"},{"line_number":874,"context_line":"                                       serverctrls\u003dserverctrls,"},{"line_number":875,"context_line":"                                       clientctrls\u003dclientctrls)"},{"line_number":876,"context_line":""},{"line_number":877,"context_line":"    def unbind_s(self):"},{"line_number":878,"context_line":"        LOG.debug(\"LDAP unbind\")"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_41ae1d82","line":875,"range":{"start_line":873,"start_character":39,"end_line":875,"end_character":62},"updated":"2014-07-22 03:44:29.000000000","message":"I still think that current simple_bind_s approach won\u0027t work for password change, so forcing keyword arguments doesn\u0027t seem to be necessary.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"47628ec559a66373fffd48a5047f0af39fb887d4","unresolved":false,"context_lines":[{"line_number":870,"context_line":"        LOG.debug(\"LDAP bind: who\u003d%s\", who)"},{"line_number":871,"context_line":"        who_utf8 \u003d utf8_encode(who)"},{"line_number":872,"context_line":"        cred_utf8 \u003d utf8_encode(cred)"},{"line_number":873,"context_line":"        return self.conn.simple_bind_s(who\u003dwho_utf8, cred\u003dcred_utf8,"},{"line_number":874,"context_line":"                                       serverctrls\u003dserverctrls,"},{"line_number":875,"context_line":"                                       clientctrls\u003dclientctrls)"},{"line_number":876,"context_line":""},{"line_number":877,"context_line":"    def unbind_s(self):"},{"line_number":878,"context_line":"        LOG.debug(\"LDAP unbind\")"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_98796684","line":875,"range":{"start_line":873,"start_character":39,"end_line":875,"end_character":62},"in_reply_to":"baada198_41ae1d82","updated":"2014-07-22 04:13:33.000000000","message":"For password change, now there is separate pool supported added and option to selectively disable if not needed.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ed819419b7e9fd513d9e0070227a2f7d43e07b5d","unresolved":false,"context_lines":[{"line_number":47,"context_line":""},{"line_number":48,"context_line":"_utf8_encoder \u003d codecs.getencoder(\u0027utf-8\u0027)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"CONNECTION_POOLS \u003d {}"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"def utf8_encode(value):"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_38fe9a06","line":50,"updated":"2014-07-22 04:49:15.000000000","message":"It might look better to store this dict in the PooledLdapHandler class.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"717ddf677042a2c0c7ea3e1207d30e61776bda8e","unresolved":false,"context_lines":[{"line_number":47,"context_line":""},{"line_number":48,"context_line":"_utf8_encoder \u003d codecs.getencoder(\u0027utf-8\u0027)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"CONNECTION_POOLS \u003d {}"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"def utf8_encode(value):"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_56004f02","line":50,"in_reply_to":"baada198_38fe9a06","updated":"2014-07-22 16:31:21.000000000","message":"I added this in module namespace as its quite clear/explicit that connection managers are maintained in global scope i.e. its re-used if matching connection manager is already defined.\n\nAdding it as class/static namespace can sometimes be confusing especially when assigning via instance or class reference.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"db37da65e28adbddc28be92491c46eb914cb0be8","unresolved":false,"context_lines":[{"line_number":47,"context_line":""},{"line_number":48,"context_line":"_utf8_encoder \u003d codecs.getencoder(\u0027utf-8\u0027)"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"CONNECTION_POOLS \u003d {}"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"def utf8_encode(value):"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_beeb3a11","line":50,"in_reply_to":"baada198_56004f02","updated":"2014-07-22 19:16:25.000000000","message":"Well, this variable is not meant to be assigned to, only its keys get new values here.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ed819419b7e9fd513d9e0070227a2f7d43e07b5d","unresolved":false,"context_lines":[{"line_number":670,"context_line":"    def connect(self, url, page_size\u003d0, alias_dereferencing\u003dNone,"},{"line_number":671,"context_line":"                use_tls\u003dFalse, tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"},{"line_number":672,"context_line":"                tls_req_cert\u003d\u0027demand\u0027, chase_referrals\u003dNone, debug_level\u003dNone,"},{"line_number":673,"context_line":"                use_pool\u003dNone, ldap_pool_size\u003dNone, pool_retry_max\u003dNone,"},{"line_number":674,"context_line":"                pool_retry_delay\u003dNone, pool_conn_timeout\u003dNone,"},{"line_number":675,"context_line":"                pool_conn_lifetime\u003dNone):"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_1e437f3f","line":673,"range":{"start_line":673,"start_character":31,"end_line":673,"end_character":45},"updated":"2014-07-22 04:49:15.000000000","message":"This is the only argument with ldap_ prefix. You should name it just pool_size.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"717ddf677042a2c0c7ea3e1207d30e61776bda8e","unresolved":false,"context_lines":[{"line_number":670,"context_line":"    def connect(self, url, page_size\u003d0, alias_dereferencing\u003dNone,"},{"line_number":671,"context_line":"                use_tls\u003dFalse, tls_cacertfile\u003dNone, tls_cacertdir\u003dNone,"},{"line_number":672,"context_line":"                tls_req_cert\u003d\u0027demand\u0027, chase_referrals\u003dNone, debug_level\u003dNone,"},{"line_number":673,"context_line":"                use_pool\u003dNone, ldap_pool_size\u003dNone, pool_retry_max\u003dNone,"},{"line_number":674,"context_line":"                pool_retry_delay\u003dNone, pool_conn_timeout\u003dNone,"},{"line_number":675,"context_line":"                pool_conn_lifetime\u003dNone):"},{"line_number":676,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_00093132","line":673,"range":{"start_line":673,"start_character":31,"end_line":673,"end_character":45},"in_reply_to":"baada198_1e437f3f","updated":"2014-07-22 16:31:21.000000000","message":"Okay. Will address in next patch.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ed819419b7e9fd513d9e0070227a2f7d43e07b5d","unresolved":false,"context_lines":[{"line_number":696,"context_line":"        else:"},{"line_number":697,"context_line":"            self.pool_url \u003d url"},{"line_number":698,"context_line":"        if self.pool_url not in CONNECTION_POOLS:"},{"line_number":699,"context_line":"            CONNECTION_POOLS[self.pool_url] \u003d ldappool.ConnectionManager("},{"line_number":700,"context_line":"                url,"},{"line_number":701,"context_line":"                size\u003dldap_pool_size,"},{"line_number":702,"context_line":"                retry_max\u003dpool_retry_max,"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_f807c21a","line":699,"updated":"2014-07-22 04:49:15.000000000","message":"I think you should store ConnectionManager you\u0027re using for this connection in the self. It won\u0027t change over time anyway. And then we won\u0027t need self.url and self.pool_url anymore.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"db37da65e28adbddc28be92491c46eb914cb0be8","unresolved":false,"context_lines":[{"line_number":696,"context_line":"        else:"},{"line_number":697,"context_line":"            self.pool_url \u003d url"},{"line_number":698,"context_line":"        if self.pool_url not in CONNECTION_POOLS:"},{"line_number":699,"context_line":"            CONNECTION_POOLS[self.pool_url] \u003d ldappool.ConnectionManager("},{"line_number":700,"context_line":"                url,"},{"line_number":701,"context_line":"                size\u003dldap_pool_size,"},{"line_number":702,"context_line":"                retry_max\u003dpool_retry_max,"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_5e005eab","line":699,"in_reply_to":"baada198_d42e1842","updated":"2014-07-22 19:16:25.000000000","message":"I mean we don\u0027t need to store url and pool_url. They are not used anywhere but in a form CONNECTION_POOLS[self.pool_url]. And we can do this subscription once here, in connect method instead of doing it in every call. So we can store ConnectionManager in CONNECTION_POOLS, but we should also store it in self._pool, for example, and use that instead of direct call to CONNECTION_POOLS.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"717ddf677042a2c0c7ea3e1207d30e61776bda8e","unresolved":false,"context_lines":[{"line_number":696,"context_line":"        else:"},{"line_number":697,"context_line":"            self.pool_url \u003d url"},{"line_number":698,"context_line":"        if self.pool_url not in CONNECTION_POOLS:"},{"line_number":699,"context_line":"            CONNECTION_POOLS[self.pool_url] \u003d ldappool.ConnectionManager("},{"line_number":700,"context_line":"                url,"},{"line_number":701,"context_line":"                size\u003dldap_pool_size,"},{"line_number":702,"context_line":"                retry_max\u003dpool_retry_max,"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_d42e1842","line":699,"in_reply_to":"baada198_f807c21a","updated":"2014-07-22 16:31:21.000000000","message":"Handlers are instances which are created per api (identity, assignment) call so they would not remain in scope after its use. We need reference to just created connection manager for subsequent re-use. So I think it needs to be in global scope.  We need uri to allow multiple pool with different characteristics.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ed819419b7e9fd513d9e0070227a2f7d43e07b5d","unresolved":false,"context_lines":[{"line_number":729,"context_line":"        return CONNECTION_POOLS[self.pool_url].connection(self.who, self.cred)"},{"line_number":730,"context_line":""},{"line_number":731,"context_line":"    @use_conn_pool"},{"line_number":732,"context_line":"    def simple_bind_s(self, conn, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":733,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":734,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":735,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_980fa638","line":732,"updated":"2014-07-22 04:49:15.000000000","message":"I don\u0027t like forcing all arguments to be keyword here. How about removing kwargs parsing from use_conn_pool decorator but making this method work without it like this:\n\n    self.who \u003d who\n    self.cred \u003d cred\n    with self._get_pool_connection() as conn:\n        self._apply_options(conn)","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"717ddf677042a2c0c7ea3e1207d30e61776bda8e","unresolved":false,"context_lines":[{"line_number":729,"context_line":"        return CONNECTION_POOLS[self.pool_url].connection(self.who, self.cred)"},{"line_number":730,"context_line":""},{"line_number":731,"context_line":"    @use_conn_pool"},{"line_number":732,"context_line":"    def simple_bind_s(self, conn, who\u003d\u0027\u0027, cred\u003d\u0027\u0027,"},{"line_number":733,"context_line":"                      serverctrls\u003dNone, clientctrls\u003dNone):"},{"line_number":734,"context_line":"        # if everything is cool, lets save the creds for the the rest"},{"line_number":735,"context_line":"        # of the operations"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_37acb6ee","line":732,"in_reply_to":"baada198_980fa638","updated":"2014-07-22 16:31:21.000000000","message":"Okay. Will make the change if this is preferred over using common approach.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ed819419b7e9fd513d9e0070227a2f7d43e07b5d","unresolved":false,"context_lines":[{"line_number":1109,"context_line":"        self.pool_conn_lifetime \u003d conf.ldap.pool_connection_lifetime"},{"line_number":1110,"context_line":""},{"line_number":1111,"context_line":"        # End user authentication pool specific config attributes"},{"line_number":1112,"context_line":"        self.use_auth_pool \u003d conf.ldap.use_auth_pool"},{"line_number":1113,"context_line":"        self.auth_ldap_pool_size \u003d conf.ldap.auth_pool_size"},{"line_number":1114,"context_line":"        self.auth_pool_conn_lifetime \u003d conf.ldap.auth_pool_connection_lifetime"},{"line_number":1115,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_5e51770c","line":1112,"updated":"2014-07-22 04:49:15.000000000","message":"You should make it depend on use_pool setting:\n\n    self.use_auth_pool \u003d self.use_pool and conf.ldap.use_auth_pool\n\nOr change help text for use_auth_pool option.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"717ddf677042a2c0c7ea3e1207d30e61776bda8e","unresolved":false,"context_lines":[{"line_number":1109,"context_line":"        self.pool_conn_lifetime \u003d conf.ldap.pool_connection_lifetime"},{"line_number":1110,"context_line":""},{"line_number":1111,"context_line":"        # End user authentication pool specific config attributes"},{"line_number":1112,"context_line":"        self.use_auth_pool \u003d conf.ldap.use_auth_pool"},{"line_number":1113,"context_line":"        self.auth_ldap_pool_size \u003d conf.ldap.auth_pool_size"},{"line_number":1114,"context_line":"        self.auth_pool_conn_lifetime \u003d conf.ldap.auth_pool_connection_lifetime"},{"line_number":1115,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_54768840","line":1112,"in_reply_to":"baada198_5e51770c","updated":"2014-07-22 16:31:21.000000000","message":"Okay will make and condition","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"fba0e6c3612bdb949698a970a4d9beed4c05979f","unresolved":false,"context_lines":[{"line_number":1347,"context_line":"            if k \u003d\u003d \u0027id\u0027 or k in self.attribute_ignore:"},{"line_number":1348,"context_line":"                continue"},{"line_number":1349,"context_line":"            if v is not None:"},{"line_number":1350,"context_line":"                attr_type \u003d self.attribute_mapping.get(k)"},{"line_number":1351,"context_line":"                if attr_type is not None:"},{"line_number":1352,"context_line":"                    attrs.append((attr_type, [v]))"},{"line_number":1353,"context_line":"                extra_attrs \u003d [attr for attr, name"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_d4ad2d86","line":1350,"updated":"2014-07-22 20:34:01.000000000","message":"I don\u0027t think this is relevant. Please revert this line.","commit_id":"de371e1370acdc1f302ff8822c69d68732070210"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"c25d7377820b0680b9e71f92f9ef4ea144985614","unresolved":false,"context_lines":[{"line_number":1347,"context_line":"            if k \u003d\u003d \u0027id\u0027 or k in self.attribute_ignore:"},{"line_number":1348,"context_line":"                continue"},{"line_number":1349,"context_line":"            if v is not None:"},{"line_number":1350,"context_line":"                attr_type \u003d self.attribute_mapping.get(k)"},{"line_number":1351,"context_line":"                if attr_type is not None:"},{"line_number":1352,"context_line":"                    attrs.append((attr_type, [v]))"},{"line_number":1353,"context_line":"                extra_attrs \u003d [attr for attr, name"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_ac3b0dc7","line":1350,"in_reply_to":"baada198_d4ad2d86","updated":"2014-07-22 23:48:48.000000000","message":"Will revert this in next patch.\n\nI have opened a bug for related livetest failures.\nhttps://bugs.launchpad.net/keystone/+bug/1347262","commit_id":"de371e1370acdc1f302ff8822c69d68732070210"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ff3f58ea49f078799ef83e4234bf358a6f615dcf","unresolved":false,"context_lines":[{"line_number":652,"context_line":"    Connector \u003d ldappool.StateConnector"},{"line_number":653,"context_line":"    auth_pool_prefix \u003d \u0027auth_pool_\u0027"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"    Connection_pools \u003d {}  # static connector pool dict"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"    def __init__(self, conn\u003dNone, use_auth_pool\u003dFalse):"},{"line_number":658,"context_line":"        super(PooledLDAPHandler, self).__init__(conn\u003dconn)"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_48c5d208","line":655,"range":{"start_line":655,"start_character":4,"end_line":655,"end_character":20},"updated":"2014-07-23 18:32:43.000000000","message":"lowercase should be used for variable names, capitalized names are for classes.","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"a1af61c48770598db8d081b7cf33e6da5430827b","unresolved":false,"context_lines":[{"line_number":652,"context_line":"    Connector \u003d ldappool.StateConnector"},{"line_number":653,"context_line":"    auth_pool_prefix \u003d \u0027auth_pool_\u0027"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":"    Connection_pools \u003d {}  # static connector pool dict"},{"line_number":656,"context_line":""},{"line_number":657,"context_line":"    def __init__(self, conn\u003dNone, use_auth_pool\u003dFalse):"},{"line_number":658,"context_line":"        super(PooledLDAPHandler, self).__init__(conn\u003dconn)"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_fe5f3c0d","line":655,"range":{"start_line":655,"start_character":4,"end_line":655,"end_character":20},"in_reply_to":"baada198_48c5d208","updated":"2014-07-23 19:14:10.000000000","message":"Okay. Will make the change.","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ff3f58ea49f078799ef83e4234bf358a6f615dcf","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        self.who \u003d \u0027\u0027"},{"line_number":660,"context_line":"        self.cred \u003d \u0027\u0027"},{"line_number":661,"context_line":"        self.conn_options \u003d {}  # connection specific options"},{"line_number":662,"context_line":"        self.url \u003d None"},{"line_number":663,"context_line":"        self.page_size \u003d None"},{"line_number":664,"context_line":"        self.use_auth_pool \u003d use_auth_pool"},{"line_number":665,"context_line":"        self.conn_pool \u003d None"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_c822c2b9","line":662,"updated":"2014-07-23 18:32:43.000000000","message":"I don\u0027t think self.url is used anywhere anymore","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"a1af61c48770598db8d081b7cf33e6da5430827b","unresolved":false,"context_lines":[{"line_number":659,"context_line":"        self.who \u003d \u0027\u0027"},{"line_number":660,"context_line":"        self.cred \u003d \u0027\u0027"},{"line_number":661,"context_line":"        self.conn_options \u003d {}  # connection specific options"},{"line_number":662,"context_line":"        self.url \u003d None"},{"line_number":663,"context_line":"        self.page_size \u003d None"},{"line_number":664,"context_line":"        self.use_auth_pool \u003d use_auth_pool"},{"line_number":665,"context_line":"        self.conn_pool \u003d None"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_5e83c854","line":662,"in_reply_to":"baada198_c822c2b9","updated":"2014-07-23 19:14:10.000000000","message":"Yes, it seems like leftover from previous changes. Will make the change.","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ff3f58ea49f078799ef83e4234bf358a6f615dcf","unresolved":false,"context_lines":[{"line_number":703,"context_line":"                connector_cls\u003dself.Connector,"},{"line_number":704,"context_line":"                use_tls\u003duse_tls,"},{"line_number":705,"context_line":"                max_lifetime\u003dpool_conn_lifetime)"},{"line_number":706,"context_line":"            self.Connection_pools[pool_url] \u003d self.conn_pool"},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"    def set_option(self, option, invalue):"},{"line_number":709,"context_line":"        self.conn_options[option] \u003d invalue"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_c85ba253","line":706,"updated":"2014-07-23 18:32:43.000000000","message":"This is fine, but I\u0027d prefer following the \"Don\u0027t ask for permission, ask for forgiveness\" way:\n\n  try:\n    self.conn_pool \u003d self.connection_pools[pool_url]\n  except KeyError:\n    self.conn_pool \u003d \u003c...\u003e\n    self.connection_pools[pool_url] \u003d self.conn_pool","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"a1af61c48770598db8d081b7cf33e6da5430827b","unresolved":false,"context_lines":[{"line_number":703,"context_line":"                connector_cls\u003dself.Connector,"},{"line_number":704,"context_line":"                use_tls\u003duse_tls,"},{"line_number":705,"context_line":"                max_lifetime\u003dpool_conn_lifetime)"},{"line_number":706,"context_line":"            self.Connection_pools[pool_url] \u003d self.conn_pool"},{"line_number":707,"context_line":""},{"line_number":708,"context_line":"    def set_option(self, option, invalue):"},{"line_number":709,"context_line":"        self.conn_options[option] \u003d invalue"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_fe9c9c6c","line":706,"in_reply_to":"baada198_c85ba253","updated":"2014-07-23 19:14:10.000000000","message":"I will make the change. In general, I avoid try-except for flow control but here its okay.","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"}],"keystone/tests/fakeldap.py":[{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":21,"context_line":"library to work with nova."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"\"\"\""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"import re"},{"line_number":26,"context_line":"import shelve"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_f1210a9f","side":"PARENT","line":24,"updated":"2014-07-18 10:30:58.000000000","message":"Why remove this empty line? It\u0027s cool, leave it :)","commit_id":"f3dcd05b3d0b0ffc925e8ab29f90465d9488f0b3"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":21,"context_line":"library to work with nova."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"\"\"\""},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"import re"},{"line_number":26,"context_line":"import shelve"},{"line_number":27,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_aea3a141","side":"PARENT","line":24,"in_reply_to":"baada198_f1210a9f","updated":"2014-07-19 01:36:58.000000000","message":"Addressing in next patch.","commit_id":"f3dcd05b3d0b0ffc925e8ab29f90465d9488f0b3"}],"keystone/tests/test_backend_ldap.py":[{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":1259,"context_line":"        # email as default. For ldap live test, its mail"},{"line_number":1260,"context_line":"        attr \u003d CONF.ldap.user_mail_attribute"},{"line_number":1261,"context_line":"        mixed_attr \u003d \u0027\u0027.join(x.upper() if i % 2 \u003d\u003d 0 else x.lower() for i, x"},{"line_number":1262,"context_line":"                             in enumerate(attr))  # EmAiL"},{"line_number":1263,"context_line":"        mock_ldap_get.return_value \u003d ("},{"line_number":1264,"context_line":"            \u0027cn\u003djunk,dc\u003dexample,dc\u003dcom\u0027,"},{"line_number":1265,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_1cdbf5ed","line":1262,"updated":"2014-07-18 10:30:58.000000000","message":"Are these changes relevant to the pooling patch?","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":1259,"context_line":"        # email as default. For ldap live test, its mail"},{"line_number":1260,"context_line":"        attr \u003d CONF.ldap.user_mail_attribute"},{"line_number":1261,"context_line":"        mixed_attr \u003d \u0027\u0027.join(x.upper() if i % 2 \u003d\u003d 0 else x.lower() for i, x"},{"line_number":1262,"context_line":"                             in enumerate(attr))  # EmAiL"},{"line_number":1263,"context_line":"        mock_ldap_get.return_value \u003d ("},{"line_number":1264,"context_line":"            \u0027cn\u003djunk,dc\u003dexample,dc\u003dcom\u0027,"},{"line_number":1265,"context_line":"            {"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_6e2269ba","line":1262,"in_reply_to":"baada198_1cdbf5ed","updated":"2014-07-19 01:36:58.000000000","message":"Existing ldap live tests are broken in master. When I run them against local instance of openLdap, there are number of failures. So I am going to add defect against them and also will address them in separate branch. \n\nThis is one of the needed change, so I can run using these tests.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"fba0e6c3612bdb949698a970a4d9beed4c05979f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"baada198_d4e52da5","updated":"2014-07-22 20:34:01.000000000","message":"Changes in this module seem to be unrelated.","commit_id":"de371e1370acdc1f302ff8822c69d68732070210"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"c25d7377820b0680b9e71f92f9ef4ea144985614","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":19,"id":"baada198_6c9c955e","in_reply_to":"baada198_d4e52da5","updated":"2014-07-22 23:48:48.000000000","message":"Will revert the changes in next patch.\n\nOpened bug for livetets failures: https://bugs.launchpad.net/keystone/+bug/1347262","commit_id":"de371e1370acdc1f302ff8822c69d68732070210"}],"keystone/tests/test_backend_ldap_pool.py":[{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    sure it works without any error."},{"line_number":32,"context_line":"    \u0027\u0027\u0027"},{"line_number":33,"context_line":"    def setUp(self):"},{"line_number":34,"context_line":"        ldap_core.PooledLDAPHandler.CONNECTOR_CLS \u003d fakeldap.FakeLdapPool"},{"line_number":35,"context_line":"        super(LdapIdentitySqlAssignment, self).setUp()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def config_files(self):"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_7cf5d14f","line":34,"updated":"2014-07-18 10:30:58.000000000","message":"You should use mock.patch here instead.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    sure it works without any error."},{"line_number":32,"context_line":"    \u0027\u0027\u0027"},{"line_number":33,"context_line":"    def setUp(self):"},{"line_number":34,"context_line":"        ldap_core.PooledLDAPHandler.CONNECTOR_CLS \u003d fakeldap.FakeLdapPool"},{"line_number":35,"context_line":"        super(LdapIdentitySqlAssignment, self).setUp()"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def config_files(self):"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_8aa6bb0d","line":34,"in_reply_to":"baada198_7cf5d14f","updated":"2014-07-19 01:36:58.000000000","message":"Thanks for the idea. Will address in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        return config_files"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def test_pooled_handler_is_used(self):"},{"line_number":43,"context_line":"        user_ref \u003d self.identity_api.get_user(self.user_foo[\u0027id\u0027])"},{"line_number":44,"context_line":"        self.user_foo.pop(\u0027password\u0027)"},{"line_number":45,"context_line":"        self.assertDictEqual(user_ref, self.user_foo)"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_7c7a31bb","line":43,"updated":"2014-07-18 10:30:58.000000000","message":"I think you should move this call to setUp (or even setUpClass).","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":40,"context_line":"        return config_files"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"    def test_pooled_handler_is_used(self):"},{"line_number":43,"context_line":"        user_ref \u003d self.identity_api.get_user(self.user_foo[\u0027id\u0027])"},{"line_number":44,"context_line":"        self.user_foo.pop(\u0027password\u0027)"},{"line_number":45,"context_line":"        self.assertDictEqual(user_ref, self.user_foo)"},{"line_number":46,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_57a7f10a","line":43,"in_reply_to":"baada198_7c7a31bb","updated":"2014-07-19 01:36:58.000000000","message":"Will address in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        who \u003d CONF.ldap.user"},{"line_number":111,"context_line":"        cred \u003d CONF.ldap.password"},{"line_number":112,"context_line":"        # get related connection manager instance"},{"line_number":113,"context_line":"        ldappool_cm \u003d ldap_core.CONNECTION_POOLS[CONF.ldap.url]"},{"line_number":114,"context_line":"        ldappool_cm.size \u003d 2"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        # 3rd connection attempt should raise Max connection error"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_bc6849f6","line":113,"updated":"2014-07-18 10:30:58.000000000","message":"This test would fail if run in isolation.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":110,"context_line":"        who \u003d CONF.ldap.user"},{"line_number":111,"context_line":"        cred \u003d CONF.ldap.password"},{"line_number":112,"context_line":"        # get related connection manager instance"},{"line_number":113,"context_line":"        ldappool_cm \u003d ldap_core.CONNECTION_POOLS[CONF.ldap.url]"},{"line_number":114,"context_line":"        ldappool_cm.size \u003d 2"},{"line_number":115,"context_line":""},{"line_number":116,"context_line":"        # 3rd connection attempt should raise Max connection error"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_74e4f60c","line":113,"in_reply_to":"baada198_bc6849f6","updated":"2014-07-19 01:36:58.000000000","message":"I can see why this will appear that way.\n\nActually this runs fine in isolation as initial setup loads default fixtures which creates user etc so CONNECTION_POOLS is initialized before entering test method. \n\nI like your earlier idea of adding pool initialization check call (if not there and to highlight pool initialization is already done) in setUp to avoid this confusion.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"class LdapIdentitySqlAssignment(test_backend_ldap.LdapIdentitySqlAssignment,"},{"line_number":110,"context_line":"                                tests.TestCase, LdapPoolCommonTestMixin):"},{"line_number":111,"context_line":"    \u0027\u0027\u0027Executes existing base class 150+ tests with pooled ldap handler to make"},{"line_number":112,"context_line":"    sure it works without any error."},{"line_number":113,"context_line":"    \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_febff5a8","line":110,"range":{"start_line":110,"start_character":48,"end_line":110,"end_character":71},"updated":"2014-07-19 09:04:51.000000000","message":"You should put mixin to the very first place in the list of parents. Only this way you can guarantee it overrides all methods properly.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":107,"context_line":""},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"class LdapIdentitySqlAssignment(test_backend_ldap.LdapIdentitySqlAssignment,"},{"line_number":110,"context_line":"                                tests.TestCase, LdapPoolCommonTestMixin):"},{"line_number":111,"context_line":"    \u0027\u0027\u0027Executes existing base class 150+ tests with pooled ldap handler to make"},{"line_number":112,"context_line":"    sure it works without any error."},{"line_number":113,"context_line":"    \u0027\u0027\u0027"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_ce01a008","line":110,"range":{"start_line":110,"start_character":48,"end_line":110,"end_character":71},"in_reply_to":"baada198_febff5a8","updated":"2014-07-19 20:13:40.000000000","message":"Will address in next patch","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        config_files.append(tests.dirs.tests_conf(\u0027backend_ldap_pool.conf\u0027))"},{"line_number":128,"context_line":"        return config_files"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def tearDown(self):"},{"line_number":131,"context_line":"        # remove pool changes in a test if any"},{"line_number":132,"context_line":"        ldap_core.CONNECTION_POOLS.pop(CONF.ldap.url, None)"},{"line_number":133,"context_line":"        super(LdapIdentitySqlAssignment, self).tearDown()"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_1ebd89b1","line":130,"range":{"start_line":130,"start_character":8,"end_line":130,"end_character":16},"updated":"2014-07-19 09:04:51.000000000","message":"You should use addCleanup in setUp instead of adding tearDown. For example, tearDown won\u0027t get executed if smth fails in setUp.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        config_files.append(tests.dirs.tests_conf(\u0027backend_ldap_pool.conf\u0027))"},{"line_number":128,"context_line":"        return config_files"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def tearDown(self):"},{"line_number":131,"context_line":"        # remove pool changes in a test if any"},{"line_number":132,"context_line":"        ldap_core.CONNECTION_POOLS.pop(CONF.ldap.url, None)"},{"line_number":133,"context_line":"        super(LdapIdentitySqlAssignment, self).tearDown()"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_6121859d","line":130,"range":{"start_line":130,"start_character":8,"end_line":130,"end_character":16},"in_reply_to":"baada198_1ebd89b1","updated":"2014-07-19 20:13:40.000000000","message":"If test logic is not executed, then no cleanup is needed in this case. But I will move it to setUp as addCleanup.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"1e42f11ded6bcb1abef6a419b6bdc47ae2203e39","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        config_files.append(tests.dirs.tests_conf(\u0027backend_ldap_pool.conf\u0027))"},{"line_number":128,"context_line":"        return config_files"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def tearDown(self):"},{"line_number":131,"context_line":"        # remove pool changes in a test if any"},{"line_number":132,"context_line":"        ldap_core.CONNECTION_POOLS.pop(CONF.ldap.url, None)"},{"line_number":133,"context_line":"        super(LdapIdentitySqlAssignment, self).tearDown()"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_9cab3ad5","line":130,"range":{"start_line":130,"start_character":8,"end_line":130,"end_character":16},"in_reply_to":"baada198_3058a030","updated":"2014-07-21 17:39:55.000000000","message":"cleanup is needed only for changes made in test logic. If there are no changes, then no cleanup is needed as setup is executed for each test method execution so would not make difference without test method execution.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"6fe21f56d2dd400976a72e308239061942949f24","unresolved":false,"context_lines":[{"line_number":127,"context_line":"        config_files.append(tests.dirs.tests_conf(\u0027backend_ldap_pool.conf\u0027))"},{"line_number":128,"context_line":"        return config_files"},{"line_number":129,"context_line":""},{"line_number":130,"context_line":"    def tearDown(self):"},{"line_number":131,"context_line":"        # remove pool changes in a test if any"},{"line_number":132,"context_line":"        ldap_core.CONNECTION_POOLS.pop(CONF.ldap.url, None)"},{"line_number":133,"context_line":"        super(LdapIdentitySqlAssignment, self).tearDown()"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_3058a030","line":130,"range":{"start_line":130,"start_character":8,"end_line":130,"end_character":16},"in_reply_to":"baada198_6121859d","updated":"2014-07-21 15:19:36.000000000","message":"As you said, some objects are created in setUp and so pool will be initialized.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        return config_files"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    @mock.patch.object(ldap_core, \u0027utf8_encode\u0027)"},{"line_number":134,"context_line":"    def test_utf8_encoded_is_used_in_pool(self, mocked_method):"},{"line_number":135,"context_line":"        def side_effect(arg):"},{"line_number":136,"context_line":"            return arg"},{"line_number":137,"context_line":"        mocked_method.side_effect \u003d side_effect"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_1de2a89f","line":134,"updated":"2014-07-22 03:44:29.000000000","message":"There\u0027s no need for this test.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"47628ec559a66373fffd48a5047f0af39fb887d4","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        return config_files"},{"line_number":132,"context_line":""},{"line_number":133,"context_line":"    @mock.patch.object(ldap_core, \u0027utf8_encode\u0027)"},{"line_number":134,"context_line":"    def test_utf8_encoded_is_used_in_pool(self, mocked_method):"},{"line_number":135,"context_line":"        def side_effect(arg):"},{"line_number":136,"context_line":"            return arg"},{"line_number":137,"context_line":"        mocked_method.side_effect \u003d side_effect"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_38121afd","line":134,"in_reply_to":"baada198_1de2a89f","updated":"2014-07-22 04:13:33.000000000","message":"Added just to address the previous concern raised around this.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"d749d030b74ccea95eefb15180f6baa98e89b9da","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        mocked_method.assert_any_call(CONF.ldap.user)"},{"line_number":140,"context_line":"        mocked_method.assert_any_call(CONF.ldap.password)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def test_pool_size_expands_correctly(self):"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        who \u003d CONF.ldap.user"},{"line_number":145,"context_line":"        cred \u003d CONF.ldap.password"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_9de5d8a5","line":142,"updated":"2014-07-22 03:44:29.000000000","message":"This seems to be a test for ldappool, not backend. I don\u0027t think it\u0027s needed here.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"47628ec559a66373fffd48a5047f0af39fb887d4","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        mocked_method.assert_any_call(CONF.ldap.user)"},{"line_number":140,"context_line":"        mocked_method.assert_any_call(CONF.ldap.password)"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"    def test_pool_size_expands_correctly(self):"},{"line_number":143,"context_line":""},{"line_number":144,"context_line":"        who \u003d CONF.ldap.user"},{"line_number":145,"context_line":"        cred \u003d CONF.ldap.password"}],"source_content_type":"text/x-python","patch_set":16,"id":"baada198_1800f6a9","line":142,"in_reply_to":"baada198_9de5d8a5","updated":"2014-07-22 04:13:33.000000000","message":"Its for pool but behavior is slightly different in live ldap test where pool StateConnector logic maintains active and connected flag correctly. In FakeLdapool, its not maintained and that\u0027s why decided to have separate impl in specific tests instead of having in mixin.","commit_id":"e674eebd46ff0c24c26a3337135f2239a92bc4f5"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ed819419b7e9fd513d9e0070227a2f7d43e07b5d","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def cleanup_pools(self):"},{"line_number":34,"context_line":"        for key in ldap_core.CONNECTION_POOLS.keys():"},{"line_number":35,"context_line":"            ldap_core.CONNECTION_POOLS.pop(key)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def test_handler_with_use_pool_enabled(self):"},{"line_number":38,"context_line":"        # by default use_pool and use_auth_pool is enabled in test pool config"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_d8775ed2","line":35,"updated":"2014-07-22 04:49:15.000000000","message":"You can just do\n\n  ldap_core.CONNECTION_POOLS.clear()\n\nhere.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"717ddf677042a2c0c7ea3e1207d30e61776bda8e","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def cleanup_pools(self):"},{"line_number":34,"context_line":"        for key in ldap_core.CONNECTION_POOLS.keys():"},{"line_number":35,"context_line":"            ldap_core.CONNECTION_POOLS.pop(key)"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":"    def test_handler_with_use_pool_enabled(self):"},{"line_number":38,"context_line":"        # by default use_pool and use_auth_pool is enabled in test pool config"}],"source_content_type":"text/x-python","patch_set":17,"id":"baada198_f73ebe8a","line":35,"in_reply_to":"baada198_d8775ed2","updated":"2014-07-22 16:31:21.000000000","message":"Okay. Will make the change.","commit_id":"c06f3ecd18f9d811097cbc93f33ecb527203d38e"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ff3f58ea49f078799ef83e4234bf358a6f615dcf","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def test_pool_retry_max_set(self):"},{"line_number":87,"context_line":"        # get related connection manager instance"},{"line_number":88,"context_line":"        ldappool_cm \u003d ldap_core.PooledLDAPHandler.\\"},{"line_number":89,"context_line":"            Connection_pools[CONF.ldap.url]"},{"line_number":90,"context_line":"        self.assertEqual(CONF.ldap.pool_retry_max, ldappool_cm.retry_max)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def test_pool_retry_delay_set(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_a3420f0d","line":89,"range":{"start_line":88,"start_character":22,"end_line":89,"end_character":28},"updated":"2014-07-23 18:32:43.000000000","message":"Just to calm backslash-haters I\u0027d store this dict in self in setUp and reference it everywhere.","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"a1af61c48770598db8d081b7cf33e6da5430827b","unresolved":false,"context_lines":[{"line_number":85,"context_line":""},{"line_number":86,"context_line":"    def test_pool_retry_max_set(self):"},{"line_number":87,"context_line":"        # get related connection manager instance"},{"line_number":88,"context_line":"        ldappool_cm \u003d ldap_core.PooledLDAPHandler.\\"},{"line_number":89,"context_line":"            Connection_pools[CONF.ldap.url]"},{"line_number":90,"context_line":"        self.assertEqual(CONF.ldap.pool_retry_max, ldappool_cm.retry_max)"},{"line_number":91,"context_line":""},{"line_number":92,"context_line":"    def test_pool_retry_delay_set(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_be794432","line":89,"range":{"start_line":88,"start_character":22,"end_line":89,"end_character":28},"in_reply_to":"baada198_a3420f0d","updated":"2014-07-23 19:14:10.000000000","message":"I will make the change.","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"}],"keystone/tests/test_ldap_pool_livetest.py":[{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":26,"context_line":"CONF \u003d config.CONF"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class LiveLDAPPoolIdentity(test_ldap_livetest.LiveLDAPIdentity):"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def setUp(self):"},{"line_number":32,"context_line":"        self._ldap_skip_live()"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_97743e9d","line":29,"updated":"2014-07-18 10:30:58.000000000","message":"From the looks of it you should inherit from test_backend_ldap_pool.LdapIdentitySqlAssignment or some mixin to avoid code repetition here. Most of the tests are the same as in that class.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":26,"context_line":"CONF \u003d config.CONF"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class LiveLDAPPoolIdentity(test_ldap_livetest.LiveLDAPIdentity):"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def setUp(self):"},{"line_number":32,"context_line":"        self._ldap_skip_live()"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_095487fc","line":29,"in_reply_to":"baada198_97743e9d","updated":"2014-07-19 01:36:58.000000000","message":"Good idea. Will address in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":33,"context_line":"        super(LiveLDAPPoolIdentity, self).setUp()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def _ldap_skip_live(self):"},{"line_number":36,"context_line":"            self.skip_if_env_not_set(\u0027ENABLE_LDAP_LIVE_TEST\u0027)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def config_files(self):"},{"line_number":39,"context_line":"        config_files \u003d super(LiveLDAPPoolIdentity, self).config_files()"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_bc4f6991","line":36,"updated":"2014-07-18 10:30:58.000000000","message":"Extra indent","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class LiveLDAPPoolIdentity(test_ldap_livetest.LiveLDAPIdentity):"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def setUp(self):"},{"line_number":32,"context_line":"        self._ldap_skip_live()"},{"line_number":33,"context_line":"        super(LiveLDAPPoolIdentity, self).setUp()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def _ldap_skip_live(self):"},{"line_number":36,"context_line":"            self.skip_if_env_not_set(\u0027ENABLE_LDAP_LIVE_TEST\u0027)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def config_files(self):"},{"line_number":39,"context_line":"        config_files \u003d super(LiveLDAPPoolIdentity, self).config_files()"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_1c25b5c1","line":36,"range":{"start_line":31,"start_character":0,"end_line":36,"end_character":61},"updated":"2014-07-18 10:30:58.000000000","message":"setUp and _ldap_skip_live methods are the same as in parent class. You should just remove them here.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":28,"context_line":""},{"line_number":29,"context_line":"class LiveLDAPPoolIdentity(test_ldap_livetest.LiveLDAPIdentity):"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    def setUp(self):"},{"line_number":32,"context_line":"        self._ldap_skip_live()"},{"line_number":33,"context_line":"        super(LiveLDAPPoolIdentity, self).setUp()"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    def _ldap_skip_live(self):"},{"line_number":36,"context_line":"            self.skip_if_env_not_set(\u0027ENABLE_LDAP_LIVE_TEST\u0027)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"    def config_files(self):"},{"line_number":39,"context_line":"        config_files \u003d super(LiveLDAPPoolIdentity, self).config_files()"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_c9744f9e","line":36,"range":{"start_line":31,"start_character":0,"end_line":36,"end_character":61},"in_reply_to":"baada198_1c25b5c1","updated":"2014-07-19 01:36:58.000000000","message":"Will address both in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"b776a49f6c6b14856db3026142e469caa817e20c","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    def tearDown(self):"},{"line_number":51,"context_line":"        # remove pool changes in a test if any"},{"line_number":52,"context_line":"        ldap_core.CONNECTION_POOLS.pop(CONF.ldap.url, None)"},{"line_number":53,"context_line":"        tests.TestCase.tearDown(self)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def test_pool_handler_is_instantiated(self):"},{"line_number":56,"context_line":"        self.identity_api.get_user(self.user_foo[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_7cff9102","line":53,"updated":"2014-07-18 10:30:58.000000000","message":"You should use super() here.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"10e07de93d4065926930147a03699537de729289","unresolved":false,"context_lines":[{"line_number":50,"context_line":"    def tearDown(self):"},{"line_number":51,"context_line":"        # remove pool changes in a test if any"},{"line_number":52,"context_line":"        ldap_core.CONNECTION_POOLS.pop(CONF.ldap.url, None)"},{"line_number":53,"context_line":"        tests.TestCase.tearDown(self)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def test_pool_handler_is_instantiated(self):"},{"line_number":56,"context_line":"        self.identity_api.get_user(self.user_foo[\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":13,"id":"baada198_29b22be5","line":53,"in_reply_to":"baada198_7cff9102","updated":"2014-07-19 01:36:58.000000000","message":"Will address in next patch.","commit_id":"03a7f1820d0972b96513ffe88f93a0eee63e6325"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class LiveLDAPPoolIdentity(test_ldap_livetest.LiveLDAPIdentity,"},{"line_number":29,"context_line":"                           test_backend_ldap_pool.LdapPoolCommonTestMixin):"},{"line_number":30,"context_line":"    \"\"\"Executes existing ldap live test with pooled ldap handler to make"},{"line_number":31,"context_line":"    sure it works without any error."},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_7eacc588","line":29,"range":{"start_line":29,"start_character":50,"end_line":29,"end_character":73},"updated":"2014-07-19 09:04:51.000000000","message":"You should move mixin to the front of this list.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":26,"context_line":""},{"line_number":27,"context_line":""},{"line_number":28,"context_line":"class LiveLDAPPoolIdentity(test_ldap_livetest.LiveLDAPIdentity,"},{"line_number":29,"context_line":"                           test_backend_ldap_pool.LdapPoolCommonTestMixin):"},{"line_number":30,"context_line":"    \"\"\"Executes existing ldap live test with pooled ldap handler to make"},{"line_number":31,"context_line":"    sure it works without any error."},{"line_number":32,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_c12fb187","line":29,"range":{"start_line":29,"start_character":50,"end_line":29,"end_character":73},"in_reply_to":"baada198_7eacc588","updated":"2014-07-19 20:13:40.000000000","message":"Will address in next patch.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    \"\"\""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def setUp(self):"},{"line_number":37,"context_line":"        self._ldap_skip_live()"},{"line_number":38,"context_line":"        super(LiveLDAPPoolIdentity, self).setUp()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def config_files(self):"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_9eb199e2","line":37,"range":{"start_line":37,"start_character":8,"end_line":37,"end_character":30},"updated":"2014-07-19 09:04:51.000000000","message":"This method is already called in the parent class.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":34,"context_line":"    \"\"\""},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    def setUp(self):"},{"line_number":37,"context_line":"        self._ldap_skip_live()"},{"line_number":38,"context_line":"        super(LiveLDAPPoolIdentity, self).setUp()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":"    def config_files(self):"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_813939da","line":37,"range":{"start_line":37,"start_character":8,"end_line":37,"end_character":30},"in_reply_to":"baada198_9eb199e2","updated":"2014-07-19 20:13:40.000000000","message":"Yes..removed it.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"0a6b03b9ea6764073aace8cd61b1699625ab86a0","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            group\u003d\u0027identity\u0027,"},{"line_number":50,"context_line":"            driver\u003d\u0027keystone.identity.backends.ldap.Identity\u0027)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def tearDown(self):"},{"line_number":53,"context_line":"        # remove pool changes in a test if any"},{"line_number":54,"context_line":"        ldap_core.CONNECTION_POOLS.pop(CONF.ldap.url, None)"},{"line_number":55,"context_line":"        super(LiveLDAPPoolIdentity, self).tearDown()"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_dec6b148","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":16},"updated":"2014-07-19 09:04:51.000000000","message":"You should use addCleanup in setUp instead of tearDown","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"24a063f037259d9056f7dd225fc364b9ee6d21b4","unresolved":false,"context_lines":[{"line_number":49,"context_line":"            group\u003d\u0027identity\u0027,"},{"line_number":50,"context_line":"            driver\u003d\u0027keystone.identity.backends.ldap.Identity\u0027)"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def tearDown(self):"},{"line_number":53,"context_line":"        # remove pool changes in a test if any"},{"line_number":54,"context_line":"        ldap_core.CONNECTION_POOLS.pop(CONF.ldap.url, None)"},{"line_number":55,"context_line":"        super(LiveLDAPPoolIdentity, self).tearDown()"}],"source_content_type":"text/x-python","patch_set":15,"id":"baada198_a1363de5","line":52,"range":{"start_line":52,"start_character":8,"end_line":52,"end_character":16},"in_reply_to":"baada198_dec6b148","updated":"2014-07-19 20:13:40.000000000","message":"Will address in next patch.","commit_id":"827fcf33a6213b6076fba4032c20e939379998d7"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"fba0e6c3612bdb949698a970a4d9beed4c05979f","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.config_fixture.config(group\u003d\u0027ldap\u0027, page_size\u003d1)"},{"line_number":63,"context_line":"        self.test_user_enable_attribute_mask()"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def test_pool_size_expands_correctly(self):"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        who \u003d CONF.ldap.user"},{"line_number":68,"context_line":"        cred \u003d CONF.ldap.password"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_2fac56ee","line":65,"updated":"2014-07-22 20:34:01.000000000","message":"This method should be unified with the (almost) same one from test_backend_ldap_pool.py.","commit_id":"de371e1370acdc1f302ff8822c69d68732070210"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"c25d7377820b0680b9e71f92f9ef4ea144985614","unresolved":false,"context_lines":[{"line_number":62,"context_line":"        self.config_fixture.config(group\u003d\u0027ldap\u0027, page_size\u003d1)"},{"line_number":63,"context_line":"        self.test_user_enable_attribute_mask()"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def test_pool_size_expands_correctly(self):"},{"line_number":66,"context_line":""},{"line_number":67,"context_line":"        who \u003d CONF.ldap.user"},{"line_number":68,"context_line":"        cred \u003d CONF.ldap.password"}],"source_content_type":"text/x-python","patch_set":19,"id":"baada198_aac4d40e","line":65,"in_reply_to":"baada198_2fac56ee","updated":"2014-07-22 23:48:48.000000000","message":"This test checks specifically if ldap connection is connected and has active status when in use. This kind of test cannot be done with FakeLdapPool.","commit_id":"de371e1370acdc1f302ff8822c69d68732070210"},{"author":{"_account_id":708,"name":"Yuriy Taraday","email":"yuriy@taraday.nl","username":"yorik-sar"},"change_message_id":"ff3f58ea49f078799ef83e4234bf358a6f615dcf","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        who \u003d CONF.ldap.user"},{"line_number":68,"context_line":"        cred \u003d CONF.ldap.password"},{"line_number":69,"context_line":"        # get related connection manager instance"},{"line_number":70,"context_line":"        ldappool_cm \u003d ldap_core.PooledLDAPHandler.\\"},{"line_number":71,"context_line":"            Connection_pools[CONF.ldap.url]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        def _get_conn():"},{"line_number":74,"context_line":"            return ldappool_cm.connection(who, cred)"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_630637cf","line":71,"range":{"start_line":70,"start_character":22,"end_line":71,"end_character":28},"updated":"2014-07-23 18:32:43.000000000","message":"See comment in test_backend_ldap_pool.","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"},{"author":{"_account_id":1091,"name":"Arun Kant","email":"arukant@cisco.com","username":"arunkant-uws"},"change_message_id":"a1af61c48770598db8d081b7cf33e6da5430827b","unresolved":false,"context_lines":[{"line_number":67,"context_line":"        who \u003d CONF.ldap.user"},{"line_number":68,"context_line":"        cred \u003d CONF.ldap.password"},{"line_number":69,"context_line":"        # get related connection manager instance"},{"line_number":70,"context_line":"        ldappool_cm \u003d ldap_core.PooledLDAPHandler.\\"},{"line_number":71,"context_line":"            Connection_pools[CONF.ldap.url]"},{"line_number":72,"context_line":""},{"line_number":73,"context_line":"        def _get_conn():"},{"line_number":74,"context_line":"            return ldappool_cm.connection(who, cred)"}],"source_content_type":"text/x-python","patch_set":21,"id":"baada198_5e890824","line":71,"range":{"start_line":70,"start_character":22,"end_line":71,"end_character":28},"in_reply_to":"baada198_630637cf","updated":"2014-07-23 19:14:10.000000000","message":"I will make the change.","commit_id":"4073726887687f9c27682091f45ca3f2bf4717b6"}],"requirements.txt":[{"author":{"_account_id":6482,"name":"Steve Martinelli","email":"s.martinelli@gmail.com","username":"stevemar"},"change_message_id":"aa9060f737d443cff0f1ee5c837344e49bed48d6","unresolved":false,"context_lines":[{"line_number":22,"context_line":"jsonschema\u003e\u003d2.0.0,\u003c3.0.0"},{"line_number":23,"context_line":"pycadf\u003e\u003d0.4.1"},{"line_number":24,"context_line":"posix_ipc"},{"line_number":25,"context_line":"ldappool\u003e\u003d1.0"}],"source_content_type":"text/plain","patch_set":2,"id":"3ae8d1ca_296d0d99","line":25,"updated":"2014-05-28 16:51:13.000000000","message":"this should probably go in test-requirements if it\u0027s optional","commit_id":"701fa08fc5b1e17eb1a5dcdb40b7e56cd523450f"}]}