)]}'
{"trove/guestagent/datastore/experimental/postgresql/pgutil.py":[{"author":{"_account_id":15321,"name":"Matthew Van Dijk","email":"mvandijk@tesora.com","username":"mvandijk"},"change_message_id":"d8d06ecbe1704c56ce5186c9e0baeefb271a84bc","unresolved":false,"context_lines":[{"line_number":234,"context_line":"        \"\"\"Query to list grants for a user.\"\"\""},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"        return ("},{"line_number":237,"context_line":"            \"SELECT datname, pg_encoding_to_char(encoding), datcollate\""},{"line_number":238,"context_line":"            \"FROM pg_database \""},{"line_number":239,"context_line":"            \"WHERE datistemplate \u003d false \""},{"line_number":240,"context_line":"            \"AND \u0027user {user}\u003dCTc\u0027 \u003d ANY (datacl)\".format(user\u003duser)"}],"source_content_type":"text/x-python","patch_set":5,"id":"bae84128_f6740837","line":237,"updated":"2016-02-22 20:21:45.000000000","message":"Why do this here? Looks like you are sneaking in a fix to an unrelated bug. This is going to be fixed by https://review.openstack.org/#/c/278120/","commit_id":"25d10e1d7036fd8dee2d6ad884ab6e064896d70b"}],"trove/guestagent/datastore/experimental/postgresql/service/access.py":[{"author":{"_account_id":8932,"name":"Telles Mota Vidal Nóbrega","email":"tellesnobrega@gmail.com","username":"tellesnobrega"},"change_message_id":"acd615b44054692d75377e43f693b5142d976a9a","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        )"},{"line_number":74,"context_line":""},{"line_number":75,"context_line":"    def list_access(self, context, username, hostname):"},{"line_number":76,"context_line":"        \"\"\"List database for which the given user as access."},{"line_number":77,"context_line":"        Return a list of serialized Postgres databases."},{"line_number":78,"context_line":"        \"\"\""},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":20,"id":"1a122d0e_6b74939b","line":76,"updated":"2016-04-25 14:37:09.000000000","message":"s/as/has","commit_id":"1ebb31e3e5930d6229e54d2a1d4546d74c38a1aa"}],"trove/guestagent/datastore/experimental/postgresql/service/database.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"3fb03dcaa3256c12ff0d680d8d3f87024854ed06","unresolved":false,"context_lines":[{"line_number":97,"context_line":"        \"\"\"List all databases on the instance."},{"line_number":98,"context_line":"        Return a paginated list of serialized Postgres databases."},{"line_number":99,"context_line":"        \"\"\""},{"line_number":100,"context_line":"        dbs \u003d [user.serialise() for user in self._get_databases()]"},{"line_number":101,"context_line":"        return pagination.paginate_list(dbs, limit, marker, include_marker)"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def _get_databases(self):"}],"source_content_type":"text/x-python","patch_set":31,"id":"9abb7d3a_d9e17a9c","line":100,"range":{"start_line":100,"start_character":20,"end_line":100,"end_character":29},"updated":"2016-06-02 21:03:12.000000000","message":"Typo here - should be \u0027serialize\u0027","commit_id":"0b60e228854bae8e9d67c7014a481f296d521ad0"}],"trove/guestagent/datastore/experimental/postgresql/service/users.py":[{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"3fb03dcaa3256c12ff0d680d8d3f87024854ed06","unresolved":false,"context_lines":[{"line_number":131,"context_line":"        \"\"\"List all users on the instance along with their access permissions."},{"line_number":132,"context_line":"        Return a paginated list of serialized Postgres users."},{"line_number":133,"context_line":"        \"\"\""},{"line_number":134,"context_line":"        users \u003d [user.serialise() for user in self._get_users(context)]"},{"line_number":135,"context_line":"        return pagination.paginate_list(users, limit, marker, include_marker)"},{"line_number":136,"context_line":""},{"line_number":137,"context_line":"    def _get_users(self, context):"}],"source_content_type":"text/x-python","patch_set":31,"id":"9abb7d3a_99fed236","line":134,"range":{"start_line":134,"start_character":22,"end_line":134,"end_character":31},"updated":"2016-06-02 21:03:12.000000000","message":"Ditto, typo \u0027serialize\u0027","commit_id":"0b60e228854bae8e9d67c7014a481f296d521ad0"}],"trove/guestagent/db/models.py":[{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"4b458da343175bd43decb02183f5a6e6b8606e29","unresolved":false,"context_lines":[{"line_number":60,"context_line":""},{"line_number":61,"context_line":"        # need one or the other, not both, not none (!\u003d ~ XOR)"},{"line_number":62,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":63,"context_line":"            raise ValueError(_(\"Bad args. name: %(name)s, \""},{"line_number":64,"context_line":"                               \"deserializing %(deser)s.\")"},{"line_number":65,"context_line":"                             % ({\u0027name\u0027: bool(name),"},{"line_number":66,"context_line":"                                 \u0027deser\u0027: bool(deserializing)}))"}],"source_content_type":"text/x-python","patch_set":4,"id":"dae33548_662dfa45","line":63,"updated":"2016-02-16 19:35:03.000000000","message":"\"Bad args\" isn\u0027t a very helpful message.  Can you make it more helpful?","commit_id":"47208514edfe232d293da4f9addb368eabde944d"},{"author":{"_account_id":9782,"name":"Morgan Jones","email":"morgan@tesora.com","username":"vgnbkr"},"change_message_id":"4b458da343175bd43decb02183f5a6e6b8606e29","unresolved":false,"context_lines":[{"line_number":552,"context_line":"        # need only one of: deserializing, name, or (name and password)"},{"line_number":553,"context_line":"        if ((not (bool(deserializing) !\u003d bool(name))) or"},{"line_number":554,"context_line":"                (bool(deserializing) and bool(password))):"},{"line_number":555,"context_line":"            raise ValueError(_(\"Bad args. name: %(name)s, \""},{"line_number":556,"context_line":"                               \"password %(pass)s, \""},{"line_number":557,"context_line":"                               \"deserializing %(deser)s.\")"},{"line_number":558,"context_line":"                             % ({\u0027name\u0027: bool(name),"}],"source_content_type":"text/x-python","patch_set":4,"id":"dae33548_e6210a4f","line":555,"updated":"2016-02-16 19:35:03.000000000","message":"\"Bad args\" again.","commit_id":"47208514edfe232d293da4f9addb368eabde944d"},{"author":{"_account_id":15321,"name":"Matthew Van Dijk","email":"mvandijk@tesora.com","username":"mvandijk"},"change_message_id":"d8d06ecbe1704c56ce5186c9e0baeefb271a84bc","unresolved":false,"context_lines":[{"line_number":85,"context_line":"        self._validate_schema_name(value)"},{"line_number":86,"context_line":"        self._name \u003d value"},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    # TODO(pmalik): The collate and character_set properties are datastore"},{"line_number":89,"context_line":"    # specific and should be moved to derived classes that require them."},{"line_number":90,"context_line":"    # They are currently here because many datastores still use the MySQL"},{"line_number":91,"context_line":"    # extensions which try to access them."}],"source_content_type":"text/x-python","patch_set":5,"id":"bae84128_568694db","line":88,"updated":"2016-02-22 20:21:45.000000000","message":"no more todo\u0027s in the code!","commit_id":"25d10e1d7036fd8dee2d6ad884ab6e064896d70b"},{"author":{"_account_id":15321,"name":"Matthew Van Dijk","email":"mvandijk@tesora.com","username":"mvandijk"},"change_message_id":"d8d06ecbe1704c56ce5186c9e0baeefb271a84bc","unresolved":false,"context_lines":[{"line_number":952,"context_line":"        return [\u0027_name\u0027]"},{"line_number":953,"context_line":""},{"line_number":954,"context_line":""},{"line_number":955,"context_line":"# TODO(pmalik): Datastores should be using their own user."},{"line_number":956,"context_line":"# Not this one which is just a MySQL user."},{"line_number":957,"context_line":"class RootUser(MySQLUser):"},{"line_number":958,"context_line":"    \"\"\"Overrides _ignore_users from the MySQLUser class.\"\"\""}],"source_content_type":"text/x-python","patch_set":5,"id":"bae84128_f6fa4842","line":955,"updated":"2016-02-22 20:21:45.000000000","message":"no more todo\u0027s!","commit_id":"25d10e1d7036fd8dee2d6ad884ab6e064896d70b"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"1c1c1b4de4776c7537c3f725e7046c3aab1a5ef6","unresolved":false,"context_lines":[{"line_number":54,"context_line":"class DatastoreSchema(Base):"},{"line_number":55,"context_line":"    \"\"\"Represents a database schema.\"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, name, deserializing\u003dFalse, *args, **kwargs):"},{"line_number":58,"context_line":"        self._name \u003d None"},{"line_number":59,"context_line":"        self._collate \u003d None"},{"line_number":60,"context_line":"        self._character_set \u003d None"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_d1d63bc2","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":42},"updated":"2016-05-13 22:00:21.000000000","message":"I\u0027m wondering if we really need this attribute here. What do you think about the two proposals below?\n\n1) Remove the attribute, always accept `name` and overwrite it if `deserialize` is called.\n\n2) Make deserialize a classmethod (deserialize_schema is already a class method) and keep name as a required attribute.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"3d282aaa8cdc660d6c5d518747f207ee3865a7b2","unresolved":false,"context_lines":[{"line_number":54,"context_line":"class DatastoreSchema(Base):"},{"line_number":55,"context_line":"    \"\"\"Represents a database schema.\"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, name, deserializing\u003dFalse, *args, **kwargs):"},{"line_number":58,"context_line":"        self._name \u003d None"},{"line_number":59,"context_line":"        self._collate \u003d None"},{"line_number":60,"context_line":"        self._character_set \u003d None"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_dae2879f","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":42},"in_reply_to":"dab17558_6d5de378","updated":"2016-05-16 20:21:46.000000000","message":"No, none of the proposals suggest having name as a keyword.\n\nThe proposal number #1 suggests always requiring the `name` attribute and let the `deserialize` call overwrite it since it\u0027s basically overwriting the instance\u0027s dict object (L37)\n\nThe second proposal is to make `deserialize` a classmethod and have `deserialize_schema` call it. The `deserialize` call could return the instance of the class and do all the transformations needed. This is a bigger change, though.\n\nI don\u0027t understand how any of the provided options remove the ability for validating this code.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"e603b27c40238b71679211682f734446e49c896e","unresolved":false,"context_lines":[{"line_number":54,"context_line":"class DatastoreSchema(Base):"},{"line_number":55,"context_line":"    \"\"\"Represents a database schema.\"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, name, deserializing\u003dFalse, *args, **kwargs):"},{"line_number":58,"context_line":"        self._name \u003d None"},{"line_number":59,"context_line":"        self._collate \u003d None"},{"line_number":60,"context_line":"        self._character_set \u003d None"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_340e478c","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":42},"in_reply_to":"dab17558_9442933c","updated":"2016-05-16 21:31:24.000000000","message":"Petr, thanks for the feedback.\n\nA couple of comments:\n\nFor proposal #2, note that I mentioned the deserialize call would return an instance of the class. This might not the most ideal thing to do for this method, though.\n\nFor proposal #1, I don\u0027t think there\u0027s actually a problem. You can create an \"empy\" object with a \"dummy\" name and then have the deserialize method overwrite it.\n\n    \u003e\u003e\u003e class A(object):\n    ...     def __init__(self, name):\n    ...         self.name \u003d name\n    ...     def deserialize(self, o):\n    ...         self.__dict__ \u003d o\n    ... \n    \u003e\u003e\u003e obj \u003d A(\u0027test\u0027)\n    \u003e\u003e\u003e obj.deserialize({\u0027name\u0027: \u0027new_name\u0027})\n    \u003e\u003e\u003e obj.name\n    \u0027new_name\u0027\n    \u003e\u003e\u003e \n\n\nI understand the point behind passing the flag is to validate that either of those are set but I personally don\u0027t see a huge value in that for this specific case. One reason I prefer #2 is because it\u0027d avoid creating an instance of the object with dummy/None data.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":9664,"name":"amrith","email":"amrith.kumar@gmail.com","username":"amrith"},"change_message_id":"d10fdd3158f941786faf6bdc96e7a97836ef39a1","unresolved":false,"context_lines":[{"line_number":54,"context_line":"class DatastoreSchema(Base):"},{"line_number":55,"context_line":"    \"\"\"Represents a database schema.\"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, name, deserializing\u003dFalse, *args, **kwargs):"},{"line_number":58,"context_line":"        self._name \u003d None"},{"line_number":59,"context_line":"        self._collate \u003d None"},{"line_number":60,"context_line":"        self._character_set \u003d None"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_6d5de378","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":42},"in_reply_to":"dab17558_a4d1b3cf","updated":"2016-05-14 20:06:45.000000000","message":"Do I understand correctly that your proposal is that when name\u003dNone, it is implied that one is deserializing?\n\nSeems unintuitive to me. I much rather the idea of having two arguments and making explicit checks on what\u0027s going on. Either one should be provided but not both.\n\nThis code seems to do that better than either provided option which offers no way of validation since deserializing is implied from name.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"7d82e84047d13119fb13b1efb032fca1be9197c1","unresolved":false,"context_lines":[{"line_number":54,"context_line":"class DatastoreSchema(Base):"},{"line_number":55,"context_line":"    \"\"\"Represents a database schema.\"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, name, deserializing\u003dFalse, *args, **kwargs):"},{"line_number":58,"context_line":"        self._name \u003d None"},{"line_number":59,"context_line":"        self._collate \u003d None"},{"line_number":60,"context_line":"        self._character_set \u003d None"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_9442933c","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":42},"in_reply_to":"dab17558_c0e1f62b","updated":"2016-05-16 21:09:55.000000000","message":"* Note that the \u0027deserializing\u0027 is not really visible to the callers (they instantiate concrete objects like MongoDBSchema etc and that explicitly accepts only a name). \nIt\u0027s an implementation detail used by the base class. Unfortunately Python does not have a notion of \u0027protected\u0027 __init__ which would let us hide deserialization from the \u0027public\u0027 interface completely.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f75e3429b151878504a52b2cbe8ee263cce3149e","unresolved":false,"context_lines":[{"line_number":54,"context_line":"class DatastoreSchema(Base):"},{"line_number":55,"context_line":"    \"\"\"Represents a database schema.\"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, name, deserializing\u003dFalse, *args, **kwargs):"},{"line_number":58,"context_line":"        self._name \u003d None"},{"line_number":59,"context_line":"        self._collate \u003d None"},{"line_number":60,"context_line":"        self._character_set \u003d None"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_a4d1b3cf","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":42},"in_reply_to":"dab17558_d1d63bc2","updated":"2016-05-13 22:26:48.000000000","message":"I realize #2 might not be super clear. The proposal is to make the `deserialize` method in the Base class a classmethod that could be called directly from `deserialize_schema`.\n\nIf #2 makes sense, I wonder if `deserialize_schema` is actually needed. We could perhaps move the `deserialize_schema` body into the `deserialize` method but I might be missing some context. Certainly not something I\u0027d recommend doing in this patch.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"6e871179da9da333e343fc0424e138def434f453","unresolved":false,"context_lines":[{"line_number":54,"context_line":"class DatastoreSchema(Base):"},{"line_number":55,"context_line":"    \"\"\"Represents a database schema.\"\"\""},{"line_number":56,"context_line":""},{"line_number":57,"context_line":"    def __init__(self, name, deserializing\u003dFalse, *args, **kwargs):"},{"line_number":58,"context_line":"        self._name \u003d None"},{"line_number":59,"context_line":"        self._collate \u003d None"},{"line_number":60,"context_line":"        self._character_set \u003d None"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_c0e1f62b","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":42},"in_reply_to":"dab17558_dae2879f","updated":"2016-05-16 21:01:30.000000000","message":"I think there are some issues with your proposals.\n\nI don\u0027t think we can make the base \u0027deserialize\u0027 into a classmethod because it loads the properties into the object itself. An \u0027empty\u0027 object has to exist first so that we can deserialize (load) into it. (your suggestion #2)\nThe \u0027deserializing\u0027 flag in the init is to skip the validation. It allows us to build this \u0027empty\u0027 object. The validation would otherwise not let us build the object with name\u003dNone (your suggestion #1).\n\nThe callers would normally create objects via the init method (and hence getting input validation) or deserialize from API payload with \u0027deserialize_...\u0027 method.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"1c1c1b4de4776c7537c3f725e7046c3aab1a5ef6","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self._character_set \u003d None"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # need one or the other, not both, not none (!\u003d ~ XOR)"},{"line_number":63,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":64,"context_line":"            raise ValueError(_(\"Name may not be specified while \""},{"line_number":65,"context_line":"                               \"deserializing.\"))"},{"line_number":66,"context_line":"        if not deserializing:"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_d1bf5bfa","line":63,"range":{"start_line":63,"start_character":16,"end_line":63,"end_character":20},"updated":"2016-05-13 22:00:21.000000000","message":"nit: why do you need bool here?\n\nAlso, what do you think about doing something like the example below instead:\n\n    if name is not None and deserializing:\n        ...\n\nor\n\n    if name and deserializing:\n        ...\n\nditto for other cases in this file.\n\nP.S: If we move forward w/o `deserializing` you won\u0027t need this check here.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"e603b27c40238b71679211682f734446e49c896e","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self._character_set \u003d None"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # need one or the other, not both, not none (!\u003d ~ XOR)"},{"line_number":63,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":64,"context_line":"            raise ValueError(_(\"Name may not be specified while \""},{"line_number":65,"context_line":"                               \"deserializing.\"))"},{"line_number":66,"context_line":"        if not deserializing:"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_d4909bef","line":63,"range":{"start_line":63,"start_character":16,"end_line":63,"end_character":20},"in_reply_to":"dab17558_80240e0d","updated":"2016-05-16 21:31:24.000000000","message":"I understand what XOR is but I wonder why we need to call `bool` on `deserializing` which is supposed to be a boolean to begin with. \n\nThe two versions of this if-statement I mentioned in my comment would achieve the same here. The latter would validate for nullness and emptyness in the name.\n\nWhat took me a bit to parse, though, is the motivation of this check and the need of the new attribute.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":14576,"name":"Petr Malik","email":"pmalik@tesora.com","username":"pmalik"},"change_message_id":"6e871179da9da333e343fc0424e138def434f453","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self._character_set \u003d None"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # need one or the other, not both, not none (!\u003d ~ XOR)"},{"line_number":63,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":64,"context_line":"            raise ValueError(_(\"Name may not be specified while \""},{"line_number":65,"context_line":"                               \"deserializing.\"))"},{"line_number":66,"context_line":"        if not deserializing:"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_80240e0d","line":63,"range":{"start_line":63,"start_character":16,"end_line":63,"end_character":20},"in_reply_to":"dab17558_d1bf5bfa","updated":"2016-05-16 21:01:30.000000000","message":"XOR is \u0027one or the other, but not both\u0027. That\u0027s exactly what we want.\n\n- either we are deserializing and then there should not be name given because we will fill the object from the payload and any user-defined name would be lost.\n\n- or we are not deserializing and then there must be a name because a schema without a name is a not valid object.\n\nPython XOR works with normalized (converted to bools) variables. That\u0027s whe they are wrapped.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"ca674f2f15046a083676e91bac4e419e58334229","unresolved":false,"context_lines":[{"line_number":60,"context_line":"        self._character_set \u003d None"},{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # need one or the other, not both, not none (!\u003d ~ XOR)"},{"line_number":63,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":64,"context_line":"            raise ValueError(_(\"Name may not be specified while \""},{"line_number":65,"context_line":"                               \"deserializing.\"))"},{"line_number":66,"context_line":"        if not deserializing:"}],"source_content_type":"text/x-python","patch_set":23,"id":"dab17558_b714f9e8","line":63,"range":{"start_line":63,"start_character":16,"end_line":63,"end_character":20},"in_reply_to":"dab17558_d4909bef","updated":"2016-05-16 21:47:44.000000000","message":"Just to be clear (just noticed my comment is a bit misleading), I meant to say that both the proposals would\u0027ve achieved the same as far as the error being raised goes. They are not exact XORs, nothing an elif couldn\u0027t achieve though.","commit_id":"dfe411650951d6cc50c4ef89d4786aabb16a1765"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"a192ff3a6083ba5932ff49d046540000523c97aa","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # need one or the other, not both, not none (!\u003d ~ XOR)"},{"line_number":63,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":64,"context_line":"            raise ValueError(_(\"Name may not be specified while \""},{"line_number":65,"context_line":"                               \"deserializing.\"))"},{"line_number":66,"context_line":"        if not deserializing:"},{"line_number":67,"context_line":"            self.name \u003d name"}],"source_content_type":"text/x-python","patch_set":24,"id":"dab17558_77dbb17c","line":64,"updated":"2016-05-16 21:42:16.000000000","message":"Since this check is a XOR you may want to change the error message to something more generic. I read this as \"you passed a value for `name` and set `deserializing` to True\". However, this will also be triggered if neither is passed. For example, name is None and deserializing is False.\n\nThis is another reason why I\u0027d prefer this if to be broken down.","commit_id":"a8686f1f1e0b72de1f22d972e9772ba223010a7b"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"66260e29823f6ed2c6060deab914dc95a141594a","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # If both or neither are passed in this is a bug."},{"line_number":63,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":64,"context_line":"            raise RuntimeError(\"Bug in DatastoreSchema()\")"},{"line_number":65,"context_line":"        if not deserializing:"},{"line_number":66,"context_line":"            self.name \u003d name"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"dab17558_5715923d","line":64,"updated":"2016-05-17 19:34:29.000000000","message":"nit: I\u0027d have used something like:\n\n\"Expected either the `name` attribute or `deserializing`\" \n\nnit2: It also feels weird do use a `RuntimeError` here. Perhaps a `ValueError` would have been better here.","commit_id":"843fa81a51517546217fbfd051b750e3d4d2786b"},{"author":{"_account_id":10215,"name":"Peter Stachowski","email":"peter@tesora.com","username":"peterstac"},"change_message_id":"a18054884fbed73b659e828375d3d4616a5d780f","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # If both or neither are passed in this is a bug."},{"line_number":63,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":64,"context_line":"            raise RuntimeError(\"Bug in DatastoreSchema()\")"},{"line_number":65,"context_line":"        if not deserializing:"},{"line_number":66,"context_line":"            self.name \u003d name"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"dab17558_f7b486f1","line":64,"in_reply_to":"dab17558_5715923d","updated":"2016-05-17 19:44:40.000000000","message":"This represents a coding bug and should never be seen by either an end user or operator, so I\u0027m ok with it. (When the person coding it hits this bug they should see the comment and be able to take the appropriate action.)","commit_id":"843fa81a51517546217fbfd051b750e3d4d2786b"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"fbcf143ccc0fc875193bddb6585833e54e3a7f40","unresolved":false,"context_lines":[{"line_number":61,"context_line":""},{"line_number":62,"context_line":"        # If both or neither are passed in this is a bug."},{"line_number":63,"context_line":"        if not (bool(deserializing) !\u003d bool(name)):"},{"line_number":64,"context_line":"            raise RuntimeError(\"Bug in DatastoreSchema()\")"},{"line_number":65,"context_line":"        if not deserializing:"},{"line_number":66,"context_line":"            self.name \u003d name"},{"line_number":67,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"dab17558_bdf2dd6c","line":64,"in_reply_to":"dab17558_f7b486f1","updated":"2016-05-17 20:28:46.000000000","message":"Fair point! Thanks.","commit_id":"843fa81a51517546217fbfd051b750e3d4d2786b"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"e1ecfd3c30738229757655726ad1fe83e956c1d1","unresolved":false,"context_lines":[{"line_number":1056,"context_line":"    def __init__(self, password\u003dNone, *args, **kwargs):"},{"line_number":1057,"context_line":"        if password is None:"},{"line_number":1058,"context_line":"            password \u003d utils.generate_random_password()"},{"line_number":1059,"context_line":"        super(CassandraRootUser, self).__init__(\"root\", password\u003dpassword,"},{"line_number":1060,"context_line":"                                                *args, **kwargs)"}],"source_content_type":"text/x-python","patch_set":26,"id":"9abb7d3a_0112fa08","line":1059,"range":{"start_line":1059,"start_character":14,"end_line":1059,"end_character":31},"updated":"2016-05-31 17:35:03.000000000","message":"needs to be CouchDBRootUser","commit_id":"ac13556a6aad2bdd16ae70134cb1e10c09d882d0"},{"author":{"_account_id":10295,"name":"mariam john","email":"mariamj@us.ibm.com","username":"mariamj"},"change_message_id":"74fcf66b3d596d674ea2479cd642df7f1843fd2c","unresolved":false,"context_lines":[{"line_number":193,"context_line":"    name_regex \u003d re.compile(r\u0027^[a-z][a-z0-9_$()+/-]*$\u0027)"},{"line_number":194,"context_line":""},{"line_number":195,"context_line":"    def __init__(self, name, *args, **kwargs):"},{"line_number":196,"context_line":"        super(CouchDBSchema, self).__init__(name, *args, **kwargs)"},{"line_number":197,"context_line":"        self._ignore_dbs \u003d cfg.get_ignored_dbs()"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"    @property"}],"source_content_type":"text/x-python","patch_set":29,"id":"9abb7d3a_27d38e61","line":196,"updated":"2016-06-01 18:22:39.000000000","message":"since the check for deserializing was moved to the base class, it calls the setter for property \u0027name\u0027 before self._ignore_dbs gets set in line 197 and hence throws an error while validating the name.","commit_id":"f80d7ae276d7035ee3421d375f1506dc7910472c"}]}