)]}'
{"trove/configuration/service.py":[{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"6d611ef880eedc19b220878a723873709da12c98","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    @staticmethod"},{"line_number":192,"context_line":"    def _load_configuration_rules(load_db_params_func, datastore_version):"},{"line_number":193,"context_line":"        rules \u003d load_db_params_func(datastore_version.id)"},{"line_number":194,"context_line":"        configuration_parameters \u003d views.ConfigurationParametersView("},{"line_number":195,"context_line":"            rules).data()[\u0027configuration-parameters\u0027]"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        # checking if there are any rules for the datastore"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_d6e5420b","line":194,"updated":"2014-10-09 23:50:32.000000000","message":"The view is only to be used to populate data in the model and to be returned. We shouldnt be calling the views for data to be processed. Looks like you just want to know whether there rule exists. I think you can just do \"not rules:\"","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"e08b3ee75ef5a3a791952a216994ad7a09624c21","unresolved":false,"context_lines":[{"line_number":191,"context_line":"    @staticmethod"},{"line_number":192,"context_line":"    def _load_configuration_rules(load_db_params_func, datastore_version):"},{"line_number":193,"context_line":"        rules \u003d load_db_params_func(datastore_version.id)"},{"line_number":194,"context_line":"        configuration_parameters \u003d views.ConfigurationParametersView("},{"line_number":195,"context_line":"            rules).data()[\u0027configuration-parameters\u0027]"},{"line_number":196,"context_line":""},{"line_number":197,"context_line":"        # checking if there are any rules for the datastore"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_d11e2c0d","line":194,"in_reply_to":"baa201ad_d6e5420b","updated":"2014-10-10 02:09:14.000000000","message":"Yeah that makes sense. I will remove the use of the ConfigurationParamatersView and just use the rules data directly from load_parameters.","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"6d611ef880eedc19b220878a723873709da12c98","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    @staticmethod"},{"line_number":212,"context_line":"    def _validate_configuration(values, datastore_version):"},{"line_number":213,"context_line":"        LOG.info(_(\"Validating configuration values\"))"},{"line_number":214,"context_line":"        rules_lookup \u003d ConfigurationsController._load_configuration_rules("},{"line_number":215,"context_line":"            models.DatastoreConfigurationParameters.load_parameters,"},{"line_number":216,"context_line":"            datastore_version)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_f6c9667a","line":214,"updated":"2014-10-09 23:50:32.000000000","message":"Any reason why you chose to pass in the function instead of calling it and pass the results? I think you did it to make the unit tests easier. Perhaps you can simplify it here and in the unit tests, you can call _validate_configuration instead.","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"e08b3ee75ef5a3a791952a216994ad7a09624c21","unresolved":false,"context_lines":[{"line_number":211,"context_line":"    @staticmethod"},{"line_number":212,"context_line":"    def _validate_configuration(values, datastore_version):"},{"line_number":213,"context_line":"        LOG.info(_(\"Validating configuration values\"))"},{"line_number":214,"context_line":"        rules_lookup \u003d ConfigurationsController._load_configuration_rules("},{"line_number":215,"context_line":"            models.DatastoreConfigurationParameters.load_parameters,"},{"line_number":216,"context_line":"            datastore_version)"},{"line_number":217,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_51d23c35","line":214,"in_reply_to":"baa201ad_f6c9667a","updated":"2014-10-10 02:09:14.000000000","message":"I am removing load_configuration_rules function and putting everything back in the validation configuration method and will unit test everything directly in there.","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"6d611ef880eedc19b220878a723873709da12c98","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            rule \u003d rules_lookup[key]"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"            # type checking"},{"line_number":234,"context_line":"            value_type \u003d rule[\u0027type\u0027]"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"            if not isinstance(v, ConfigurationsController._find_type("},{"line_number":237,"context_line":"                    value_type)):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_36d76eef","line":234,"updated":"2014-10-09 23:50:32.000000000","message":"You wouldnt have to change the way these variables are accessed. These attributes can be accessed with the . format.","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"e08b3ee75ef5a3a791952a216994ad7a09624c21","unresolved":false,"context_lines":[{"line_number":231,"context_line":"            rule \u003d rules_lookup[key]"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"            # type checking"},{"line_number":234,"context_line":"            value_type \u003d rule[\u0027type\u0027]"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"            if not isinstance(v, ConfigurationsController._find_type("},{"line_number":237,"context_line":"                    value_type)):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_bc725fab","line":234,"in_reply_to":"baa201ad_36d76eef","updated":"2014-10-10 02:09:14.000000000","message":"Agreed. Reverting.","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":10725,"name":"Sergey Gotliv","email":"sgotliv@redhat.com","username":"sgotliv"},"change_message_id":"9ebb686828de1b2db27e5ea5b07414ff0373538c","unresolved":false,"context_lines":[{"line_number":243,"context_line":"            # integer min/max checking"},{"line_number":244,"context_line":"            if isinstance(v, (int, long)) and not isinstance(v, bool):"},{"line_number":245,"context_line":"                try:"},{"line_number":246,"context_line":"                    min_value \u003d int(rule[\u0027min_size\u0027])"},{"line_number":247,"context_line":"                except ValueError:"},{"line_number":248,"context_line":"                    raise exception.TroveError(_("},{"line_number":249,"context_line":"                        \"Invalid or unsupported min value defined in the \""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_0c3a5336","line":246,"updated":"2014-10-10 11:31:41.000000000","message":"I assume you will revert this one and another one below as well.","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"20a7adb33cc72746ded46d198b7409ae553f24ac","unresolved":false,"context_lines":[{"line_number":243,"context_line":"            # integer min/max checking"},{"line_number":244,"context_line":"            if isinstance(v, (int, long)) and not isinstance(v, bool):"},{"line_number":245,"context_line":"                try:"},{"line_number":246,"context_line":"                    min_value \u003d int(rule[\u0027min_size\u0027])"},{"line_number":247,"context_line":"                except ValueError:"},{"line_number":248,"context_line":"                    raise exception.TroveError(_("},{"line_number":249,"context_line":"                        \"Invalid or unsupported min value defined in the \""}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_02112fac","line":246,"in_reply_to":"baa201ad_0c3a5336","updated":"2014-10-10 19:17:16.000000000","message":"Yes I have updated all three on the newest patch set.","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"0d0cffcdac84511fbf633a8df8b6331aad0b9569","unresolved":false,"context_lines":[{"line_number":101,"context_line":"            ConfigurationsController._validate_configuration("},{"line_number":102,"context_line":"                body[\u0027configuration\u0027][\u0027values\u0027],"},{"line_number":103,"context_line":"                datastore_version,"},{"line_number":104,"context_line":"                models.DatastoreConfigurationParameters.load_parameters("},{"line_number":105,"context_line":"                    datastore_version.id))"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"            for k, v in values.iteritems():"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_590f3039","line":104,"updated":"2014-10-16 00:56:06.000000000","message":"why move to caller?","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"20552b8ec4e4ac9ec294c9d6d28e84d3cb0021d3","unresolved":false,"context_lines":[{"line_number":101,"context_line":"            ConfigurationsController._validate_configuration("},{"line_number":102,"context_line":"                body[\u0027configuration\u0027][\u0027values\u0027],"},{"line_number":103,"context_line":"                datastore_version,"},{"line_number":104,"context_line":"                models.DatastoreConfigurationParameters.load_parameters("},{"line_number":105,"context_line":"                    datastore_version.id))"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"            for k, v in values.iteritems():"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_1ca2f6ad","line":104,"in_reply_to":"baa201ad_590f3039","updated":"2014-10-16 01:49:38.000000000","message":"For unit testing. This way we do not need to rely on the integration test run in order to do validation testing. We can cover the validation scenarios in the core unit tests without having to have a Fake DB.","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"0d0cffcdac84511fbf633a8df8b6331aad0b9569","unresolved":false,"context_lines":[{"line_number":183,"context_line":"            ConfigurationsController._validate_configuration("},{"line_number":184,"context_line":"                configuration[\u0027values\u0027],"},{"line_number":185,"context_line":"                ds_version,"},{"line_number":186,"context_line":"                models.DatastoreConfigurationParameters.load_parameters("},{"line_number":187,"context_line":"                    ds_version.id))"},{"line_number":188,"context_line":"            for k, v in configuration[\u0027values\u0027].iteritems():"},{"line_number":189,"context_line":"                items.append(DBConfigurationParameter("}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_7914f44c","line":186,"updated":"2014-10-16 00:56:06.000000000","message":"why move to caller?","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"20552b8ec4e4ac9ec294c9d6d28e84d3cb0021d3","unresolved":false,"context_lines":[{"line_number":183,"context_line":"            ConfigurationsController._validate_configuration("},{"line_number":184,"context_line":"                configuration[\u0027values\u0027],"},{"line_number":185,"context_line":"                ds_version,"},{"line_number":186,"context_line":"                models.DatastoreConfigurationParameters.load_parameters("},{"line_number":187,"context_line":"                    ds_version.id))"},{"line_number":188,"context_line":"            for k, v in configuration[\u0027values\u0027].iteritems():"},{"line_number":189,"context_line":"                items.append(DBConfigurationParameter("}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_9ccee681","line":186,"in_reply_to":"baa201ad_7914f44c","updated":"2014-10-16 01:49:38.000000000","message":"See comment above.","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"0d0cffcdac84511fbf633a8df8b6331aad0b9569","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        # create rules dictionary based on parameter name"},{"line_number":201,"context_line":"        rules_lookup \u003d {}"},{"line_number":202,"context_line":"        for item in config_rules:"},{"line_number":203,"context_line":"            rules_lookup[item.name.lower()] \u003d item"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        # checking if there are any rules for the datastore"},{"line_number":206,"context_line":"        if not rules_lookup:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_990c3844","line":203,"updated":"2014-10-16 00:56:06.000000000","message":"caseless eh?","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"20552b8ec4e4ac9ec294c9d6d28e84d3cb0021d3","unresolved":false,"context_lines":[{"line_number":200,"context_line":"        # create rules dictionary based on parameter name"},{"line_number":201,"context_line":"        rules_lookup \u003d {}"},{"line_number":202,"context_line":"        for item in config_rules:"},{"line_number":203,"context_line":"            rules_lookup[item.name.lower()] \u003d item"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        # checking if there are any rules for the datastore"},{"line_number":206,"context_line":"        if not rules_lookup:"}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_3cd97a57","line":203,"in_reply_to":"baa201ad_990c3844","updated":"2014-10-16 01:49:38.000000000","message":"Was keeping parity with original functionality which allowed parameters to be caseless insensitive. I moved everything to be in the new rules based dictionary instead of having to call the load_paramater_by_name each time.","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"0d0cffcdac84511fbf633a8df8b6331aad0b9569","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            rules_lookup[item.name.lower()] \u003d item"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        # checking if there are any rules for the datastore"},{"line_number":206,"context_line":"        if not rules_lookup:"},{"line_number":207,"context_line":"            output \u003d {\"version\": datastore_version.name,"},{"line_number":208,"context_line":"                      \"name\": datastore_version.datastore_name}"},{"line_number":209,"context_line":"            msg \u003d _(\"Configuration groups are not supported for this \""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_f9ff0400","line":206,"updated":"2014-10-16 00:56:06.000000000","message":"this check changes the behavior of the routine in some cases.","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"20552b8ec4e4ac9ec294c9d6d28e84d3cb0021d3","unresolved":false,"context_lines":[{"line_number":203,"context_line":"            rules_lookup[item.name.lower()] \u003d item"},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"        # checking if there are any rules for the datastore"},{"line_number":206,"context_line":"        if not rules_lookup:"},{"line_number":207,"context_line":"            output \u003d {\"version\": datastore_version.name,"},{"line_number":208,"context_line":"                      \"name\": datastore_version.datastore_name}"},{"line_number":209,"context_line":"            msg \u003d _(\"Configuration groups are not supported for this \""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_1cbbd6e5","line":206,"in_reply_to":"baa201ad_f9ff0400","updated":"2014-10-16 01:49:38.000000000","message":"This is the main portion of the bug fix that if no rules are found we should raise an exception. This is the case when you have a database instance but the rules table is empty.","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"}],"trove/tests/api/configurations.py":[{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"0d0cffcdac84511fbf633a8df8b6331aad0b9569","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                CONFIG_NAME,"},{"line_number":223,"context_line":"                values,"},{"line_number":224,"context_line":"                CONFIG_DESC)"},{"line_number":225,"context_line":"        except exceptions.UnprocessableEntity:"},{"line_number":226,"context_line":"            resp, body \u003d instance_info.dbaas.client.last_response"},{"line_number":227,"context_line":"            assert_equal(resp.status, 422)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_391eec70","line":225,"updated":"2014-10-16 00:56:06.000000000","message":"why?","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"20552b8ec4e4ac9ec294c9d6d28e84d3cb0021d3","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                CONFIG_NAME,"},{"line_number":223,"context_line":"                values,"},{"line_number":224,"context_line":"                CONFIG_DESC)"},{"line_number":225,"context_line":"        except exceptions.UnprocessableEntity:"},{"line_number":226,"context_line":"            resp, body \u003d instance_info.dbaas.client.last_response"},{"line_number":227,"context_line":"            assert_equal(resp.status, 422)"},{"line_number":228,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_bc92ca54","line":225,"in_reply_to":"baa201ad_391eec70","updated":"2014-10-16 01:49:38.000000000","message":"Updated this because the original intention of the method was to throw a 422 for parameter validation, but the load_paramater_by_name was raising 404, thus never hitting the previous \"if not rule check\" from line 199 of service.py. 404 is also an incorrect status code for parameter value validation. 404 should be reserved for resource and routing failures. Per the API change guidelines: https://wiki.openstack.org/wiki/APIChangeGuidelines I thought we can update an error response code to be more accurate.","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"}],"trove/tests/unittests/configuration/test_configuration_controller.py":[{"author":{"_account_id":5390,"name":"Steve Leon","username":"steve-leon","inactive":true},"change_message_id":"6d611ef880eedc19b220878a723873709da12c98","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        output \u003d ConfigurationsController._load_configuration_rules(load_db_params_func, data_version)"},{"line_number":186,"context_line":"        self.assertDictContainsSubset(output[config_val1.name], result[0])"},{"line_number":187,"context_line":"        self.assertDictContainsSubset(output[config_val2.name], result[1])"},{"line_number":188,"context_line":""},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"class TestConfigurationsParameterController(TestCase):"},{"line_number":191,"context_line":"    def setUp(self):"}],"source_content_type":"text/x-python","patch_set":1,"id":"baa201ad_d6246210","line":188,"updated":"2014-10-09 23:50:32.000000000","message":"Also can you create a new unit test that check the 2nd 1st scenary from the bug? ie the param provided is not in the validation-rules.","commit_id":"851e953dcea6eb815a82f0603a9ab02fee657382"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"0d0cffcdac84511fbf633a8df8b6331aad0b9569","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        config_val1.name \u003d \u0027max_connections\u0027"},{"line_number":146,"context_line":"        config_val1.restart_required \u003d \u0027false\u0027"},{"line_number":147,"context_line":"        config_val1.datastore_version_id \u003d 5.5"},{"line_number":148,"context_line":"        config_val1.max_size \u003d 1"},{"line_number":149,"context_line":"        config_val1.min_size \u003d 0"},{"line_number":150,"context_line":"        config_val1.data_type \u003d \u0027integer\u0027"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_197a08f1","line":148,"range":{"start_line":148,"start_character":20,"end_line":148,"end_character":28},"updated":"2014-10-16 00:56:06.000000000","message":"seems like there\u0027s a lot of duplicated code that could be simplified.","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"},{"author":{"_account_id":13355,"name":"Edmond Kotowski","email":"ekotowski@gmail.com","username":"ekotowski"},"change_message_id":"20552b8ec4e4ac9ec294c9d6d28e84d3cb0021d3","unresolved":false,"context_lines":[{"line_number":145,"context_line":"        config_val1.name \u003d \u0027max_connections\u0027"},{"line_number":146,"context_line":"        config_val1.restart_required \u003d \u0027false\u0027"},{"line_number":147,"context_line":"        config_val1.datastore_version_id \u003d 5.5"},{"line_number":148,"context_line":"        config_val1.max_size \u003d 1"},{"line_number":149,"context_line":"        config_val1.min_size \u003d 0"},{"line_number":150,"context_line":"        config_val1.data_type \u003d \u0027integer\u0027"},{"line_number":151,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"baa201ad_dc722e49","line":148,"in_reply_to":"baa201ad_197a08f1","updated":"2014-10-16 01:49:38.000000000","message":"I do agree. What is the way we are handling data driven tests in the unit testing framework? Can this framework do parameterized tests, or do just need to craft this? I left the duplication in order to keep the tests as simple as possible to read, because they all hit different validation areas.","commit_id":"726cf816611d85c69eacc7fa7eb8262840057d26"}]}