)]}'
{"requirements.txt":[{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":8,"context_line":"oslo.serialization!\u003d2.19.1,\u003e\u003d2.18.0 # Apache-2.0"},{"line_number":9,"context_line":"testtools\u003e\u003d2.2.0 # MIT"},{"line_number":10,"context_line":"tempest\u003e\u003d17.1.0 # Apache-2.0"},{"line_number":11,"context_line":"SQLAlchemy!\u003d1.1.5,!\u003d1.1.6,!\u003d1.1.7,!\u003d1.1.8,\u003e\u003d1.0.10 # MIT"},{"line_number":12,"context_line":"PyMySQL\u003e\u003d0.7.6 # MIT"}],"source_content_type":"text/plain","patch_set":2,"id":"3f79a3b5_9750100c","line":12,"range":{"start_line":11,"start_character":0,"end_line":12,"end_character":20},"updated":"2018-09-09 11:30:29.000000000","message":"How about using upper constraints as well ?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":8,"context_line":"oslo.serialization!\u003d2.19.1,\u003e\u003d2.18.0 # Apache-2.0"},{"line_number":9,"context_line":"testtools\u003e\u003d2.2.0 # MIT"},{"line_number":10,"context_line":"tempest\u003e\u003d17.1.0 # Apache-2.0"},{"line_number":11,"context_line":"SQLAlchemy!\u003d1.1.5,!\u003d1.1.6,!\u003d1.1.7,!\u003d1.1.8,\u003e\u003d1.0.10 # MIT"},{"line_number":12,"context_line":"PyMySQL\u003e\u003d0.7.6 # MIT"}],"source_content_type":"text/plain","patch_set":2,"id":"3f79a3b5_9e9da858","line":12,"range":{"start_line":11,"start_character":0,"end_line":12,"end_character":20},"in_reply_to":"3f79a3b5_9750100c","updated":"2018-09-20 12:59:48.000000000","message":"Upper constraints are defined globally in openstack/requirements repository [1].\n\n[1] https://github.com/openstack/requirements/blob/master/upper-constraints.txt","commit_id":"18ba0db60706548c87856581724410b16420a2c6"}],"trove_tempest_plugin/adapters/__init__.py":[{"author":{"_account_id":29159,"name":"Cezary Żukowski","email":"c.zukowski@partner.samsung.com","username":"c.zukowski"},"change_message_id":"f46d5518572b7eb6447d961beb3380517adf83b6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"3f79a3b5_79172df0","updated":"2018-09-26 07:35:37.000000000","message":"Overall, it follows the \"factory\" design pattern. Always in dispute:\n- balance defining in implicit vs. explicit manner (e.g. wrapped in class or package contained)\n- the configuration injection vs. the global configuration (e.g. relevant in the context of unit testing)\n- division into groups such as experimental vs. stable vs. plugin \n\nImplications vary and here is one of the illustrative snippets:\n\nclass AdapterFactory:\n    _adapters \u003d dict(mysql\u003dMySQLAdapter,\n                     mariadb\u003dMariaDBAdapter,\n                     cassandra\u003dCassandraAdapter)\n    # _experimental_adapters \u003d {}\n\n    @classmethod\n    def list(cls):\n        return cls._adapters.keys()\n\n    @classmethod\n    def set_global_config(cls, config):\n        cls._CONF \u003d config.CONF\n\n    @classmethod\n    def get_adapter(cls, config, datastore\u003dNone):\n        if not config:  # fallback: use global config\n            datastore \u003d cls._CONF.database.datastore\n        return cls._get_adapter(datastore)\n\n    @classmethod\n    def _get_adapter(cls, datastore):\n        if datastore not in cls._adapters:\n            raise NotYetSupportedDatastoreException(\n                \"The datastore \u0027{}\u0027 is not supported yet. \"\n                \"Call for new features via person@example.com, please.\".format(datastore))\n        return cls._adapters[datastore]()\n\n    @classmethod\n    def upload_adapter(cls, **kwargs):\n        cls._adapters.update(kwargs)","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":29159,"name":"Cezary Żukowski","email":"c.zukowski@partner.samsung.com","username":"c.zukowski"},"change_message_id":"f46d5518572b7eb6447d961beb3380517adf83b6","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from trove_tempest_plugin.adapters.mariadb import MariadbAdapter"},{"line_number":19,"context_line":"from trove_tempest_plugin.adapters.mysql import MysqlAdapter"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"CONF \u003d config.CONF"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"def get_adapter(datastore\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d30e678b","line":21,"range":{"start_line":21,"start_character":0,"end_line":21,"end_character":18},"updated":"2018-09-26 07:35:37.000000000","message":"Why couldn\u0027t be coded in a single line:  \n   from tempest.config import CONF\n, as in trove_tempest_plugin\\managers\\floating_ips.py, \ninstead of a couple of lines:\n  from tempest import config\n  CONF \u003d config.CONF\n?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"c02be09b3dbb7a4e31da7f0f534b0256a7cefa6b","unresolved":false,"context_lines":[{"line_number":18,"context_line":"from trove_tempest_plugin.adapters.mariadb import MariadbAdapter"},{"line_number":19,"context_line":"from trove_tempest_plugin.adapters.mysql import MysqlAdapter"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"CONF \u003d config.CONF"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"def get_adapter(datastore\u003dNone):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_e4390aa3","line":21,"range":{"start_line":21,"start_character":0,"end_line":21,"end_character":18},"in_reply_to":"3f79a3b5_d30e678b","updated":"2018-10-01 10:37:22.000000000","message":"I was suggested by other contributors to follow this pattern. Seems to be a convention that they adopted in the project. Will change the import in floating_ips module.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":21,"context_line":"CONF \u003d config.CONF"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"def get_adapter(datastore\u003dNone):"},{"line_number":25,"context_line":"    if not datastore:"},{"line_number":26,"context_line":"        datastore \u003d CONF.database.datastore"},{"line_number":27,"context_line":"    adapter_class \u003d _get_adapter_class(datastore)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b7466c4f","line":24,"range":{"start_line":24,"start_character":4,"end_line":24,"end_character":32},"updated":"2018-09-09 11:30:29.000000000","message":"Should we allow to call such a function None ?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":21,"context_line":"CONF \u003d config.CONF"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""},{"line_number":24,"context_line":"def get_adapter(datastore\u003dNone):"},{"line_number":25,"context_line":"    if not datastore:"},{"line_number":26,"context_line":"        datastore \u003d CONF.database.datastore"},{"line_number":27,"context_line":"    adapter_class \u003d _get_adapter_class(datastore)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_bee2e4d2","line":24,"range":{"start_line":24,"start_character":4,"end_line":24,"end_character":32},"in_reply_to":"3f79a3b5_b7466c4f","updated":"2018-09-20 12:59:48.000000000","message":"In most cases we want to get adapter for datastore set in Tempest configuration. Fetching datastore from config and passing it to this method each time would be a unnecessary burden. That\u0027s why the argument was implemented as optional (datastore\u003dNone).\n\nIf someone passes \"None\" value as argument explicitely they will still experience the same behaviour. I don\u0027t see any problem with that.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def _get_adapter_class(datastore):"},{"line_number":32,"context_line":"    if datastore \u003d\u003d \u0027mysql\u0027:"},{"line_number":33,"context_line":"        return MysqlAdapter"},{"line_number":34,"context_line":"    elif datastore \u003d\u003d \u0027mariadb\u0027:"},{"line_number":35,"context_line":"        return MariadbAdapter"},{"line_number":36,"context_line":"    else:"},{"line_number":37,"context_line":"        raise ValueError(\"Unknown datastore %s.\" % datastore)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_5756f806","line":37,"range":{"start_line":32,"start_character":0,"end_line":37,"end_character":61},"updated":"2018-09-09 11:30:29.000000000","message":"Maybe a better way is to define a dict(datastore \u003d\u003e adapter_class)?\nWe could also try to think about generating such dict dynamically based on available files in a convention over configuration fashion.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def _get_adapter_class(datastore):"},{"line_number":32,"context_line":"    if datastore \u003d\u003d \u0027mysql\u0027:"},{"line_number":33,"context_line":"        return MysqlAdapter"},{"line_number":34,"context_line":"    elif datastore \u003d\u003d \u0027mariadb\u0027:"},{"line_number":35,"context_line":"        return MariadbAdapter"},{"line_number":36,"context_line":"    else:"},{"line_number":37,"context_line":"        raise ValueError(\"Unknown datastore %s.\" % datastore)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_5913aa59","line":37,"range":{"start_line":32,"start_character":0,"end_line":37,"end_character":61},"in_reply_to":"3f79a3b5_5756f806","updated":"2018-09-20 12:59:48.000000000","message":"I like the ideas. Will try to refactor.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":29159,"name":"Cezary Żukowski","email":"c.zukowski@partner.samsung.com","username":"c.zukowski"},"change_message_id":"f46d5518572b7eb6447d961beb3380517adf83b6","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def _get_adapter_class(datastore):"},{"line_number":32,"context_line":"    if datastore \u003d\u003d \u0027mysql\u0027:"},{"line_number":33,"context_line":"        return MysqlAdapter"},{"line_number":34,"context_line":"    elif datastore \u003d\u003d \u0027mariadb\u0027:"},{"line_number":35,"context_line":"        return MariadbAdapter"},{"line_number":36,"context_line":"    else:"},{"line_number":37,"context_line":"        raise ValueError(\"Unknown datastore %s.\" % datastore)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_6438e49e","line":37,"range":{"start_line":32,"start_character":0,"end_line":37,"end_character":61},"in_reply_to":"3f79a3b5_5913aa59","updated":"2018-09-26 07:35:37.000000000","message":"I would opt to fix/hardcode the major (or stable) adapters, and consider experimental/plugin section/routine as an option to upload further adapters (afaik, Trove has the experimental facility for datastores).\n\nIn terms of design patterns and adapter-as-a-plugin, \"registry\" pattern might be worth looking at. In nutshell: every class (every adapter) derived from AdapterBase, automatically becomes registered in a global variable. Illustration: https://github.com/faif/python-patterns/blob/master/behavioral/registry.py","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"c02be09b3dbb7a4e31da7f0f534b0256a7cefa6b","unresolved":false,"context_lines":[{"line_number":29,"context_line":""},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"def _get_adapter_class(datastore):"},{"line_number":32,"context_line":"    if datastore \u003d\u003d \u0027mysql\u0027:"},{"line_number":33,"context_line":"        return MysqlAdapter"},{"line_number":34,"context_line":"    elif datastore \u003d\u003d \u0027mariadb\u0027:"},{"line_number":35,"context_line":"        return MariadbAdapter"},{"line_number":36,"context_line":"    else:"},{"line_number":37,"context_line":"        raise ValueError(\"Unknown datastore %s.\" % datastore)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_fe8c3136","line":37,"range":{"start_line":32,"start_character":0,"end_line":37,"end_character":61},"in_reply_to":"3f79a3b5_6438e49e","updated":"2018-10-01 10:37:22.000000000","message":"After your suggestion I attempted to apply the registry pattern, however, ended up with a similar problem related to mapping configured datastore names to class names in the registry dict.\n\nIn test configuration we have:\n\n datastore \u003d mariadb|cassandra|redis\n\nAnd the registry provides the following dict:\n\n {\n     \u0027MariadbAdapter\u0027: MariadbAdapter,\n     \u0027CassandraAdapter\u0027: MariadbAdapter,\n     \u0027RedisAdapter\u0027: MariadbAdapter,\n     ...\n }\n\nIn order to corelate config value with registry key, we would have to do string interpolation e.g.\n\n adapter_cls_name \u003d \"%sAdapter\"\n\nAnother option was to add class-level attribute to adapter classes e.g. \"adapter_name\" and use them as registry keys.\n\nBoth approaches were not too elegant.\n\nSince everything seemed to reduce to hardcoding a dict with mapping between datastores and adapter classes, I decided to use entry_points which allow to achieve exactly that and additionaly provide pluggable interface.\n\nMany other OpenStack projects use common stevedore library for loading drivers from entry points [1-2].\n\nAnyway, thanks for note about registry pattern. It is one of these interesting design patterns that I didn\u0027t know about before. Happy to learn something new :)\n\nI\u0027m also opened for further discussion in this topic.\n\n[1] https://github.com/openstack/oslo.messaging/blob/master/setup.cfg#L39\n[2] https://github.com/openstack/oslo.messaging/blob/815e25e55b73bacbe0dba61d153024d395864ded/oslo_messaging/transport.py#L204","commit_id":"18ba0db60706548c87856581724410b16420a2c6"}],"trove_tempest_plugin/adapters/base.py":[{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"class BaseAdapter(object):"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    \"\"\"Base database adapter class"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    Adapter provides a unified interface to interact with various types of"},{"line_number":22,"context_line":"    databases. Among other things, it implements the logic responsible for:"},{"line_number":23,"context_line":"        - checking database availability,"},{"line_number":24,"context_line":"        - manipulating data,"},{"line_number":25,"context_line":"        - reading configuration."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    Testing scenarios use adapters in steps that require direct interaction"},{"line_number":28,"context_line":"    with the database. As a consequence, it is possible to define one universal"},{"line_number":29,"context_line":"    test run that adapts its behavior depending on the type of database"},{"line_number":30,"context_line":"    configured."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def __init__(self):"},{"line_number":34,"context_line":"        self.__client \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d741a837","line":31,"range":{"start_line":19,"start_character":0,"end_line":31,"end_character":7},"updated":"2018-09-09 11:30:29.000000000","message":"We should define here API for adapters here explicitly","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":16,"context_line":""},{"line_number":17,"context_line":"class BaseAdapter(object):"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"    \"\"\"Base database adapter class"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    Adapter provides a unified interface to interact with various types of"},{"line_number":22,"context_line":"    databases. Among other things, it implements the logic responsible for:"},{"line_number":23,"context_line":"        - checking database availability,"},{"line_number":24,"context_line":"        - manipulating data,"},{"line_number":25,"context_line":"        - reading configuration."},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    Testing scenarios use adapters in steps that require direct interaction"},{"line_number":28,"context_line":"    with the database. As a consequence, it is possible to define one universal"},{"line_number":29,"context_line":"    test run that adapts its behavior depending on the type of database"},{"line_number":30,"context_line":"    configured."},{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def __init__(self):"},{"line_number":34,"context_line":"        self.__client \u003d None"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_9964e2e6","line":31,"range":{"start_line":19,"start_character":0,"end_line":31,"end_character":7},"in_reply_to":"3f79a3b5_d741a837","updated":"2018-09-20 12:59:48.000000000","message":"I documented methods of the interface in individual docstrings of these methods. Isn\u0027t it sufficient?\n\nWhat do you propose to describe in the docstring of the class?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":23707,"name":"Łukasz Deptuła","email":"l.deptula@samsung.com"},"change_message_id":"7c7a43275c3f0f7e6a29fa2dfb35cfbad9d0b06f","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def __init__(self):"},{"line_number":34,"context_line":"        self.__client \u003d None"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def client(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_cb4ef294","line":34,"updated":"2018-09-21 05:36:48.000000000","message":"For private properties it is recommended to use single underscore:\nhttps://www.python.org/dev/peps/pep-0008/?#method-names-and-instance-variables","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"c02be09b3dbb7a4e31da7f0f534b0256a7cefa6b","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def __init__(self):"},{"line_number":34,"context_line":"        self.__client \u003d None"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def client(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_febbd8e0","line":34,"in_reply_to":"3f79a3b5_736513e2","updated":"2018-10-01 10:37:22.000000000","message":"That was my intention. I don\u0027t want child classes to override __client variable since it is managed by property method defined in the base class.\n\nThis is how I marked the variable as private to the base. If I wanted to mark it as protected and allow overriding in child classes, I would use single underscore as you suggest.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":29159,"name":"Cezary Żukowski","email":"c.zukowski@partner.samsung.com","username":"c.zukowski"},"change_message_id":"f46d5518572b7eb6447d961beb3380517adf83b6","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    \"\"\""},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    def __init__(self):"},{"line_number":34,"context_line":"        self.__client \u003d None"},{"line_number":35,"context_line":""},{"line_number":36,"context_line":"    @property"},{"line_number":37,"context_line":"    def client(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_736513e2","line":34,"in_reply_to":"3f79a3b5_cb4ef294","updated":"2018-09-26 07:35:37.000000000","message":"Using double underscore herein is perhaps for \"mangling\" whose purpose differs and it prevents accidental variable override in a child class. Illustration:\n\nclass Parent:\n    _not_mangled \u003d \u0027non-M\u0027\n    __mangled \u003d \u0027M\u0027\n\nclass Child(Parent):\n    def __init__(self):\n        try:\n            print(self.__mangled)\n        except Exception as ex:\n            print(\"Error: {}\".format(ex))\n\n        print(self._not_mangled)\n        self._not_mangled \u003d \u0027Changed\u0027\n        print(self._not_mangled)\n        print(self._Parent__mangled)\n\nc \u003d C()\n\n# Output:\n# Error: \u0027Child\u0027 object has no attribute \u0027_Child__mangled\u0027\n# non-M\n# Changed\n# M","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        self.__client \u003d None"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def ping(self):"},{"line_number":76,"context_line":"        \"\"\"Perform a database ping"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        Performs a simple ping to the database server to verify that it is"},{"line_number":79,"context_line":"        functional. The ways of implementing the ping may vary between"},{"line_number":80,"context_line":"        databases. Some of them have APIs built into the client to check server"},{"line_number":81,"context_line":"        availability, while others require implementing own checking mechanism."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b7b42c37","line":78,"range":{"start_line":76,"start_character":11,"end_line":78,"end_character":30},"updated":"2018-09-09 11:30:29.000000000","message":"IMHO we could compress it a little bit. In basically each method you are repeating similar message in short version and in the longer version.\nAlso ping() method name seems to be self-explanatory.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        self.__client \u003d None"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def ping(self):"},{"line_number":76,"context_line":"        \"\"\"Perform a database ping"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"        Performs a simple ping to the database server to verify that it is"},{"line_number":79,"context_line":"        functional. The ways of implementing the ping may vary between"},{"line_number":80,"context_line":"        databases. Some of them have APIs built into the client to check server"},{"line_number":81,"context_line":"        availability, while others require implementing own checking mechanism."}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_5edb90e8","line":78,"range":{"start_line":76,"start_character":11,"end_line":78,"end_character":30},"in_reply_to":"3f79a3b5_b7b42c37","updated":"2018-09-20 12:59:48.000000000","message":"Cannot compress. Multi-line docstring must have summary line.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        :param int size: Number of aggregates that should be populated into the"},{"line_number":103,"context_line":"                         database"},{"line_number":104,"context_line":"        \"\"\""},{"line_number":105,"context_line":"        raise NotImplementedError("},{"line_number":106,"context_line":"            \"Method must be implemented in derived class.\")"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    def verify_data(self, label, size):"},{"line_number":109,"context_line":"        \"\"\"Verify data in the database"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d75a081c","line":106,"range":{"start_line":105,"start_character":0,"end_line":106,"end_character":59},"updated":"2018-09-09 11:30:29.000000000","message":"How wabout using @abstractmethod ?\n\n    @abstractmethod\n    def do_something(self):\n        pass\n\nNow there is a lot of repeatable code.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"}],"trove_tempest_plugin/adapters/mariadb.py":[{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from trove_tempest_plugin.adapters.mysql import MysqlAdapter"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class MariadbAdapter(MysqlAdapter):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    \"\"\"Adapter for MariaDB database\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def __init__(self):"},{"line_number":24,"context_line":"        super(MariadbAdapter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d7af68dd","line":24,"range":{"start_line":19,"start_character":0,"end_line":24,"end_character":46},"updated":"2018-09-09 11:30:29.000000000","message":"IMHO Comment here is unnecessary.\n\nalso in Python you can just say super().__init__() instead of super(ChildB, self).__init__() looks much nicer","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from trove_tempest_plugin.adapters.mysql import MysqlAdapter"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class MariadbAdapter(MysqlAdapter):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    \"\"\"Adapter for MariaDB database\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def __init__(self):"},{"line_number":24,"context_line":"        super(MariadbAdapter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_f9839654","line":24,"range":{"start_line":19,"start_character":0,"end_line":24,"end_character":46},"in_reply_to":"3f79a3b5_d7af68dd","updated":"2018-09-20 12:59:48.000000000","message":"Proposed invocation is available only in Python 3:\n\n super().__init__()\n\nSince we must stick with Python 2 for a longer while I would rather stay with current invocation.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":23707,"name":"Łukasz Deptuła","email":"l.deptula@samsung.com"},"change_message_id":"7c7a43275c3f0f7e6a29fa2dfb35cfbad9d0b06f","unresolved":false,"context_lines":[{"line_number":16,"context_line":"from trove_tempest_plugin.adapters.mysql import MysqlAdapter"},{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class MariadbAdapter(MysqlAdapter):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    \"\"\"Adapter for MariaDB database\"\"\""},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    def __init__(self):"},{"line_number":24,"context_line":"        super(MariadbAdapter, self).__init__()"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_4b1f426f","line":24,"range":{"start_line":19,"start_character":0,"end_line":24,"end_character":46},"in_reply_to":"3f79a3b5_f9839654","updated":"2018-09-21 05:36:48.000000000","message":"This invocation will behave exactly in the same way without __init__ method here:\n\nclass MariadbAdapter(MysqlAdapter):\n    \"\"\"Adapter for MariaDB database\"\"\"\n\n---\nEOF","commit_id":"18ba0db60706548c87856581724410b16420a2c6"}],"trove_tempest_plugin/config.py":[{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":39,"context_line":"               help\u003d\"Datastore to use in database tests.\"),"},{"line_number":40,"context_line":"    cfg.StrOpt(\u0027datastore_version\u0027,"},{"line_number":41,"context_line":"               help\u003d\"Datastore version to use in database tests.\"),"},{"line_number":42,"context_line":"    cfg.StrOpt(\u0027flavor_ref\u0027,"},{"line_number":43,"context_line":"               help\u003d\"Flavor to use for database instances in tests.\"),"},{"line_number":44,"context_line":"    cfg.IntOpt(\u0027volume_size\u0027,"},{"line_number":45,"context_line":"               default\u003d1,"},{"line_number":46,"context_line":"               help\u003d\"Volume size to use for database instances in tests.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_1793a0a4","line":43,"range":{"start_line":42,"start_character":0,"end_line":43,"end_character":70},"updated":"2018-09-09 11:30:29.000000000","message":"Why do we need to change db_flavor_ref which seemed to be used already ?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":39,"context_line":"               help\u003d\"Datastore to use in database tests.\"),"},{"line_number":40,"context_line":"    cfg.StrOpt(\u0027datastore_version\u0027,"},{"line_number":41,"context_line":"               help\u003d\"Datastore version to use in database tests.\"),"},{"line_number":42,"context_line":"    cfg.StrOpt(\u0027flavor_ref\u0027,"},{"line_number":43,"context_line":"               help\u003d\"Flavor to use for database instances in tests.\"),"},{"line_number":44,"context_line":"    cfg.IntOpt(\u0027volume_size\u0027,"},{"line_number":45,"context_line":"               default\u003d1,"},{"line_number":46,"context_line":"               help\u003d\"Volume size to use for database instances in tests.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_d9f1fa01","line":43,"range":{"start_line":42,"start_character":0,"end_line":43,"end_character":70},"in_reply_to":"3f79a3b5_1793a0a4","updated":"2018-09-20 12:59:48.000000000","message":"I didn\u0027t like \"db_\" prefix and wanted to continue without this convention for \"volume_size\" and other parameters in the future.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":29159,"name":"Cezary Żukowski","email":"c.zukowski@partner.samsung.com","username":"c.zukowski"},"change_message_id":"f46d5518572b7eb6447d961beb3380517adf83b6","unresolved":false,"context_lines":[{"line_number":39,"context_line":"               help\u003d\"Datastore to use in database tests.\"),"},{"line_number":40,"context_line":"    cfg.StrOpt(\u0027datastore_version\u0027,"},{"line_number":41,"context_line":"               help\u003d\"Datastore version to use in database tests.\"),"},{"line_number":42,"context_line":"    cfg.StrOpt(\u0027flavor_ref\u0027,"},{"line_number":43,"context_line":"               help\u003d\"Flavor to use for database instances in tests.\"),"},{"line_number":44,"context_line":"    cfg.IntOpt(\u0027volume_size\u0027,"},{"line_number":45,"context_line":"               default\u003d1,"},{"line_number":46,"context_line":"               help\u003d\"Volume size to use for database instances in tests.\")"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_690e8634","line":43,"range":{"start_line":42,"start_character":0,"end_line":43,"end_character":70},"in_reply_to":"3f79a3b5_d9f1fa01","updated":"2018-09-26 07:35:37.000000000","message":"Still, might be not consistent/coherent: in file trove_tempest_plugin\\tests\\base_test.py remains: \n\ncls.db_flavor_ref \u003d CONF.database.flavor_ref\n\nAnyway, is the naming convention somehow related to the fact that a couple of flavor client exists? The clients:\n\ncls.flavors_client \u003d cls.os_primary.flavors_client\ncls.database_flavors_client \u003d cls.os_primary.database_flavors_client\n\n, or another incomprehensible circumstance?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"}],"trove_tempest_plugin/exceptions.py":[{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class ResourceStatusException(exceptions.TempestException):"},{"line_number":20,"context_line":"    message \u003d (\"Resource failed to reach expected status %(expected_status)s. \""},{"line_number":21,"context_line":"               \"Current status: %(current_status)s.\")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_b78d0c7d","line":20,"range":{"start_line":20,"start_character":13,"end_line":20,"end_character":15},"updated":"2018-09-09 11:30:29.000000000","message":"Shouldn\u0027t it be translated with _( method call ?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class ResourceStatusException(exceptions.TempestException):"},{"line_number":20,"context_line":"    message \u003d (\"Resource failed to reach expected status %(expected_status)s. \""},{"line_number":21,"context_line":"               \"Current status: %(current_status)s.\")"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_399a6e6e","line":20,"range":{"start_line":20,"start_character":13,"end_line":20,"end_character":15},"in_reply_to":"3f79a3b5_b78d0c7d","updated":"2018-09-20 12:59:48.000000000","message":"Should we bother with translations in project that is not consumed by end user?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"}],"trove_tempest_plugin/managers/floating_ips.py":[{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class FloatingIPsManagerMixin(object):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    \"\"\"Floating IPs manager"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    Implements helper methods for managing floating IPs."},{"line_number":24,"context_line":"    \"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def _assign_floating_ip(self, instance):"},{"line_number":27,"context_line":"        instance_details \u003d self.instances_admin_client.show("}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_17f400ce","line":24,"range":{"start_line":21,"start_character":4,"end_line":24,"end_character":7},"updated":"2018-09-09 11:30:29.000000000","message":"Do we need to have such comments?","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class FloatingIPsManagerMixin(object):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    \"\"\"Floating IPs manager"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    Implements helper methods for managing floating IPs."},{"line_number":24,"context_line":"    \"\"\""},{"line_number":25,"context_line":""},{"line_number":26,"context_line":"    def _assign_floating_ip(self, instance):"},{"line_number":27,"context_line":"        instance_details \u003d self.instances_admin_client.show("}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_39502ed5","line":24,"range":{"start_line":21,"start_character":4,"end_line":24,"end_character":7},"in_reply_to":"3f79a3b5_17f400ce","updated":"2018-09-20 12:59:48.000000000","message":"I find adding human-readable class descriptions to be a good practice: for rendering official docs from code; encourages docs development.","commit_id":"18ba0db60706548c87856581724410b16420a2c6"}],"trove_tempest_plugin/tests/scenario/test_basic_provisioning.py":[{"author":{"_account_id":28646,"name":"Dariusz Krol","email":"d.krol@samsung.com","username":"dkrol"},"change_message_id":"c37eb2c80f5db2bf1f39986105b4881b171395f2","unresolved":false,"context_lines":[{"line_number":23,"context_line":"    \"\"\"Basic instance provisioning"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    Steps:"},{"line_number":26,"context_line":"        1. Create a database instance."},{"line_number":27,"context_line":"        2. Verify access to instance:"},{"line_number":28,"context_line":"            * perform a database ping"},{"line_number":29,"context_line":"        3. Perform basic database operations:"},{"line_number":30,"context_line":"            * add data to the database"},{"line_number":31,"context_line":"            * verify that data has been persisted"},{"line_number":32,"context_line":"            * remove data"},{"line_number":33,"context_line":"    \"\"\""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @decorators.idempotent_id(\u0027c2d1590e-222a-4b6d-9bdb-b366db9357f8\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_171de00d","line":32,"range":{"start_line":26,"start_character":0,"end_line":32,"end_character":25},"updated":"2018-09-09 11:30:29.000000000","message":"Please add 4. Delete the database instance","commit_id":"18ba0db60706548c87856581724410b16420a2c6"},{"author":{"_account_id":28695,"name":"Bartosz Zurkowski","email":"b.zurkowski@samsung.com","username":"b.zurkowski"},"change_message_id":"ec221d48fc52ed0218b939cf7ae7cf47482721bb","unresolved":false,"context_lines":[{"line_number":23,"context_line":"    \"\"\"Basic instance provisioning"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"    Steps:"},{"line_number":26,"context_line":"        1. Create a database instance."},{"line_number":27,"context_line":"        2. Verify access to instance:"},{"line_number":28,"context_line":"            * perform a database ping"},{"line_number":29,"context_line":"        3. Perform basic database operations:"},{"line_number":30,"context_line":"            * add data to the database"},{"line_number":31,"context_line":"            * verify that data has been persisted"},{"line_number":32,"context_line":"            * remove data"},{"line_number":33,"context_line":"    \"\"\""},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"    @decorators.idempotent_id(\u0027c2d1590e-222a-4b6d-9bdb-b366db9357f8\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"3f79a3b5_59ddea6a","line":32,"range":{"start_line":26,"start_character":0,"end_line":32,"end_character":25},"in_reply_to":"3f79a3b5_171de00d","updated":"2018-09-20 12:59:48.000000000","message":"Good catch!","commit_id":"18ba0db60706548c87856581724410b16420a2c6"}]}