)]}'
{"monasca_api/healthcheck/metrics_db_check.py":[{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"6f7265b3ffdacd33d474a9b88ca85ecedabf7c92","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        if db \u003d\u003d \u0027influxdb\u0027:"},{"line_number":58,"context_line":"            status \u003d self._check_influxdb_status()"},{"line_number":59,"context_line":"        else:"},{"line_number":60,"context_line":"            if not has_cassandra:"},{"line_number":61,"context_line":"                # Should never happen"},{"line_number":62,"context_line":"                status \u003d [False, \"Cassandra driver not imported\"]"},{"line_number":63,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_3a1e7dba","line":60,"updated":"2017-06-12 07:03:00.000000000","message":"oslo.utils are part of requirements here. That part of code can be written as\n\n   cluster \u003d oslo_utils.importutils.try_import(\u0027cassandra.cluster\u0027, False)\n   \nand rest of the code as followed with minor adjustments.","commit_id":"90a23bc77693c7d111e9352866c06d44507df9b3"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"69642d8147363860da228805c28625a52257831c","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        if db \u003d\u003d \u0027influxdb\u0027:"},{"line_number":58,"context_line":"            status \u003d self._check_influxdb_status()"},{"line_number":59,"context_line":"        else:"},{"line_number":60,"context_line":"            if not has_cassandra:"},{"line_number":61,"context_line":"                # Should never happen"},{"line_number":62,"context_line":"                status \u003d [False, \"Cassandra driver not imported\"]"},{"line_number":63,"context_line":"            else:"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_e85f0d3f","line":60,"in_reply_to":"7f231b9d_3a1e7dba","updated":"2017-06-12 14:05:58.000000000","message":"Good idea.","commit_id":"90a23bc77693c7d111e9352866c06d44507df9b3"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"6f7265b3ffdacd33d474a9b88ca85ecedabf7c92","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    @staticmethod"},{"line_number":91,"context_line":"    def _check_cassandra_status():"},{"line_number":92,"context_line":"        try:"},{"line_number":93,"context_line":"            cassandra \u003d cluster.Cluster("},{"line_number":94,"context_line":"                CONF.cassandra.cluster_ip_addresses.split(\u0027,\u0027)"},{"line_number":95,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_da82e9c8","line":92,"updated":"2017-06-12 07:03:00.000000000","message":"I\u0027d also put that here. Instead of the above.","commit_id":"90a23bc77693c7d111e9352866c06d44507df9b3"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"5360f6537ad96b0eadc7c1ff0d2ec886a1d6b8d0","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    @staticmethod"},{"line_number":91,"context_line":"    def _check_cassandra_status():"},{"line_number":92,"context_line":"        try:"},{"line_number":93,"context_line":"            cassandra \u003d cluster.Cluster("},{"line_number":94,"context_line":"                CONF.cassandra.cluster_ip_addresses.split(\u0027,\u0027)"},{"line_number":95,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_488a761b","line":92,"in_reply_to":"7f231b9d_681add7d","updated":"2017-06-12 18:45:36.000000000","message":"hmm...I think that\u0027s ok. Even if you install the requirement, I guess you will restart API anyway.","commit_id":"90a23bc77693c7d111e9352866c06d44507df9b3"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"69642d8147363860da228805c28625a52257831c","unresolved":false,"context_lines":[{"line_number":89,"context_line":""},{"line_number":90,"context_line":"    @staticmethod"},{"line_number":91,"context_line":"    def _check_cassandra_status():"},{"line_number":92,"context_line":"        try:"},{"line_number":93,"context_line":"            cassandra \u003d cluster.Cluster("},{"line_number":94,"context_line":"                CONF.cassandra.cluster_ip_addresses.split(\u0027,\u0027)"},{"line_number":95,"context_line":"            )"}],"source_content_type":"text/x-python","patch_set":1,"id":"7f231b9d_681add7d","line":92,"in_reply_to":"7f231b9d_da82e9c8","updated":"2017-06-12 14:05:58.000000000","message":"I don\u0027t like calling importutils every time this method is called. I think it is better to call it once.","commit_id":"90a23bc77693c7d111e9352866c06d44507df9b3"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"9d0fc627131292b50ca0681fda7ea37c5e78c0c9","unresolved":false,"context_lines":[{"line_number":26,"context_line":"# as the metrics db is influx"},{"line_number":27,"context_line":"cluster \u003d importutils.try_import(\u0027cassandra.cluster\u0027, None)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"driver_import_logged \u003d False"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":32,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_e41606f2","line":29,"updated":"2017-06-12 22:59:27.000000000","message":"Why exactly we need this ?","commit_id":"b8215da05ad998cdf11357e3b68105789efec143"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"267128fee9bb6b6a47eb4e96ac16f4e9dd6cb907","unresolved":false,"context_lines":[{"line_number":26,"context_line":"# as the metrics db is influx"},{"line_number":27,"context_line":"cluster \u003d importutils.try_import(\u0027cassandra.cluster\u0027, None)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"driver_import_logged \u003d False"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":32,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_a4a48eba","line":29,"in_reply_to":"7f231b9d_e41606f2","updated":"2017-06-12 23:02:09.000000000","message":"So we only log once that the cassandra driver isn\u0027t there. Otherwise, it fills the log file","commit_id":"b8215da05ad998cdf11357e3b68105789efec143"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"9d0fc627131292b50ca0681fda7ea37c5e78c0c9","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    @staticmethod"},{"line_number":86,"context_line":"    def _check_cassandra_status():"},{"line_number":87,"context_line":"        if cluster is None:"},{"line_number":88,"context_line":"            if not driver_import_logged:"},{"line_number":89,"context_line":"                LOG.error(\"Metrics Database is Cassandra but cassandra.cluster\""},{"line_number":90,"context_line":"                          \"not importable. Unable to do health check\")"},{"line_number":91,"context_line":"                driver_import_logged \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_041c6210","line":88,"updated":"2017-06-12 22:59:27.000000000","message":"I think you need to have \n\n   global cluster \n\nand \n   \n   global driver_import_logged\n\nafter all now you refer to those as to variables.","commit_id":"b8215da05ad998cdf11357e3b68105789efec143"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"267128fee9bb6b6a47eb4e96ac16f4e9dd6cb907","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    @staticmethod"},{"line_number":86,"context_line":"    def _check_cassandra_status():"},{"line_number":87,"context_line":"        if cluster is None:"},{"line_number":88,"context_line":"            if not driver_import_logged:"},{"line_number":89,"context_line":"                LOG.error(\"Metrics Database is Cassandra but cassandra.cluster\""},{"line_number":90,"context_line":"                          \"not importable. Unable to do health check\")"},{"line_number":91,"context_line":"                driver_import_logged \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_64b9d6e7","line":88,"in_reply_to":"7f231b9d_041c6210","updated":"2017-06-12 23:02:09.000000000","message":"But aren\u0027t LOG and CONF also variables? Why don\u0027t we have to have a global for them?","commit_id":"b8215da05ad998cdf11357e3b68105789efec143"},{"author":{"_account_id":20033,"name":"Artur Basiak","email":"artur.basiak@ts.fujitsu.com","username":"abasiak"},"change_message_id":"137c55e99ffbeee90f481317fd5a15062b7ac40b","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    @staticmethod"},{"line_number":86,"context_line":"    def _check_cassandra_status():"},{"line_number":87,"context_line":"        if cluster is None:"},{"line_number":88,"context_line":"            if not driver_import_logged:"},{"line_number":89,"context_line":"                LOG.error(\"Metrics Database is Cassandra but cassandra.cluster\""},{"line_number":90,"context_line":"                          \"not importable. Unable to do health check\")"},{"line_number":91,"context_line":"                driver_import_logged \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_869bc4c7","line":88,"in_reply_to":"7f231b9d_08c1102f","updated":"2017-06-13 12:25:14.000000000","message":"Still you need to use here:\n    global driver_import_logged\nBecause you want to change the value of this variable (which is global) in line 91 and that\u0027s why you need to use \u0027global\u0027 to give function a permission to change it.","commit_id":"b8215da05ad998cdf11357e3b68105789efec143"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"47f8bf2d315cea4e970ba5bab22af54c13dc8000","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    @staticmethod"},{"line_number":86,"context_line":"    def _check_cassandra_status():"},{"line_number":87,"context_line":"        if cluster is None:"},{"line_number":88,"context_line":"            if not driver_import_logged:"},{"line_number":89,"context_line":"                LOG.error(\"Metrics Database is Cassandra but cassandra.cluster\""},{"line_number":90,"context_line":"                          \"not importable. Unable to do health check\")"},{"line_number":91,"context_line":"                driver_import_logged \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_08c1102f","line":88,"in_reply_to":"7f231b9d_64b9d6e7","updated":"2017-06-13 05:33:40.000000000","message":"Good point...","commit_id":"b8215da05ad998cdf11357e3b68105789efec143"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"211b88ad699e10f75dd80005473f37cd4a3ee2bb","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    @staticmethod"},{"line_number":86,"context_line":"    def _check_cassandra_status():"},{"line_number":87,"context_line":"        if cluster is None:"},{"line_number":88,"context_line":"            if not driver_import_logged:"},{"line_number":89,"context_line":"                LOG.error(\"Metrics Database is Cassandra but cassandra.cluster\""},{"line_number":90,"context_line":"                          \"not importable. Unable to do health check\")"},{"line_number":91,"context_line":"                driver_import_logged \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_ec7368b0","line":88,"in_reply_to":"7f231b9d_69c50a5b","updated":"2017-06-13 21:26:29.000000000","message":"I made the boolean a class variable instead of a global variable. Seemed wrong to have it as a global. I\u0027m not sure the real value of using static or class methods in this class as opposed to instance methods, but it was a smaller change than going to instance methods.","commit_id":"b8215da05ad998cdf11357e3b68105789efec143"},{"author":{"_account_id":11809,"name":"Craig Bryant","email":"craig.bryant@hpe.com","username":"craig-bryant"},"change_message_id":"bffde87efc045ede5020f33fbfce9b3fdcc5d0b1","unresolved":false,"context_lines":[{"line_number":85,"context_line":"    @staticmethod"},{"line_number":86,"context_line":"    def _check_cassandra_status():"},{"line_number":87,"context_line":"        if cluster is None:"},{"line_number":88,"context_line":"            if not driver_import_logged:"},{"line_number":89,"context_line":"                LOG.error(\"Metrics Database is Cassandra but cassandra.cluster\""},{"line_number":90,"context_line":"                          \"not importable. Unable to do health check\")"},{"line_number":91,"context_line":"                driver_import_logged \u003d True"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f231b9d_69c50a5b","line":88,"in_reply_to":"7f231b9d_869bc4c7","updated":"2017-06-13 21:07:04.000000000","message":"I talked to our local python expert and he says:\nYou can always read a global variable, however, as soon as you assign something to it, python then treats it as a local variable. That then changes the first use of driver_import_logged to a local variable and it is being read before it is set. The others are only read so they are global variables.\n\nThe solution is to use the global driver_import_logged as multiple people have suggested, but I wanted to understand exactly why before making any changes.","commit_id":"b8215da05ad998cdf11357e3b68105789efec143"},{"author":{"_account_id":16168,"name":"Tomasz Trębski","email":"kornicameister@gmail.com","username":"trebskit"},"change_message_id":"99af6a5f6447b7b0b253ba09f4cb898e55344c49","unresolved":false,"context_lines":[{"line_number":24,"context_line":""},{"line_number":25,"context_line":"# Try to import cassandra. Not a problem if it can\u0027t be imported as long"},{"line_number":26,"context_line":"# as the metrics db is influx"},{"line_number":27,"context_line":"cluster \u003d importutils.try_import(\u0027cassandra.cluster\u0027, None)"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"LOG \u003d log.getLogger(__name__)"},{"line_number":30,"context_line":"CONF \u003d cfg.CONF"}],"source_content_type":"text/x-python","patch_set":3,"id":"7f231b9d_6e231ec0","line":27,"updated":"2017-06-20 11:23:44.000000000","message":"Not really sure, but I guess the major part of having troubles in unit testing that is that you have this line here. As long as it would be part of the class (we could have two internal properties there)\n\n  def __init__(self):\n   \n    self._cassandra \u003d importutils.try_import(...)\n    self._influx \u003d importutils.try_import(...)\n\nwe would be already in good position to mock things. Obviously you\u0027d want to have this set to None and set it lazily based on \n\n   CONF.repositories.metrics_driver\n\nOnce set (i.e. not None) you would now that you\u0027ve already logged that exception you have hence cassandra_driver_import_failure_logged would become redundant.","commit_id":"df9c38302c3b14389e961c38613477208c958278"}]}