)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"9a8ace33aa7c17a4f442d85704e54b6680c94aef","unresolved":false,"context_lines":[{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Add version column"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This extends versioned objects and uses the obj_make_compatible"},{"line_number":10,"context_line":"methods to also convert between versioned data stored in the database."},{"line_number":11,"context_line":""},{"line_number":12,"context_line":"Version column is introduced to make sure reading the DB object"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":36,"id":"9a30ddce_bafff965","line":9,"updated":"2017-03-08 19:06:08.000000000","message":"I\u0027d rephrase this to indicate *how* it extends the objects.","commit_id":"c49f2744d437de4c8b1784430996344bb47ae8a3"}],"etc/ironic/ironic.conf.sample":[{"author":{"_account_id":19593,"name":"Anup Navare","email":"anup.d.navare@intel.com","username":"anup"},"change_message_id":"77d9fa9dddafd97af3f8e77fc2fd2e950d284152","unresolved":false,"context_lines":[{"line_number":2570,"context_line":"# value)"},{"line_number":2571,"context_line":"#sasl_config_name \u003d"},{"line_number":2572,"context_line":""},{"line_number":2573,"context_line":"# SASL realm to use if no realm present in username (string"},{"line_number":2574,"context_line":"# value)"},{"line_number":2575,"context_line":"#sasl_default_realm \u003d"},{"line_number":2576,"context_line":""},{"line_number":2577,"context_line":"# DEPRECATED: User name for message broker authentication"},{"line_number":2578,"context_line":"# (string value)"}],"source_content_type":"application/octet-stream","patch_set":46,"id":"ff0f0b1f_21cdf4b7","line":2575,"range":{"start_line":2573,"start_character":1,"end_line":2575,"end_character":21},"updated":"2017-05-19 21:38:04.000000000","message":"Unrelated, I think conf should be regenerated.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":19593,"name":"Anup Navare","email":"anup.d.navare@intel.com","username":"anup"},"change_message_id":"77d9fa9dddafd97af3f8e77fc2fd2e950d284152","unresolved":false,"context_lines":[{"line_number":2802,"context_line":"# The maximum number of attempts to re-send a notification"},{"line_number":2803,"context_line":"# message which failed to be delivered due to a recoverable"},{"line_number":2804,"context_line":"# error. 0 - No retry, -1 - indefinite (integer value)"},{"line_number":2805,"context_line":"#retry \u003d -1"},{"line_number":2806,"context_line":""},{"line_number":2807,"context_line":""},{"line_number":2808,"context_line":"[oslo_messaging_rabbit]"}],"source_content_type":"application/octet-stream","patch_set":46,"id":"ff0f0b1f_01d0b04f","line":2805,"range":{"start_line":2805,"start_character":1,"end_line":2805,"end_character":11},"updated":"2017-05-19 21:38:04.000000000","message":"unrelated","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"}],"ironic/conf/default.py":[{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"b19eea7e80345ccdf5d208a3ff841f7f4db81a4e","unresolved":false,"context_lines":[{"line_number":267,"context_line":"               # TODO(xek): mutable\u003dTrue,"},{"line_number":268,"context_line":"               help\u003d_(\u0027WARNING: This configuration option is part of the \u0027"},{"line_number":269,"context_line":"                      \u0027incomplete rolling upgrades work. Do not change this \u0027"},{"line_number":270,"context_line":"                      \u0027from the default value. \u0027"},{"line_number":271,"context_line":"                      \u0027Used for rolling upgrades. Setting this option \u0027"},{"line_number":272,"context_line":"                      \u0027downgrades (or pins) the internal ironic RPC \u0027"},{"line_number":273,"context_line":"                      \u0027communication to the specified version to enable \u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"bf091321_0d4f17be","line":270,"updated":"2017-06-08 07:00:37.000000000","message":"Previously this value has no effect but now it is. Shouldn\u0027t we mention this in release notes? or force value to None always until we activate this variable?","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6401d368c562b9443a3a17dfad69a86a1f54524b","unresolved":false,"context_lines":[{"line_number":267,"context_line":"               # TODO(xek): mutable\u003dTrue,"},{"line_number":268,"context_line":"               help\u003d_(\u0027WARNING: This configuration option is part of the \u0027"},{"line_number":269,"context_line":"                      \u0027incomplete rolling upgrades work. Do not change this \u0027"},{"line_number":270,"context_line":"                      \u0027from the default value. \u0027"},{"line_number":271,"context_line":"                      \u0027Used for rolling upgrades. Setting this option \u0027"},{"line_number":272,"context_line":"                      \u0027downgrades (or pins) the internal ironic RPC \u0027"},{"line_number":273,"context_line":"                      \u0027communication to the specified version to enable \u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"bf091321_853f73d5","line":270,"in_reply_to":"bf091321_0d4f17be","updated":"2017-06-08 12:28:40.000000000","message":"Hmm. Wrt release notes, the feature isn\u0027t complete so there are no release notes yet. I don\u0027t think it makes sense to add a release note that says here\u0027s an incomplete feature.\n\nwrt default value or forcing it to None. I suppose we could do that but I\u0027m not sure that it buys us much except extra hoops while we\u0027re working on a feature.\n\nAlso -- nothing breaks if this config is changed (unless you see Mark Goddard\u0027s physnet code [1] breaking cuz of this but even then, it should break I think!), and we want this config to work for the patches after this, so we can test the feature.\n\n[1] https://review.openstack.org/#/c/469931/","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"0392e5ffaaa836b8d63ca7958c1b2be37e870925","unresolved":false,"context_lines":[{"line_number":267,"context_line":"               # TODO(xek): mutable\u003dTrue,"},{"line_number":268,"context_line":"               help\u003d_(\u0027WARNING: This configuration option is part of the \u0027"},{"line_number":269,"context_line":"                      \u0027incomplete rolling upgrades work. Do not change this \u0027"},{"line_number":270,"context_line":"                      \u0027from the default value. \u0027"},{"line_number":271,"context_line":"                      \u0027Used for rolling upgrades. Setting this option \u0027"},{"line_number":272,"context_line":"                      \u0027downgrades (or pins) the internal ironic RPC \u0027"},{"line_number":273,"context_line":"                      \u0027communication to the specified version to enable \u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"bf091321_96d0c7cb","line":270,"in_reply_to":"bf091321_1691579a","updated":"2017-06-08 13:58:09.000000000","message":"My bad. I had an old version of master; the only change so far is port.pyhsical_network was added. That is OK so far because the user cannot actually set it yet. And the way that patch series is going, they won\u0027t be able to if this is pinned, so  nothing \u0027breaks\u0027 but they won\u0027t get the behaviour they may have been expecting, but the code works as planned. That\u0027s what they get for not reading the help :-)","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"784d629dffce4be5945b4c2875d85511dbcd7378","unresolved":false,"context_lines":[{"line_number":267,"context_line":"               # TODO(xek): mutable\u003dTrue,"},{"line_number":268,"context_line":"               help\u003d_(\u0027WARNING: This configuration option is part of the \u0027"},{"line_number":269,"context_line":"                      \u0027incomplete rolling upgrades work. Do not change this \u0027"},{"line_number":270,"context_line":"                      \u0027from the default value. \u0027"},{"line_number":271,"context_line":"                      \u0027Used for rolling upgrades. Setting this option \u0027"},{"line_number":272,"context_line":"                      \u0027downgrades (or pins) the internal ironic RPC \u0027"},{"line_number":273,"context_line":"                      \u0027communication to the specified version to enable \u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"bf091321_1691579a","line":270,"in_reply_to":"bf091321_40f6e991","updated":"2017-06-08 13:43:19.000000000","message":"That\u0027s a good point. I think we\u0027re ok though (or err the person that set the option is ok). I mean, if someone pins this to ocata, they will get ocata objects. To date (although I could have missed something) there is nothing new wrt objects in master, that pinning to ocata would cause a problem. in fact, that\u0027s the whole idea of rolling upgrades, etc, pinning just means you don\u0027t/shouldn\u0027t get access to new changes in master.\n\nOk, I just looked (at common/release_mappings.py). So far, nothing wrt rpc/object versions has changed between master \u0026 ocata. So far so good :-)","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":14525,"name":"Vasyl Saienko","email":"vsaienko@mirantis.com","username":"vsaienko"},"change_message_id":"74b6d6d3c6ebeadca5810b9f202931117542b45d","unresolved":false,"context_lines":[{"line_number":267,"context_line":"               # TODO(xek): mutable\u003dTrue,"},{"line_number":268,"context_line":"               help\u003d_(\u0027WARNING: This configuration option is part of the \u0027"},{"line_number":269,"context_line":"                      \u0027incomplete rolling upgrades work. Do not change this \u0027"},{"line_number":270,"context_line":"                      \u0027from the default value. \u0027"},{"line_number":271,"context_line":"                      \u0027Used for rolling upgrades. Setting this option \u0027"},{"line_number":272,"context_line":"                      \u0027downgrades (or pins) the internal ironic RPC \u0027"},{"line_number":273,"context_line":"                      \u0027communication to the specified version to enable \u0027"}],"source_content_type":"text/x-python","patch_set":49,"id":"bf091321_40f6e991","line":270,"in_reply_to":"bf091321_853f73d5","updated":"2017-06-08 12:51:50.000000000","message":"My concern here is the following, previously we told that this option has no effect and we can\u0027t guarantee that someone didn\u0027t set this option. So if someone set this option with this patch, we will save objects to database with honor of this  https://review.openstack.org/#/c/412397/49/ironic/objects/base.py@241","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"}],"ironic/db/sqlalchemy/alembic/versions/868cb606a74a_add_version_field_in_base_class.py":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"57e7d564c64bafc404280d9fc0ba8dc1e2aeb968","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":32,"context_line":"    op.add_column(\u0027conductors\u0027,"},{"line_number":33,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":34,"context_line":"    op.add_column(\u0027node_tags\u0027,"},{"line_number":35,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":36,"context_line":"    op.add_column(\u0027nodes\u0027,"},{"line_number":37,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a430d35_ffadab84","line":34,"updated":"2017-02-07 17:47:55.000000000","message":"Why does this have a version column?\n\nIt doesn\u0027t have a release mapping","commit_id":"34e9e73a9747468beec3026c8fdbfa4a74816057"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"57e7d564c64bafc404280d9fc0ba8dc1e2aeb968","unresolved":false,"context_lines":[{"line_number":43,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":44,"context_line":"    op.add_column(\u0027volume_targets\u0027,"},{"line_number":45,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":46,"context_line":"    op.add_column(\u0027conductor_hardware_interfaces\u0027,"},{"line_number":47,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a430d35_7fc29b35","line":46,"updated":"2017-02-07 17:47:55.000000000","message":"ditto here","commit_id":"34e9e73a9747468beec3026c8fdbfa4a74816057"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"9a8ace33aa7c17a4f442d85704e54b6680c94aef","unresolved":false,"context_lines":[{"line_number":31,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":32,"context_line":"    op.add_column(\u0027conductors\u0027,"},{"line_number":33,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":34,"context_line":"    op.add_column(\u0027node_tags\u0027,"},{"line_number":35,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":36,"context_line":"    op.add_column(\u0027nodes\u0027,"},{"line_number":37,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"}],"source_content_type":"text/x-python","patch_set":36,"id":"9a30ddce_3ac6a948","line":34,"updated":"2017-03-08 19:06:08.000000000","message":"++ to Mario\u0027s comment","commit_id":"c49f2744d437de4c8b1784430996344bb47ae8a3"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"213227856b3c0710c9d5eec37b2a4b73e4fe69eb","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    #             to IronicBase class. All inherited classes/tables have"},{"line_number":32,"context_line":"    #             this new column."},{"line_number":33,"context_line":"    op.add_column(\u0027chassis\u0027,"},{"line_number":34,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":35,"context_line":"    op.add_column(\u0027conductors\u0027,"},{"line_number":36,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":37,"context_line":"    op.add_column(\u0027node_tags\u0027,"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_4bff4567","line":34,"range":{"start_line":34,"start_character":56,"end_line":34,"end_character":58},"updated":"2017-06-09 22:24:09.000000000","message":"For some silly reason, 15 chars worries me.... but that also seems like a completely unreasonable worry. :)","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a765829eea932f7e644e1d0aa8235e7039fe9539","unresolved":false,"context_lines":[{"line_number":31,"context_line":"    #             to IronicBase class. All inherited classes/tables have"},{"line_number":32,"context_line":"    #             this new column."},{"line_number":33,"context_line":"    op.add_column(\u0027chassis\u0027,"},{"line_number":34,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":35,"context_line":"    op.add_column(\u0027conductors\u0027,"},{"line_number":36,"context_line":"                  sa.Column(\u0027version\u0027, sa.String(length\u003d15), nullable\u003dTrue))"},{"line_number":37,"context_line":"    op.add_column(\u0027node_tags\u0027,"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_fd1229b1","line":34,"range":{"start_line":34,"start_character":56,"end_line":34,"end_character":58},"in_reply_to":"7f231b9d_4bff4567","updated":"2017-06-12 20:07:52.000000000","message":"Now that I am looking at this, I don\u0027t recall where 15 came from. If we assume a version is in the form: aaaa.bbbb.cccc, I think we\u0027re good. If versions go beyond that, then ironic objects will either have been poorly designed or a huge success :-)","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"}],"ironic/db/sqlalchemy/models.py":[{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"9c7287efd58eb634512d3eef8c4b6a95c3d3f040","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    metadata \u003d None"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    version \u003d Column(String(15), nullable\u003dTrue)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def as_dict(self):"},{"line_number":56,"context_line":"        d \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_a69c976a","line":53,"updated":"2016-12-29 15:49:14.000000000","message":"There is NodeTag model that is not represents any objects.","commit_id":"bbf7aa76b14d7557063efa261e3690fbd477b389"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"852b88b519e7df56085dcae3c5a1625cbca14f04","unresolved":false,"context_lines":[{"line_number":50,"context_line":""},{"line_number":51,"context_line":"    metadata \u003d None"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    version \u003d Column(String(15), nullable\u003dTrue)"},{"line_number":54,"context_line":""},{"line_number":55,"context_line":"    def as_dict(self):"},{"line_number":56,"context_line":"        d \u003d {}"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba5201f7_5c95733e","line":53,"in_reply_to":"ba5201f7_a69c976a","updated":"2016-12-30 07:48:35.000000000","message":"I saw that, I don\u0027t think it absolutely needs a model. But you are welcome to propose a patch.","commit_id":"bbf7aa76b14d7557063efa261e3690fbd477b389"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"3b651ea6fb475a7a785f223ae9be6134a3488949","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"from ironic.conf import CONF"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"_DEFAULT_SQL_CONNECTION \u003d \u0027sqlite:///\u0027 + path.join(\u0027$state_path\u0027,"},{"line_number":36,"context_line":"                                                   \u0027ironic.sqlite\u0027)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"bff0334d_479c390d","line":35,"updated":"2017-04-10 23:30:26.000000000","message":"why not use the path.state_path_def as in previous version of this patch? For me it looked like a right tool for the job","commit_id":"9c11d102e6eaef521f3276c7c4d8602969596fbd"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8b29d36e2f30048cd46be7661f6f9e77aa15839f","unresolved":false,"context_lines":[{"line_number":32,"context_line":""},{"line_number":33,"context_line":"from ironic.conf import CONF"},{"line_number":34,"context_line":""},{"line_number":35,"context_line":"_DEFAULT_SQL_CONNECTION \u003d \u0027sqlite:///\u0027 + path.join(\u0027$state_path\u0027,"},{"line_number":36,"context_line":"                                                   \u0027ironic.sqlite\u0027)"},{"line_number":37,"context_line":""},{"line_number":38,"context_line":""}],"source_content_type":"text/x-python","patch_set":42,"id":"bff0334d_be68ab1c","line":35,"in_reply_to":"bff0334d_479c390d","updated":"2017-04-11 03:04:51.000000000","message":"this was changed in a separate patch (the change is on master). It has nothing to do with this patch.","commit_id":"9c11d102e6eaef521f3276c7c4d8602969596fbd"}],"ironic/objects/base.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3a20f1f68ee0d68124dcc9dec656bf9ba1e8b04f","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    OBJ_SERIAL_NAMESPACE \u003d \u0027ironic_object\u0027"},{"line_number":56,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    _original_version \u003d None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @property"},{"line_number":61,"context_line":"    def _unpinned(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_7efd853a","line":58,"updated":"2016-12-20 01:20:06.000000000","message":"why do you call it \u0027original_version\u0027 ? I guess if you had a description of what this is for, it would help me understand.\n\nI mean, as opposed to eg, calling this \u0027_version\u0027 or \u0027_something_version\u0027.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"4e80db61a4c2dab9d8dd66965737e2a315f66472","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    OBJ_SERIAL_NAMESPACE \u003d \u0027ironic_object\u0027"},{"line_number":56,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    _original_version \u003d None"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @property"},{"line_number":61,"context_line":"    def _unpinned(self):"}],"source_content_type":"text/x-python","patch_set":2,"id":"da4df55a_87f0df68","line":58,"in_reply_to":"1a6eadb0_7efd853a","updated":"2016-12-30 07:48:03.000000000","message":"This is actually what the attribute VERSION is also used for (it is set when an object is deserialized), so I modified the code to use self.__class__.VERSION or self.VERSION instead.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3a20f1f68ee0d68124dcc9dec656bf9ba1e8b04f","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        If object is unpinned, obj_make_compatible methods are not"},{"line_number":65,"context_line":"        called, even if pin_release_version configuration is set."},{"line_number":66,"context_line":"        \"\"\""},{"line_number":67,"context_line":"        return self._original_version \u003d\u003d self.VERSION"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    # TODO(lintan) Refactor these fields and create PersistentObject and"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_be72bd0f","line":66,"updated":"2016-12-20 01:20:06.000000000","message":"I am thinking that we might want to err on the side of more documentation, for future maintainers of this code.\n\nThis is true if the object is unpinned (and false otherwise), right?","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"4e80db61a4c2dab9d8dd66965737e2a315f66472","unresolved":false,"context_lines":[{"line_number":63,"context_line":""},{"line_number":64,"context_line":"        If object is unpinned, obj_make_compatible methods are not"},{"line_number":65,"context_line":"        called, even if pin_release_version configuration is set."},{"line_number":66,"context_line":"        \"\"\""},{"line_number":67,"context_line":"        return self._original_version \u003d\u003d self.VERSION"},{"line_number":68,"context_line":""},{"line_number":69,"context_line":"    # TODO(lintan) Refactor these fields and create PersistentObject and"}],"source_content_type":"text/x-python","patch_set":2,"id":"da4df55a_e2d439be","line":66,"in_reply_to":"1a6eadb0_be72bd0f","updated":"2016-12-30 07:48:03.000000000","message":"Yes, I updated the docstring with more explanation of what it is used for.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"dc4c4d3741131ca7ba93ab29fefa333768f0e6bb","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        if target_version !\u003d self.VERSION:  # convert changes to older format"},{"line_number":101,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":102,"context_line":"                changes, target_version, version_manifest)"},{"line_number":103,"context_line":"        if self._original_version !\u003d target_version:  # version change"},{"line_number":104,"context_line":"            changes[\u0027version\u0027] \u003d target_version"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_7d303908","line":101,"updated":"2016-12-19 18:08:48.000000000","message":"It\u0027d be good to have some test coverage for this line in test_objects.py.\n\nThis should work based on reading the oslo code, but the behavior of obj_make_compatible_from_manifest isn\u0027t well-documented. A test case would avoid regressions if this method changed behavior in newer versions of oslo versionedobjects.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"4e80db61a4c2dab9d8dd66965737e2a315f66472","unresolved":false,"context_lines":[{"line_number":98,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        if target_version !\u003d self.VERSION:  # convert changes to older format"},{"line_number":101,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":102,"context_line":"                changes, target_version, version_manifest)"},{"line_number":103,"context_line":"        if self._original_version !\u003d target_version:  # version change"},{"line_number":104,"context_line":"            changes[\u0027version\u0027] \u003d target_version"}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5201f7_4a1db70e","line":101,"in_reply_to":"1a6eadb0_7d303908","updated":"2016-12-30 07:48:03.000000000","message":"Done","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3a20f1f68ee0d68124dcc9dec656bf9ba1e8b04f","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        objver \u003d db_object[\u0027version\u0027]"},{"line_number":122,"context_line":"        obj._original_version \u003d objver"},{"line_number":123,"context_line":"        if objver is None:  # backfill versions for newton release"},{"line_number":124,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027newton\u0027][\u0027objects\u0027][objname]"},{"line_number":125,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"},{"line_number":126,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":127,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_2109322d","line":124,"updated":"2016-12-20 01:20:06.000000000","message":"we added volume_connector, volume_target objects after newton, ie they will be in this upcoming ocata release. So this will raise an exception. Can we initialize the versions (in theDB) to be the current volume_target \u0026 volume_connector versions respectively?","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"4e80db61a4c2dab9d8dd66965737e2a315f66472","unresolved":false,"context_lines":[{"line_number":121,"context_line":"        objver \u003d db_object[\u0027version\u0027]"},{"line_number":122,"context_line":"        obj._original_version \u003d objver"},{"line_number":123,"context_line":"        if objver is None:  # backfill versions for newton release"},{"line_number":124,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027newton\u0027][\u0027objects\u0027][objname]"},{"line_number":125,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"},{"line_number":126,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":127,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"}],"source_content_type":"text/x-python","patch_set":2,"id":"da4df55a_02b3fd98","line":124,"in_reply_to":"1a6eadb0_2109322d","updated":"2016-12-30 07:48:03.000000000","message":"These tables are empty right now, and any newly saved object will have a version, so the execution wouldn\u0027t go into the if clause. But I\u0027ll add a \u00271.0\u0027 default just in case someone wants to upgrade from master.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"dc4c4d3741131ca7ba93ab29fefa333768f0e6bb","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        obj._original_version \u003d objver"},{"line_number":123,"context_line":"        if objver is None:  # backfill versions for newton release"},{"line_number":124,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027newton\u0027][\u0027objects\u0027][objname]"},{"line_number":125,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"},{"line_number":126,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":127,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_3d4961d1","line":125,"updated":"2016-12-19 18:08:48.000000000","message":"Can we add some test coverage for this case as well as the backfilling on L123?\n\nThe backfilling can be tested by mocking out versions.RELEASE_MAPPING.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"4e80db61a4c2dab9d8dd66965737e2a315f66472","unresolved":false,"context_lines":[{"line_number":122,"context_line":"        obj._original_version \u003d objver"},{"line_number":123,"context_line":"        if objver is None:  # backfill versions for newton release"},{"line_number":124,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027newton\u0027][\u0027objects\u0027][objname]"},{"line_number":125,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"},{"line_number":126,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":127,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"},{"line_number":128,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ba5201f7_a5a014a7","line":125,"in_reply_to":"1a6eadb0_3d4961d1","updated":"2016-12-30 07:48:03.000000000","message":"I added some tests of _from_db_object, since there where none.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3a20f1f68ee0d68124dcc9dec656bf9ba1e8b04f","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    def _get_version_manifest():"},{"line_number":151,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":152,"context_line":"        if pin:"},{"line_number":153,"context_line":"            return versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def _get_target_version(self):"},{"line_number":156,"context_line":"        if self._unpinned:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_81bf66fb","line":153,"updated":"2016-12-20 01:20:06.000000000","message":"that is a dictionary, right? Otherwise, it returns None? (if so, would like it to be explicit that we are returning None.) Wondering if it makes sense to return {}, but I guess it depends on how this is used.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"4e80db61a4c2dab9d8dd66965737e2a315f66472","unresolved":false,"context_lines":[{"line_number":150,"context_line":"    def _get_version_manifest():"},{"line_number":151,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":152,"context_line":"        if pin:"},{"line_number":153,"context_line":"            return versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":154,"context_line":""},{"line_number":155,"context_line":"    def _get_target_version(self):"},{"line_number":156,"context_line":"        if self._unpinned:"}],"source_content_type":"text/x-python","patch_set":2,"id":"da4df55a_6754db20","line":153,"in_reply_to":"1a6eadb0_81bf66fb","updated":"2016-12-30 07:48:03.000000000","message":"Done","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"3a20f1f68ee0d68124dcc9dec656bf9ba1e8b04f","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            return self.VERSION"},{"line_number":158,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":159,"context_line":"        if version_manifest:"},{"line_number":160,"context_line":"            return version_manifest[self.obj_name()]"},{"line_number":161,"context_line":"        return self.VERSION"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"1a6eadb0_c1715e46","line":160,"updated":"2016-12-20 01:20:06.000000000","message":"should we play it safe and do a .get(self.obj_name()) instead? it isn\u0027t clear to me how we handle new objects, or if they have pinned to some older release (i know, pinning isn\u0027t meant for that but who knows what they are doing) that doesn\u0027t have that object. Although i guess, maybe we want it to barf soon in that case. Not sure.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"4e80db61a4c2dab9d8dd66965737e2a315f66472","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            return self.VERSION"},{"line_number":158,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":159,"context_line":"        if version_manifest:"},{"line_number":160,"context_line":"            return version_manifest[self.obj_name()]"},{"line_number":161,"context_line":"        return self.VERSION"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"da4df55a_e25b59f4","line":160,"in_reply_to":"1a6eadb0_c1715e46","updated":"2016-12-30 07:48:03.000000000","message":"I think we should fail fast in this case, since new objects, which are not present in the previous version can\u0027t be pinned to that version. At the same time, they shouldn\u0027t be used to send data to be read by the previous release.\n\nNew objects would typically be used in newly introduced functionality and RPC functions or new arguments to these functions. The RPC function would then check for the version pin and decide whether to send the object. They can also be introduced in a relation to a currently existing object, in which case the make compatible method for the object which exists in both versions would make the translation in which it will remove that object.","commit_id":"a0bc4c340cbb826bf023b2ac64cf71dbb0499146"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"71d538f4dfd98d3627096cd34a7a8efb457b7356","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":109,"context_line":"                changes, target_version, version_manifest)"},{"line_number":110,"context_line":"        if (self.VERSION !\u003d target_version"},{"line_number":111,"context_line":"            or \u0027VERSION\u0027 not in self.__dict__):  # version attribute modified"},{"line_number":112,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":113,"context_line":"        return changes"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_0f91e5f0","line":111,"range":{"start_line":111,"start_character":15,"end_line":111,"end_character":45},"updated":"2016-12-30 21:02:04.000000000","message":"shouldn\u0027t this check go before checking self.VERSION !\u003d target_version?","commit_id":"e263a1458505960ed90cc7326680466d7a4d9018"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"057d1129124110d280dee6cef147fbea3c934f99","unresolved":false,"context_lines":[{"line_number":108,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":109,"context_line":"                changes, target_version, version_manifest)"},{"line_number":110,"context_line":"        if (self.VERSION !\u003d target_version"},{"line_number":111,"context_line":"            or \u0027VERSION\u0027 not in self.__dict__):  # version attribute modified"},{"line_number":112,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":113,"context_line":"        return changes"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_ffeb8b76","line":111,"range":{"start_line":111,"start_character":15,"end_line":111,"end_character":45},"in_reply_to":"ba5201f7_0f91e5f0","updated":"2017-01-09 14:40:43.000000000","message":"I\u0027ll make the change. It\u0027ll make the obj_make_compatible methods able to overwrite also the version field.","commit_id":"e263a1458505960ed90cc7326680466d7a4d9018"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"71d538f4dfd98d3627096cd34a7a8efb457b7356","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"class IronicObjectSerializer(object_base.VersionedObjectSerializer):"},{"line_number":176,"context_line":"    # Base class to use for object hydration"},{"line_number":177,"context_line":"    OBJ_BASE_CLASS \u003d IronicObject"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def serialize_entity(self, context, entity):"},{"line_number":180,"context_line":"        if isinstance(entity, (tuple, list, set, dict)):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_2fbce96b","line":177,"updated":"2016-12-30 21:02:04.000000000","message":"The overridden __init__ method was just introduced by the previous patch set (https://review.openstack.org/#/c/407491), which hasn\u0027t landed yet. Why not just implement _get_target_version and _get_version_manifest there, instead of adding the method and then removing it immediately after that patch lands?","commit_id":"e263a1458505960ed90cc7326680466d7a4d9018"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"4db017335024b98ee2b0a7cc506722626e4a3c97","unresolved":false,"context_lines":[{"line_number":174,"context_line":""},{"line_number":175,"context_line":"class IronicObjectSerializer(object_base.VersionedObjectSerializer):"},{"line_number":176,"context_line":"    # Base class to use for object hydration"},{"line_number":177,"context_line":"    OBJ_BASE_CLASS \u003d IronicObject"},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"    def serialize_entity(self, context, entity):"},{"line_number":180,"context_line":"        if isinstance(entity, (tuple, list, set, dict)):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_61d9b983","line":177,"in_reply_to":"ba5201f7_2fbce96b","updated":"2017-01-09 14:51:43.000000000","message":"Done","commit_id":"e263a1458505960ed90cc7326680466d7a4d9018"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a1584db0b6aa8fca9b363381f8650c508e3b8af1","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        (set in VERSION attribute of the class definition). The object is"},{"line_number":67,"context_line":"        saved in that format to make sure that data stored in any newly"},{"line_number":68,"context_line":"        introduced fields is not lost. When an object is unpinned"},{"line_number":69,"context_line":"        (object instance version is set and it equals to the defined version),"},{"line_number":70,"context_line":"        obj_make_compatible methods are not called, even if the"},{"line_number":71,"context_line":"        pin_release_version configuration variable is set."},{"line_number":72,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"ba5201f7_ac007867","line":69,"updated":"2017-01-11 20:19:20.000000000","message":"by \u0027defined\u0027, you mean the class VERSION?","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"85741e6fd7a3375c50e0e81100bb09e01b9b276e","unresolved":false,"context_lines":[{"line_number":66,"context_line":"        (set in VERSION attribute of the class definition). The object is"},{"line_number":67,"context_line":"        saved in that format to make sure that data stored in any newly"},{"line_number":68,"context_line":"        introduced fields is not lost. When an object is unpinned"},{"line_number":69,"context_line":"        (object instance version is set and it equals to the defined version),"},{"line_number":70,"context_line":"        obj_make_compatible methods are not called, even if the"},{"line_number":71,"context_line":"        pin_release_version configuration variable is set."},{"line_number":72,"context_line":"        \"\"\""}],"source_content_type":"text/x-python","patch_set":15,"id":"9a57fde8_e7cbfd1c","line":69,"in_reply_to":"ba5201f7_ac007867","updated":"2017-01-12 13:55:12.000000000","message":"yes, I\u0027ll fix the description","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a1584db0b6aa8fca9b363381f8650c508e3b8af1","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        (object instance version is set and it equals to the defined version),"},{"line_number":70,"context_line":"        obj_make_compatible methods are not called, even if the"},{"line_number":71,"context_line":"        pin_release_version configuration variable is set."},{"line_number":72,"context_line":"        \"\"\""},{"line_number":73,"context_line":"        return (\u0027VERSION\u0027 in self.__dict__"},{"line_number":74,"context_line":"                and self.VERSION \u003d\u003d self.__class__.VERSION)"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"ba5201f7_2c49e8b7","line":72,"updated":"2017-01-11 20:19:20.000000000","message":"I am not a fond of \u0027negative\u0027 Booleans. Eg, this returns TRUE if unpinned, and FALSE if pinned. It might make things easier to understand if it was the other way around; if it returned TRUE for pinned, and FALSE for unpinned.","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"ad8648c7add7e08313af6142fc22a5d6c981e0c2","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        (object instance version is set and it equals to the defined version),"},{"line_number":70,"context_line":"        obj_make_compatible methods are not called, even if the"},{"line_number":71,"context_line":"        pin_release_version configuration variable is set."},{"line_number":72,"context_line":"        \"\"\""},{"line_number":73,"context_line":"        return (\u0027VERSION\u0027 in self.__dict__"},{"line_number":74,"context_line":"                and self.VERSION \u003d\u003d self.__class__.VERSION)"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"7a3c09a3_b4512811","line":72,"in_reply_to":"9a57fde8_879311fa","updated":"2017-01-12 21:32:34.000000000","message":"If it is only used in one place, then it might be better to just move the code + comments there. That would make it more localized/easier to read. Or I suppose we can wait to see what the code looks like towards \u0027the end\u0027 of this exercise, in case it is needed elsewhere.","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"85741e6fd7a3375c50e0e81100bb09e01b9b276e","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        (object instance version is set and it equals to the defined version),"},{"line_number":70,"context_line":"        obj_make_compatible methods are not called, even if the"},{"line_number":71,"context_line":"        pin_release_version configuration variable is set."},{"line_number":72,"context_line":"        \"\"\""},{"line_number":73,"context_line":"        return (\u0027VERSION\u0027 in self.__dict__"},{"line_number":74,"context_line":"                and self.VERSION \u003d\u003d self.__class__.VERSION)"},{"line_number":75,"context_line":""}],"source_content_type":"text/x-python","patch_set":15,"id":"9a57fde8_879311fa","line":72,"in_reply_to":"ba5201f7_2c49e8b7","updated":"2017-01-12 13:55:12.000000000","message":"I think of the \"unpinned\" as a positive, i.e. the code behaves different when it is true.\n\nIt is used only on line 167. I added a method to serve as additional documentation and to be more consistent with the wording in the spec.\n\nMaybe we should remove this method and move the doc to a comment inside the _get_target_version method?","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a1584db0b6aa8fca9b363381f8650c508e3b8af1","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        \"\"\"Returns a dict of changed fields and their new values.\"\"\""},{"line_number":102,"context_line":"        changes \u003d super(IronicObject, self).obj_get_changes()"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":105,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        if (self.VERSION !\u003d target_version"}],"source_content_type":"text/x-python","patch_set":15,"id":"ba5201f7_f56ec02d","line":104,"updated":"2017-01-11 20:19:20.000000000","message":"I think we should add a comment, either in L101, or above this line, describing what else this method is doing.","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"85741e6fd7a3375c50e0e81100bb09e01b9b276e","unresolved":false,"context_lines":[{"line_number":101,"context_line":"        \"\"\"Returns a dict of changed fields and their new values.\"\"\""},{"line_number":102,"context_line":"        changes \u003d super(IronicObject, self).obj_get_changes()"},{"line_number":103,"context_line":""},{"line_number":104,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":105,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        if (self.VERSION !\u003d target_version"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a57fde8_c281874f","line":104,"in_reply_to":"ba5201f7_f56ec02d","updated":"2017-01-12 13:55:12.000000000","message":"Done","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a1584db0b6aa8fca9b363381f8650c508e3b8af1","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        if (self.VERSION !\u003d target_version"},{"line_number":108,"context_line":"            or \u0027VERSION\u0027 not in self.__dict__):  # version attribute modified"},{"line_number":109,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":110,"context_line":"        if target_version !\u003d self.__class__.VERSION:"},{"line_number":111,"context_line":"            self.obj_make_compatible_from_manifest("}],"source_content_type":"text/x-python","patch_set":15,"id":"ba5201f7_b5da7849","line":108,"updated":"2017-01-11 20:19:20.000000000","message":"shouldn\u0027t we do this check first, before accessing self.VERSION above?","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"85741e6fd7a3375c50e0e81100bb09e01b9b276e","unresolved":false,"context_lines":[{"line_number":105,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"        if (self.VERSION !\u003d target_version"},{"line_number":108,"context_line":"            or \u0027VERSION\u0027 not in self.__dict__):  # version attribute modified"},{"line_number":109,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":110,"context_line":"        if target_version !\u003d self.__class__.VERSION:"},{"line_number":111,"context_line":"            self.obj_make_compatible_from_manifest("}],"source_content_type":"text/x-python","patch_set":15,"id":"9a57fde8_07b6418a","line":108,"in_reply_to":"ba5201f7_b5da7849","updated":"2017-01-12 13:55:12.000000000","message":"I placed it in this order because I think the first operation is quicker.","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a1584db0b6aa8fca9b363381f8650c508e3b8af1","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        objver \u003d db_object[\u0027version\u0027]"},{"line_number":130,"context_line":"        if objver is None:  # backfill versions for Newton release"},{"line_number":131,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027newton\u0027][\u0027objects\u0027].get("},{"line_number":132,"context_line":"                objname, \u00271.0\u0027)"},{"line_number":133,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"},{"line_number":134,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":135,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"}],"source_content_type":"text/x-python","patch_set":15,"id":"ba5201f7_f8cb1751","line":132,"updated":"2017-01-11 20:19:20.000000000","message":"instead of 1.0, maybe use obj.__class__.VERSION?","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"85741e6fd7a3375c50e0e81100bb09e01b9b276e","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        objver \u003d db_object[\u0027version\u0027]"},{"line_number":130,"context_line":"        if objver is None:  # backfill versions for Newton release"},{"line_number":131,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027newton\u0027][\u0027objects\u0027].get("},{"line_number":132,"context_line":"                objname, \u00271.0\u0027)"},{"line_number":133,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"},{"line_number":134,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":135,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a57fde8_e2d76b21","line":132,"in_reply_to":"ba5201f7_f8cb1751","updated":"2017-01-12 13:55:12.000000000","message":"We can\u0027t, because thats the current version. We want to fill in versions from Newton + changes from Ocata that were merged before this patch.","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a1584db0b6aa8fca9b363381f8650c508e3b8af1","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        for field in obj.fields:"},{"line_number":140,"context_line":"            obj[field] \u003d db_object[field]"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        obj.obj_reset_changes()"},{"line_number":143,"context_line":"        return obj"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":15,"id":"ba5201f7_66906844","line":142,"updated":"2017-01-11 20:19:20.000000000","message":"I think I am missing something.\n\nI took an example, Portgroup. In Newton, it is version 1.2, but current (master) code (ie __class__.VERSION\u003d 1.3.\n\nA.\nIf I set CONF.pin_release_version to \u0027newton\u0027 and _from_db_object is called and db_object[\u0027version\u0027] is None, I get (I think):\n- L131. objver \u003d 1.2\n- L137. obj.VERSION \u003d 1.2\n- now obj.obj_reset_changes() is called\n- L116 obj.VERSION \u003d 1.2\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\nB.\nIf I do not set CONF.pin_release_version and _from_db_object is called and db_object[\u0027version\u0027] is None, I get (I think):\n- L131. objver \u003d 1.2\n- L137. obj.VERSION \u003d 1.2\n- now obj.obj_reset_changes() is called\n- L116 obj.VERSION \u003d 1.3\n\n\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\u003d\n\nIs B what we want? I would have thought it\u0027d be 1.2 still. If I am doing a rolling upgrade from Newton to Ocata, I\u0027ll set CONF.pin_release_version to \u0027newton\u0027. But after the upgrade is done, I\u0027ll unpin but may not have run the ironic-dbsync migration command. So if I were to access an existing Portgroup from the DB, the version should still be 1.2, not 1.3.","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"5fe592542695601810acc93d48b01ba059f3796c","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        for field in obj.fields:"},{"line_number":140,"context_line":"            obj[field] \u003d db_object[field]"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        obj.obj_reset_changes()"},{"line_number":143,"context_line":"        return obj"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":15,"id":"7a3c09a3_c36c239f","line":142,"in_reply_to":"9a57fde8_62ce1b5c","updated":"2017-01-13 11:52:41.000000000","message":"I\u0027ll change the logic somewhat, because oslo.versionedobjects doesn\u0027t permit \"forward-porting\" of objects (we would have to hack around it if we wanted to do this).\n\nSince there is an assumption that the new code has to handle older objects, it doesn\u0027t have to forward-convert them. This will be the job for online data migrations.","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"85741e6fd7a3375c50e0e81100bb09e01b9b276e","unresolved":false,"context_lines":[{"line_number":139,"context_line":"        for field in obj.fields:"},{"line_number":140,"context_line":"            obj[field] \u003d db_object[field]"},{"line_number":141,"context_line":""},{"line_number":142,"context_line":"        obj.obj_reset_changes()"},{"line_number":143,"context_line":"        return obj"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"    @classmethod"}],"source_content_type":"text/x-python","patch_set":15,"id":"9a57fde8_62ce1b5c","line":142,"in_reply_to":"ba5201f7_66906844","updated":"2017-01-12 13:55:12.000000000","message":"I did that because when the service is unpinned it has full functionality, so it should always save objects in new versions. But I think you\u0027re right, the version change shouldn\u0027t happen here, but only in the obj_get_changes, right before saving.\n\nGood find. I added a test for that.","commit_id":"180be86dd33a52239f9ae2a7c2cee8b7088736b5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"440fd02ee85e13cae18fc02f9b46789cfd470be4","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    OBJ_SERIAL_NAMESPACE \u003d \u0027ironic_object\u0027"},{"line_number":56,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    _version_field_set \u003d True  # keeps track of obj_reset_changes"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @property"},{"line_number":61,"context_line":"    def _unpinned(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_52ced42e","line":58,"updated":"2017-01-16 21:05:42.000000000","message":"Maybe you can elaborate on this. This is a class variable, whereas obj_reset_changes() acts on an instance, so I am missing the connection between the two.","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"b757c19f3a3ff3be6553ebb53b00f5a251b7d6ca","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    OBJ_SERIAL_NAMESPACE \u003d \u0027ironic_object\u0027"},{"line_number":56,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    _version_field_set \u003d True  # keeps track of obj_reset_changes"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @property"},{"line_number":61,"context_line":"    def _unpinned(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a3905b3_3160cc35","line":58,"in_reply_to":"7a3c09a3_1ffc173d","updated":"2017-01-19 14:28:23.000000000","message":"I checked and I think it\u0027s the correct way to set default values, it is used for instance in Python httplib2 in the Response class to set default status and reason to 100 and \u0027Ok\u0027","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f3209886c25aeeb2179c3c9c5b7f4428a4f27588","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    OBJ_SERIAL_NAMESPACE \u003d \u0027ironic_object\u0027"},{"line_number":56,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    _version_field_set \u003d True  # keeps track of obj_reset_changes"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @property"},{"line_number":61,"context_line":"    def _unpinned(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_1ffc173d","line":58,"in_reply_to":"7a3c09a3_42c97b58","updated":"2017-01-17 21:05:36.000000000","message":"So objects can have default values. They are typically set in their __init__() ?","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"cc6492157f4f4a1d9750198748fef2231e83fa1b","unresolved":false,"context_lines":[{"line_number":55,"context_line":"    OBJ_SERIAL_NAMESPACE \u003d \u0027ironic_object\u0027"},{"line_number":56,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    _version_field_set \u003d True  # keeps track of obj_reset_changes"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    @property"},{"line_number":61,"context_line":"    def _unpinned(self):"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_42c97b58","line":58,"in_reply_to":"7a3c09a3_52ced42e","updated":"2017-01-17 14:46:52.000000000","message":"This is the default value, I could perhaps set it in __init__ but I\u0027m not sure if this would be more proper...","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"440fd02ee85e13cae18fc02f9b46789cfd470be4","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":141,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        obj.VERSION \u003d objver"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        for field in obj.fields:"},{"line_number":146,"context_line":"            obj[field] \u003d db_object[field]"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_cd24e5c6","line":143,"updated":"2017-01-16 21:05:42.000000000","message":"One of the things that bothers me, and maybe I\u0027m just not a great python developer, is the use of a class variable being overridden by an instance of that class. (I\u0027m OK with subclasses overriding them.) It seems to me that this makes it hard to debug what is going on. And actually, it seems wrong. Wouldn\u0027t it be better to have an instance ._version or .version or whatever instead?","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f3209886c25aeeb2179c3c9c5b7f4428a4f27588","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":141,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        obj.VERSION \u003d objver"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        for field in obj.fields:"},{"line_number":146,"context_line":"            obj[field] \u003d db_object[field]"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_ff0aeb24","line":143,"in_reply_to":"7a3c09a3_829a836c","updated":"2017-01-17 21:05:36.000000000","message":"Maybe I am out of my league here. That versionedobjects code is using it BUT it is a class.VERSION not an instance.VERSION.\n\nWhereas here, it is an instance.VERSION, right? Or is obj a class?","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"cc6492157f4f4a1d9750198748fef2231e83fa1b","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":141,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        obj.VERSION \u003d objver"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        for field in obj.fields:"},{"line_number":146,"context_line":"            obj[field] \u003d db_object[field]"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_829a836c","line":143,"in_reply_to":"7a3c09a3_cd24e5c6","updated":"2017-01-17 14:46:52.000000000","message":"This is the same way oslo versionedobjects do this. https://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/base.py#L393\n\nI actually had that in a separate variable in a previous iteration of this patch, but I modified it so that it follows the oslo model to make this method behave the same way.","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"b757c19f3a3ff3be6553ebb53b00f5a251b7d6ca","unresolved":false,"context_lines":[{"line_number":140,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":141,"context_line":"                objname\u003dobjname, objver\u003dobjver, supported\u003dobj.VERSION)"},{"line_number":142,"context_line":""},{"line_number":143,"context_line":"        obj.VERSION \u003d objver"},{"line_number":144,"context_line":""},{"line_number":145,"context_line":"        for field in obj.fields:"},{"line_number":146,"context_line":"            obj[field] \u003d db_object[field]"}],"source_content_type":"text/x-python","patch_set":21,"id":"5a3905b3_51a638c2","line":143,"in_reply_to":"7a3c09a3_ff0aeb24","updated":"2017-01-19 14:28:23.000000000","message":"In the first line of the oslo method, the class is instantiated with\nself \u003d cls()\nhttps://github.com/openstack/oslo.versionedobjects/blob/master/oslo_versionedobjects/base.py#L391\n\nSo it also sets the object attribute.\n\nThis is specifically what I\u0027m mentioning in the description of the _unpinned property above - oslo does this upon deserialization, and here, we are also doing the same thing when reading from the DB, to be consistent with how oslo behaves.","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"864bbd8c402777be1b7af64bb54cc813367c844b","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":116,"context_line":"                changes, target_version, version_manifest)"},{"line_number":117,"context_line":"        if self._version_field_set:"},{"line_number":118,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":119,"context_line":"        return changes"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def obj_reset_changes(self):"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_86c36588","line":118,"updated":"2017-01-17 19:31:46.000000000","message":"Just checking my understanding....\n\nThis is needed so that \"version\" in the DB is only set once per version change on an object, right?\n\nMeaning if _version_field_set is False, obj_reset_changes has been called at least once, meaning that the DB \"version\" has been updated to the version this service is working with?","commit_id":"845ff5997d11116b22d4e8acb2dc68c64598ebf1"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"5af2d55dc5d2e282ca1aed225a04fd215a1c7587","unresolved":false,"context_lines":[{"line_number":115,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":116,"context_line":"                changes, target_version, version_manifest)"},{"line_number":117,"context_line":"        if self._version_field_set:"},{"line_number":118,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":119,"context_line":"        return changes"},{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def obj_reset_changes(self):"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a3905b3_e9c65a15","line":118,"in_reply_to":"7a3c09a3_86c36588","updated":"2017-01-19 14:10:39.000000000","message":"Yes. This should mostly behave like an object field (for db r/w access), but it shouldn\u0027t be sent over RPC, since the version is already set in a separate entry in the serialized format of an object.\n\nSome unit tests expect that after an obj_reset_changes(), obj_get_changes() returns an empty dict, with additional values only when they are set verbatim. I could return the version in every case, but I didn\u0027t want to change that behavior, to stay consistent with how oslo versioned objects work in other projects.","commit_id":"845ff5997d11116b22d4e8acb2dc68c64598ebf1"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    OBJ_SERIAL_NAMESPACE \u003d \u0027ironic_object\u0027"},{"line_number":54,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    _version_field_set \u003d True  # keeps track of obj_reset_changes"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    @property"},{"line_number":59,"context_line":"    def _unpinned(self):"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_ba6acc18","line":56,"updated":"2017-01-19 16:44:28.000000000","message":"I think it would be useful to have a more detailed comment about this; it isn\u0027t clear to me.\n\nDoes it mean: 1. the version field has been set; 2. the version field should be set; 3. obj_reset_changes did something to the version_field? 4. ??","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":53,"context_line":"    OBJ_SERIAL_NAMESPACE \u003d \u0027ironic_object\u0027"},{"line_number":54,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    _version_field_set \u003d True  # keeps track of obj_reset_changes"},{"line_number":57,"context_line":""},{"line_number":58,"context_line":"    @property"},{"line_number":59,"context_line":"    def _unpinned(self):"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_f39b7bc7","line":56,"in_reply_to":"5a3905b3_ba6acc18","updated":"2017-01-20 09:31:54.000000000","message":"I added a more detailed note.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"1ba0afef9d4a69f2179472426d4ba417608886cb","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        obj_make_compatible methods are not called, even if the"},{"line_number":71,"context_line":"        pin_release_version configuration variable is set."},{"line_number":72,"context_line":"        \"\"\""},{"line_number":73,"context_line":"        return (\u0027VERSION\u0027 in self.__dict__"},{"line_number":74,"context_line":"                and self.VERSION \u003d\u003d self.__class__.VERSION)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    # TODO(lintan) Refactor these fields and create PersistentObject and"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_4be91d1a","line":73,"range":{"start_line":73,"start_character":16,"end_line":73,"end_character":42},"updated":"2017-01-19 16:55:48.000000000","message":"hasattr(self, \u0027VERSION\u0027)","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        obj_make_compatible methods are not called, even if the"},{"line_number":71,"context_line":"        pin_release_version configuration variable is set."},{"line_number":72,"context_line":"        \"\"\""},{"line_number":73,"context_line":"        return (\u0027VERSION\u0027 in self.__dict__"},{"line_number":74,"context_line":"                and self.VERSION \u003d\u003d self.__class__.VERSION)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    # TODO(lintan) Refactor these fields and create PersistentObject and"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_b8ab901c","line":73,"range":{"start_line":73,"start_character":16,"end_line":73,"end_character":42},"in_reply_to":"5a3905b3_4be91d1a","updated":"2017-01-20 09:31:54.000000000","message":"This would always return True, while the above returns true only if the object attribute is set.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"1ba0afef9d4a69f2179472426d4ba417608886cb","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    def obj_get_changes(self):"},{"line_number":101,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        The changes may contain new values if the object is unpinned and"},{"line_number":104,"context_line":"        the version changes. In this case, the object\u0027s obj_make_compatible()"},{"line_number":105,"context_line":"        method is called."},{"line_number":106,"context_line":"        \"\"\""},{"line_number":107,"context_line":"        changes \u003d super(IronicObject, self).obj_get_changes()"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_fd4b6eae","line":104,"range":{"start_line":103,"start_character":57,"end_line":104,"end_character":27},"updated":"2017-01-19 16:55:48.000000000","message":"Seems this is called by remotable decorator, and target_version can be \u003c self.VERSION in this case, and we call obj_make_compatible_from_manifest? May need to dig more into ovo.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":100,"context_line":"    def obj_get_changes(self):"},{"line_number":101,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"        The changes may contain new values if the object is unpinned and"},{"line_number":104,"context_line":"        the version changes. In this case, the object\u0027s obj_make_compatible()"},{"line_number":105,"context_line":"        method is called."},{"line_number":106,"context_line":"        \"\"\""},{"line_number":107,"context_line":"        changes \u003d super(IronicObject, self).obj_get_changes()"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_f32c5b4c","line":104,"range":{"start_line":103,"start_character":57,"end_line":104,"end_character":27},"in_reply_to":"5a3905b3_fd4b6eae","updated":"2017-01-20 09:31:54.000000000","message":"Yes, I think the \"may contain new values\" is misleading, I\u0027ll remove it.\n\nOn line 112 we don\u0027t have to do a check like in oslo, converting the versions to tuples and comparing target_version \u003c self.VERSION, because _get_target_version already returns a target_version \u003c\u003d self.VERSION.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"1ba0afef9d4a69f2179472426d4ba417608886cb","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":110,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        if target_version !\u003d self.VERSION:"},{"line_number":113,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":114,"context_line":"                changes, target_version, version_manifest)"},{"line_number":115,"context_line":"        if self._version_field_set:"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_261ec8ba","line":112,"updated":"2017-01-19 16:55:48.000000000","message":"Can move L109 inside this if","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":110,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"        if target_version !\u003d self.VERSION:"},{"line_number":113,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":114,"context_line":"                changes, target_version, version_manifest)"},{"line_number":115,"context_line":"        if self._version_field_set:"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_73214b4d","line":112,"in_reply_to":"5a3905b3_261ec8ba","updated":"2017-01-20 09:31:54.000000000","message":"Done","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"1ba0afef9d4a69f2179472426d4ba417608886cb","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        return None"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def _get_target_version(self):"},{"line_number":171,"context_line":"        if self._unpinned:"},{"line_number":172,"context_line":"            return self.VERSION"},{"line_number":173,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":174,"context_line":"        if version_manifest:"},{"line_number":175,"context_line":"            return version_manifest[self.obj_name()]"},{"line_number":176,"context_line":"        return self.VERSION"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"class IronicObjectSerializer(object_base.VersionedObjectSerializer):"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_6b17b9a3","line":176,"range":{"start_line":171,"start_character":8,"end_line":176,"end_character":27},"updated":"2017-01-19 16:55:48.000000000","message":"2 lines shorter version :)\n\n version_manifest \u003d self._get_version_manifest()\n if not self._unpinned and version_manifest:\n   return version_manifest[self.obj_name()]\n return self.VERSION","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":168,"context_line":"        return None"},{"line_number":169,"context_line":""},{"line_number":170,"context_line":"    def _get_target_version(self):"},{"line_number":171,"context_line":"        if self._unpinned:"},{"line_number":172,"context_line":"            return self.VERSION"},{"line_number":173,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":174,"context_line":"        if version_manifest:"},{"line_number":175,"context_line":"            return version_manifest[self.obj_name()]"},{"line_number":176,"context_line":"        return self.VERSION"},{"line_number":177,"context_line":""},{"line_number":178,"context_line":""},{"line_number":179,"context_line":"class IronicObjectSerializer(object_base.VersionedObjectSerializer):"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_73306ae9","line":176,"range":{"start_line":171,"start_character":8,"end_line":176,"end_character":27},"in_reply_to":"5a3905b3_6b17b9a3","updated":"2017-01-20 09:31:54.000000000","message":"I moved the contents of the _unpinned property here (due to previous discussions with Ruby). I think we should stay with the more verbose version, it may be more comprehensive.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"bd34b0441f040b5e675cb54c7d8879b79c7ad682","unresolved":false,"context_lines":[{"line_number":171,"context_line":"        # set and it equals to the class version), obj_make_compatible methods"},{"line_number":172,"context_line":"        # are not called, even if the pin_release_version configuration is set."},{"line_number":173,"context_line":"        if (\u0027VERSION\u0027 in self.__dict__"},{"line_number":174,"context_line":"            and self.VERSION \u003d\u003d self.__class__.VERSION):"},{"line_number":175,"context_line":"            return self.VERSION  # object is \"unpinned\""},{"line_number":176,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":177,"context_line":"        if version_manifest:"}],"source_content_type":"text/x-python","patch_set":27,"id":"5a3905b3_589c5569","line":174,"updated":"2017-01-20 15:07:28.000000000","message":"This additional check should be removed to be more consistent with how oslo versionedobject deserializer behaves.\n\nThe behavior differs in a situation, in which the operator  makes a mistake of doing a rolling upgrade skipping a release, so it\u0027s pretty extreme, but I think I should fix it for consistency.\n\nOslo doesn\u0027t change versions, it also raises an exception if the major version differs (which is also what we do when retrieving objects from db). I will add tests for this.","commit_id":"47260a4a277d7f5f044bb90575307c08a0ab62bc"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"f2ec3bc026b0cb72ccdd376a13e76ecfc35ed9cb","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    # NOTE(xek): The _version_field_set attribute keeps track of calls to"},{"line_number":57,"context_line":"    # obj_reset_changes(). It determes if the version should be returned by"},{"line_number":58,"context_line":"    # object\u0027s obj_get_changes() method."},{"line_number":59,"context_line":"    _version_field_set \u003d True"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"5a3905b3_a678aac5","line":57,"updated":"2017-01-20 20:41:45.000000000","message":"determes/determines\n\nAlso, this comment still seems unclear - keeping track of calls reminds me rather of mock call counters. Maybe (not sure if describes it well):\nDetermines whether the version should be included in the return value of object\u0027s obj_get_changes() method","commit_id":"e9bdc870039a689ae0748ced10fb994057195ba3"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"3471c6edaca98409297c215cf0cc6c61286f9043","unresolved":false,"context_lines":[{"line_number":54,"context_line":"    OBJ_PROJECT_NAMESPACE \u003d \u0027ironic\u0027"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    # NOTE(xek): The _version_field_set attribute keeps track of calls to"},{"line_number":57,"context_line":"    # obj_reset_changes(). It determes if the version should be returned by"},{"line_number":58,"context_line":"    # object\u0027s obj_get_changes() method."},{"line_number":59,"context_line":"    _version_field_set \u003d True"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":29,"id":"5a3905b3_879d3e68","line":57,"in_reply_to":"5a3905b3_a678aac5","updated":"2017-01-23 07:28:28.000000000","message":"Done","commit_id":"e9bdc870039a689ae0748ced10fb994057195ba3"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"42a9e2a7eec00b037291e6c8729841db7b9a5bb4","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                    self[field] !\u003d loaded_object[field]):"},{"line_number":83,"context_line":"                self[field] \u003d loaded_object[field]"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    def obj_get_changes(self):"},{"line_number":86,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        If the object is pinned and the version changes, the object\u0027s"},{"line_number":89,"context_line":"        obj_make_compatible() method is called."},{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        changes \u003d super(IronicObject, self).obj_get_changes()"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        if target_version !\u003d self.VERSION:"},{"line_number":96,"context_line":"            version_manifest \u003d self._get_version_manifest()"},{"line_number":97,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":98,"context_line":"                changes, target_version, version_manifest)"},{"line_number":99,"context_line":"        if self._version_field_set:"},{"line_number":100,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":101,"context_line":"        return changes"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def obj_reset_changes(self):"},{"line_number":104,"context_line":"        self._version_field_set \u003d False"},{"line_number":105,"context_line":"        return super(IronicObject, self).obj_reset_changes()"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @staticmethod"},{"line_number":108,"context_line":"    def _from_db_object(obj, db_object):"}],"source_content_type":"text/x-python","patch_set":32,"id":"5a3905b3_78153e3e","line":105,"range":{"start_line":85,"start_character":0,"end_line":105,"end_character":60},"updated":"2017-01-23 14:24:45.000000000","message":"I don\u0027t really get why we need to add this as of now. None of our objects\u0027 methods are remotable, and as we\u0027re doing explicit rpc calls, the serializer should handle everything we need in terms of backporting the object fields/changes.\n\nMaybe do this when we switch our objects to use @remotable decorator?","commit_id":"34e9e73a9747468beec3026c8fdbfa4a74816057"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"9ff4cb69d4e9725f3273feda0b9f5f12b8d24818","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                    self[field] !\u003d loaded_object[field]):"},{"line_number":83,"context_line":"                self[field] \u003d loaded_object[field]"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    def obj_get_changes(self):"},{"line_number":86,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        If the object is pinned and the version changes, the object\u0027s"},{"line_number":89,"context_line":"        obj_make_compatible() method is called."},{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        changes \u003d super(IronicObject, self).obj_get_changes()"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        if target_version !\u003d self.VERSION:"},{"line_number":96,"context_line":"            version_manifest \u003d self._get_version_manifest()"},{"line_number":97,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":98,"context_line":"                changes, target_version, version_manifest)"},{"line_number":99,"context_line":"        if self._version_field_set:"},{"line_number":100,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":101,"context_line":"        return changes"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def obj_reset_changes(self):"},{"line_number":104,"context_line":"        self._version_field_set \u003d False"},{"line_number":105,"context_line":"        return super(IronicObject, self).obj_reset_changes()"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @staticmethod"},{"line_number":108,"context_line":"    def _from_db_object(obj, db_object):"}],"source_content_type":"text/x-python","patch_set":32,"id":"5a3905b3_7ddc4873","line":105,"range":{"start_line":85,"start_character":0,"end_line":105,"end_character":60},"in_reply_to":"5a3905b3_78153e3e","updated":"2017-01-23 16:34:58.000000000","message":"Ah, sorry, I must be blind :), these are used in objects\u0027 create/destroy methods and _from_db_object now, so is needed. So the only thing it does, is giving a hint that an object was read from database (which means, it is save or destroy, not create call), and version was included in it. For RPC (in case of all save/destroy calls), we do not need to set it, serializer will take care of it. In case of node create call, version cap is used. Is that correct?","commit_id":"34e9e73a9747468beec3026c8fdbfa4a74816057"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"57e7d564c64bafc404280d9fc0ba8dc1e2aeb968","unresolved":false,"context_lines":[{"line_number":82,"context_line":"                    self[field] !\u003d loaded_object[field]):"},{"line_number":83,"context_line":"                self[field] \u003d loaded_object[field]"},{"line_number":84,"context_line":""},{"line_number":85,"context_line":"    def obj_get_changes(self):"},{"line_number":86,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":87,"context_line":""},{"line_number":88,"context_line":"        If the object is pinned and the version changes, the object\u0027s"},{"line_number":89,"context_line":"        obj_make_compatible() method is called."},{"line_number":90,"context_line":"        \"\"\""},{"line_number":91,"context_line":"        changes \u003d super(IronicObject, self).obj_get_changes()"},{"line_number":92,"context_line":""},{"line_number":93,"context_line":"        target_version \u003d self._get_target_version()"},{"line_number":94,"context_line":""},{"line_number":95,"context_line":"        if target_version !\u003d self.VERSION:"},{"line_number":96,"context_line":"            version_manifest \u003d self._get_version_manifest()"},{"line_number":97,"context_line":"            self.obj_make_compatible_from_manifest("},{"line_number":98,"context_line":"                changes, target_version, version_manifest)"},{"line_number":99,"context_line":"        if self._version_field_set:"},{"line_number":100,"context_line":"            changes[\u0027version\u0027] \u003d target_version"},{"line_number":101,"context_line":"        return changes"},{"line_number":102,"context_line":""},{"line_number":103,"context_line":"    def obj_reset_changes(self):"},{"line_number":104,"context_line":"        self._version_field_set \u003d False"},{"line_number":105,"context_line":"        return super(IronicObject, self).obj_reset_changes()"},{"line_number":106,"context_line":""},{"line_number":107,"context_line":"    @staticmethod"},{"line_number":108,"context_line":"    def _from_db_object(obj, db_object):"}],"source_content_type":"text/x-python","patch_set":32,"id":"1a430d35_423c28c8","line":105,"range":{"start_line":85,"start_character":0,"end_line":105,"end_character":60},"in_reply_to":"5a3905b3_7ddc4873","updated":"2017-02-07 17:47:55.000000000","message":"Quoting from an old review I did on here:\n\n\"Just checking my understanding....\nThis is needed so that \"version\" in the DB is only set once per version change on an object, right?\nMeaning if _version_field_set is False, obj_reset_changes has been called at least once, meaning that the DB \"version\" has been updated to the version this service is working with?\"\n\nI think there should be a comment here, since it seems to be a confusing point for \u003e1 person.","commit_id":"34e9e73a9747468beec3026c8fdbfa4a74816057"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"42a9e2a7eec00b037291e6c8729841db7b9a5bb4","unresolved":false,"context_lines":[{"line_number":158,"context_line":"        database or sent over the wire via RPC."},{"line_number":159,"context_line":""},{"line_number":160,"context_line":"        \"\"\""},{"line_number":161,"context_line":"        # NOTE(xek): Individual object versions can be unpinned."},{"line_number":162,"context_line":"        # This happens when we retrieve an object from the DB or receive it"},{"line_number":163,"context_line":"        # via RPC. We can detect this situation by checking if object\u0027s"},{"line_number":164,"context_line":"        # VERSION attribute is set. The VERSION attribute is set both at"}],"source_content_type":"text/x-python","patch_set":32,"id":"5a3905b3_2938cbee","line":161,"updated":"2017-01-23 14:24:45.000000000","message":"So, unpinned \u003d\u003d version is set on class, not instance. This means that in case of an unpinned object, self.__class__.VERSION \u003d\u003d self.VERSION. Do we actually care if VERSION was set on an object, or on a class? This fact should not affect the object representation in this case.","commit_id":"34e9e73a9747468beec3026c8fdbfa4a74816057"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"9a8ace33aa7c17a4f442d85704e54b6680c94aef","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        return changes"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def obj_reset_changes(self):"},{"line_number":107,"context_line":"        self._version_field_set \u003d False"},{"line_number":108,"context_line":"        return super(IronicObject, self).obj_reset_changes()"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":36,"id":"9a30ddce_da9d8d5a","line":107,"updated":"2017-03-08 19:06:08.000000000","message":"++ on adding comment here","commit_id":"c49f2744d437de4c8b1784430996344bb47ae8a3"},{"author":{"_account_id":10343,"name":"Jim Rollenhagen","email":"jim@jimrollenhagen.com","username":"jimrollenhagen"},"change_message_id":"1ae363ef331ce7edb47623dad0481f17ad583e66","unresolved":false,"context_lines":[{"line_number":118,"context_line":""},{"line_number":119,"context_line":"        objname \u003d obj.obj_name()"},{"line_number":120,"context_line":"        objver \u003d db_object[\u0027version\u0027]"},{"line_number":121,"context_line":"        if objver is None:  # backfill versions for Newton release"},{"line_number":122,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027newton\u0027][\u0027objects\u0027].get("},{"line_number":123,"context_line":"                objname, \u00271.0\u0027)"},{"line_number":124,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"}],"source_content_type":"text/x-python","patch_set":36,"id":"ba2be162_c82e04fa","line":121,"updated":"2017-03-06 20:02:24.000000000","message":"probably s/newton/ocata/ at this point, yes?","commit_id":"c49f2744d437de4c8b1784430996344bb47ae8a3"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"9a8ace33aa7c17a4f442d85704e54b6680c94aef","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        objname \u003d obj.obj_name()"},{"line_number":120,"context_line":"        objver \u003d db_object[\u0027version\u0027]"},{"line_number":121,"context_line":"        if objver is None:  # backfill versions for Newton release"},{"line_number":122,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027newton\u0027][\u0027objects\u0027].get("},{"line_number":123,"context_line":"                objname, \u00271.0\u0027)"},{"line_number":124,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"},{"line_number":125,"context_line":"            raise exception.IncompatibleObjectVersion("}],"source_content_type":"text/x-python","patch_set":36,"id":"9a30ddce_3a78692a","line":122,"updated":"2017-03-08 19:06:08.000000000","message":"++\nWhat is more - wouldn\u0027t it be better if if wasn\u0027t a magic string? Then we wouldn\u0027t have to remember about updating this with each release?","commit_id":"c49f2744d437de4c8b1784430996344bb47ae8a3"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d771a27ceb9c1aeb05989be97132202358eb81ab","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"        objname \u003d obj.obj_name()"},{"line_number":127,"context_line":"        objver \u003d db_object[\u0027version\u0027]"},{"line_number":128,"context_line":"        if objver is None:  # backfill versions for Newton release"},{"line_number":129,"context_line":"            objver \u003d versions.RELEASE_MAPPING[\u0027ocata\u0027][\u0027objects\u0027].get("},{"line_number":130,"context_line":"                objname, \u00271.0\u0027)"},{"line_number":131,"context_line":"        if not versionutils.is_compatible(objver, obj.VERSION):"}],"source_content_type":"text/x-python","patch_set":39,"id":"1a1ced50_6e1c4c6a","line":128,"updated":"2017-03-23 13:57:11.000000000","message":"I thought I had changed it. s/Newton/Ocata/ \n\nAlthough I should mention that it is really the release prior to the one with this code.","commit_id":"b79a8ad01c9b3b66b285535e2d148c296fac0e86"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"3b651ea6fb475a7a785f223ae9be6134a3488949","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":196,"context_line":"        if version_manifest:"},{"line_number":197,"context_line":"            pinned_version \u003d version_manifest[self.obj_name()]"},{"line_number":198,"context_line":"        return pinned_version or self.VERSION"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"class IronicObjectSerializer(object_base.VersionedObjectSerializer):"}],"source_content_type":"text/x-python","patch_set":42,"id":"bff0334d_27481585","line":198,"updated":"2017-04-10 23:30:26.000000000","message":"the logic here was changed; previously it worked the other way:\nself.VERSION or pinned_version.\n\nIs this change proper? (I see the return value description in docstring saying so, but I just want to make sure)","commit_id":"9c11d102e6eaef521f3276c7c4d8602969596fbd"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8b29d36e2f30048cd46be7661f6f9e77aa15839f","unresolved":false,"context_lines":[{"line_number":195,"context_line":"        version_manifest \u003d self._get_version_manifest()"},{"line_number":196,"context_line":"        if version_manifest:"},{"line_number":197,"context_line":"            pinned_version \u003d version_manifest[self.obj_name()]"},{"line_number":198,"context_line":"        return pinned_version or self.VERSION"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":""},{"line_number":201,"context_line":"class IronicObjectSerializer(object_base.VersionedObjectSerializer):"}],"source_content_type":"text/x-python","patch_set":42,"id":"bff0334d_7ed343d6","line":198,"in_reply_to":"bff0334d_27481585","updated":"2017-04-11 03:04:51.000000000","message":"This is correct. You want the pinned version. Only if there isn\u0027t any pinned version, do you return the actual version. Although where did you see that it was the other way around?","commit_id":"9c11d102e6eaef521f3276c7c4d8602969596fbd"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"8b29d36e2f30048cd46be7661f6f9e77aa15839f","unresolved":false,"context_lines":[{"line_number":214,"context_line":"            # (via entity.obj_to_primitive()) to backport the object before"},{"line_number":215,"context_line":"            # serialization."},{"line_number":216,"context_line":"            entity \u003d entity.obj_to_primitive(target_version\u003dtarget_version)"},{"line_number":217,"context_line":"        elif not isinstance(entity, (int, str, bool, float, type)) and entity:"},{"line_number":218,"context_line":"            LOG.warning(\"Entity %s was not serialized.\", str(entity))"},{"line_number":219,"context_line":"        return entity"}],"source_content_type":"text/x-python","patch_set":42,"id":"bff0334d_5edcffe2","line":217,"updated":"2017-04-11 03:04:51.000000000","message":"eww, my rebase didn\u0027t work properly; lost some changes in master.","commit_id":"9c11d102e6eaef521f3276c7c4d8602969596fbd"},{"author":{"_account_id":7368,"name":"Thomas Maddox","email":"me@thomasem.net","username":"thomasm"},"change_message_id":"3057285335489523b7948f57adde484b0363ebdd","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        db_version \u003d None"},{"line_number":208,"context_line":"        if hasattr(self, \u0027version\u0027):"},{"line_number":209,"context_line":"            # Don\u0027t just do getattr(self, \u0027version\u0027, \u00271.0\u0027) because of"},{"line_number":210,"context_line":"            # the way the versioned objects do loading"},{"line_number":211,"context_line":"            db_version \u003d getattr(self, \u0027version\u0027)"},{"line_number":212,"context_line":"        if db_version and ("},{"line_number":213,"context_line":"                versionutils.convert_version_to_tuple(target_version) \u003c"}],"source_content_type":"text/x-python","patch_set":43,"id":"bff0334d_724ebd38","line":210,"updated":"2017-04-11 16:40:58.000000000","message":"Mind elaborating a bit here? Not sure I understand what about the way versioned objects do loading necessitates this?","commit_id":"5e878bb1201aa29309bfab44f3f0b1317c82b504"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"21c1be65cc489f8a5e688ec957e54294d0282151","unresolved":false,"context_lines":[{"line_number":207,"context_line":"        db_version \u003d None"},{"line_number":208,"context_line":"        if hasattr(self, \u0027version\u0027):"},{"line_number":209,"context_line":"            # Don\u0027t just do getattr(self, \u0027version\u0027, \u00271.0\u0027) because of"},{"line_number":210,"context_line":"            # the way the versioned objects do loading"},{"line_number":211,"context_line":"            db_version \u003d getattr(self, \u0027version\u0027)"},{"line_number":212,"context_line":"        if db_version and ("},{"line_number":213,"context_line":"                versionutils.convert_version_to_tuple(target_version) \u003c"}],"source_content_type":"text/x-python","patch_set":43,"id":"bff0334d_9e6caa9c","line":210,"in_reply_to":"bff0334d_724ebd38","updated":"2017-04-11 18:53:20.000000000","message":"That comment was a bit of hand-waving. I was working on something else and didn\u0027t want to look into it. Thx for reminding me. I updated this code. I don\u0027t know of official documentation that describes this about versioned objects, but this email exchange helps: [1].\n\n[1] http://lists.openstack.org/pipermail/openstack-dev/2016-December/109086.html","commit_id":"5e878bb1201aa29309bfab44f3f0b1317c82b504"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e811899e4c9bacad1c778bf024d0c72e2e869eb6","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":45,"id":"3f044301_00dd7f13","updated":"2017-05-05 17:33:26.000000000","message":"Are there unit tests for any of these changes?","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"137b376275bc15dfbea46a62fa045332d9424ac3","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":45,"id":"1f013ff3_ae3b63cf","in_reply_to":"1f013ff3_9370d074","updated":"2017-05-15 16:15:50.000000000","message":"Yeah, seems like there should be unit tests. Though I\u0027m not sure if anyone ever created unit tests for this file before :(","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"af60137cd25a4530d3889eb6ba40f5c45bb19361","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":45,"id":"1f013ff3_795403cb","in_reply_to":"1f013ff3_ae3b63cf","updated":"2017-05-15 16:52:10.000000000","message":"Ugh, my bad. I thought I had added tests, looks like they are in https://review.openstack.org/#/c/412397/45/ironic/tests/unit/objects/test_objects.py.\n\nWould it make it easier if we renamed that file to test_base.py?","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d0391d52e1ced8e435a6ff7b3fe31a8e7214eb8b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":45,"id":"1f013ff3_9370d074","in_reply_to":"3f044301_00dd7f13","updated":"2017-05-15 16:05:25.000000000","message":"I don\u0027t know, I didn\u0027t write most of this code. I mean, there are no explicit unit tests that I can see; I am assuming they are being tested via the tests/unit/objects/*.py unit tests.\n\nwrt the changes I added, they are being tested via the tests/unit/objects/*.py that are affected.","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"7cb6d2f202c6ad5b84867553c708ec47a82451d4","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                  to the pin. Otherwise, returns the version of the object."},{"line_number":102,"context_line":"        \"\"\""},{"line_number":103,"context_line":"        pinned_version \u003d None"},{"line_number":104,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":105,"context_line":"        if pin:"},{"line_number":106,"context_line":"            version_manifest \u003d versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":107,"context_line":"            pinned_version \u003d version_manifest.get(self.obj_name())"}],"source_content_type":"text/x-python","patch_set":45,"id":"3f044301_a3311ca5","line":104,"updated":"2017-05-05 17:56:03.000000000","message":"Does this mean that the description in ironic/conf/defaults.py should be updated for this option? Is it now working?","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"137b376275bc15dfbea46a62fa045332d9424ac3","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                  to the pin. Otherwise, returns the version of the object."},{"line_number":102,"context_line":"        \"\"\""},{"line_number":103,"context_line":"        pinned_version \u003d None"},{"line_number":104,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":105,"context_line":"        if pin:"},{"line_number":106,"context_line":"            version_manifest \u003d versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":107,"context_line":"            pinned_version \u003d version_manifest.get(self.obj_name())"}],"source_content_type":"text/x-python","patch_set":45,"id":"1f013ff3_13516037","line":104,"in_reply_to":"1f013ff3_f3b154e3","updated":"2017-05-15 16:15:50.000000000","message":"That would work for me. Sounds reasonable, especially if you like it :)","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d0391d52e1ced8e435a6ff7b3fe31a8e7214eb8b","unresolved":false,"context_lines":[{"line_number":101,"context_line":"                  to the pin. Otherwise, returns the version of the object."},{"line_number":102,"context_line":"        \"\"\""},{"line_number":103,"context_line":"        pinned_version \u003d None"},{"line_number":104,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":105,"context_line":"        if pin:"},{"line_number":106,"context_line":"            version_manifest \u003d versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":107,"context_line":"            pinned_version \u003d version_manifest.get(self.obj_name())"}],"source_content_type":"text/x-python","patch_set":45,"id":"1f013ff3_f3b154e3","line":104,"in_reply_to":"3f044301_a3311ca5","updated":"2017-05-15 16:05:25.000000000","message":"Sigh. I wish there was some way to block a feature until it was *all* done, vs this incremental stuff.\n\nIt will work, but the entire feature won\u0027t work yet, eg, the dbsync command patch is still needed.\n\nDo you think I should change the part about \u0027changing this value has no effect\u0027 (that\u0027s the only untrue thing).  I could just delete that part. I still would like to leave the WARNING part.","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"e811899e4c9bacad1c778bf024d0c72e2e869eb6","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        return [cls._from_db_object(cls(context), db_obj)"},{"line_number":184,"context_line":"                for db_obj in db_objects]"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def do_changes_for_db(self):"},{"line_number":187,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        These are changes for the object that are to be saved in the"}],"source_content_type":"text/x-python","patch_set":45,"id":"3f044301_28c72b48","line":186,"updated":"2017-05-05 17:33:26.000000000","message":"Not sure I like this function name. What kind of changes? Does it actually change anything?\n\nget_changed_fields?","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":14760,"name":"John L. Villalovos","email":"openstack.org@sodarock.com","username":"jlvillal"},"change_message_id":"137b376275bc15dfbea46a62fa045332d9424ac3","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        return [cls._from_db_object(cls(context), db_obj)"},{"line_number":184,"context_line":"                for db_obj in db_objects]"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def do_changes_for_db(self):"},{"line_number":187,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        These are changes for the object that are to be saved in the"}],"source_content_type":"text/x-python","patch_set":45,"id":"1f013ff3_d3189882","line":186,"in_reply_to":"1f013ff3_93521067","updated":"2017-05-15 16:15:50.000000000","message":"to me \u0027do_changes_for_db\u0027 doesn\u0027t mean much when I read it. Well it almost sounds like it is changing the db. But then again, maybe it is changing something for the db. But changing what?\n\nI\u0027m not sure on a good name though :(","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"af60137cd25a4530d3889eb6ba40f5c45bb19361","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        return [cls._from_db_object(cls(context), db_obj)"},{"line_number":184,"context_line":"                for db_obj in db_objects]"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def do_changes_for_db(self):"},{"line_number":187,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        These are changes for the object that are to be saved in the"}],"source_content_type":"text/x-python","patch_set":45,"id":"1f013ff3_1965ffb5","line":186,"in_reply_to":"1f013ff3_d3189882","updated":"2017-05-15 16:52:10.000000000","message":"John and I discussed, and \u0027do_version_changes_for_db\u0027 is better.","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"d0391d52e1ced8e435a6ff7b3fe31a8e7214eb8b","unresolved":false,"context_lines":[{"line_number":183,"context_line":"        return [cls._from_db_object(cls(context), db_obj)"},{"line_number":184,"context_line":"                for db_obj in db_objects]"},{"line_number":185,"context_line":""},{"line_number":186,"context_line":"    def do_changes_for_db(self):"},{"line_number":187,"context_line":"        \"\"\"Returns a dict of changed fields and their new values."},{"line_number":188,"context_line":""},{"line_number":189,"context_line":"        These are changes for the object that are to be saved in the"}],"source_content_type":"text/x-python","patch_set":45,"id":"1f013ff3_93521067","line":186,"in_reply_to":"3f044301_28c72b48","updated":"2017-05-15 16:05:25.000000000","message":"I probably need to update this description since it isn\u0027t clear. L199 sez, \u0027If needed, the object will be converted to the desired version\u0027.\n\nSo it doesn\u0027t just \u0027get\u0027 something; the object itself might be changed to be in a different version.\n\nWhat do you suggest?","commit_id":"d56a2b3f5d7cc3c9d87bb0f0f3daa9c7e1c9f3d5"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"351611a00f07942dcf45db494bc012a931eefbd1","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from oslo_log import log"},{"line_number":18,"context_line":"from oslo_utils import versionutils"},{"line_number":19,"context_line":"from oslo_versionedobjects import base as object_base"},{"line_number":20,"context_line":"from oslo_versionedobjects import exception"},{"line_number":21,"context_line":"import six"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from ironic.common import release_mappings as versions"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_3e64ea21","line":20,"range":{"start_line":20,"start_character":33,"end_line":20,"end_character":43},"updated":"2017-05-20 18:03:35.000000000","message":"Hrm, we don\u0027t usually raise non-ironic exceptions, not sure we should do this. At least would be good to make it ovo_exception for clarity.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"30317271b7b84f9236346ba8748b12fb351a75e5","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from oslo_log import log"},{"line_number":18,"context_line":"from oslo_utils import versionutils"},{"line_number":19,"context_line":"from oslo_versionedobjects import base as object_base"},{"line_number":20,"context_line":"from oslo_versionedobjects import exception"},{"line_number":21,"context_line":"import six"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from ironic.common import release_mappings as versions"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_6efeb06b","line":20,"range":{"start_line":20,"start_character":33,"end_line":20,"end_character":43},"in_reply_to":"ff0f0b1f_3e64ea21","updated":"2017-05-25 02:16:58.000000000","message":"Good point. Let me think about this one.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6b72138e8e9a8cd913c2074565f6f5a7ff0f8bac","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from oslo_log import log"},{"line_number":18,"context_line":"from oslo_utils import versionutils"},{"line_number":19,"context_line":"from oslo_versionedobjects import base as object_base"},{"line_number":20,"context_line":"from oslo_versionedobjects import exception"},{"line_number":21,"context_line":"import six"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from ironic.common import release_mappings as versions"}],"source_content_type":"text/x-python","patch_set":46,"id":"df140735_062684b8","line":20,"range":{"start_line":20,"start_character":33,"end_line":20,"end_character":43},"in_reply_to":"ff0f0b1f_6efeb06b","updated":"2017-05-31 23:17:34.000000000","message":"I\u0027m not quite sure what the right thing to do is. I could add a new ironic exception. Although we could also get ovo exceptions from serializing/deserializing, so I\u0027m not sure if it is worth having ironic \u0026 ovo exceptions for similar things. (and i think i just noticed a bug with ovo code, sigh.)","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"351611a00f07942dcf45db494bc012a931eefbd1","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                    self[field] !\u003d loaded_object[field]):"},{"line_number":82,"context_line":"                self[field] \u003d loaded_object[field]"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def convert_to_version(self, target_version):"},{"line_number":85,"context_line":"        \"\"\"Convert this object to the target version."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        Subclasses should make any changes to the object (e.g. do any"},{"line_number":88,"context_line":"        migrations) to convert the object to the specified version."},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        :param target_version: the desired version of the object"},{"line_number":91,"context_line":"        \"\"\""},{"line_number":92,"context_line":"        self.VERSION \u003d target_version"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def get_target_version(self):"},{"line_number":95,"context_line":"        \"\"\"Returns the target version for this object."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        This is the version in which the object should be manipulated, e.g."},{"line_number":98,"context_line":"        sent over the wire via RPC."},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        :returns: if pinned, returns the version of this object corresponding"},{"line_number":101,"context_line":"                  to the pin. Otherwise, returns the version of the object."},{"line_number":102,"context_line":"        \"\"\""},{"line_number":103,"context_line":"        pinned_version \u003d None"},{"line_number":104,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":105,"context_line":"        if pin:"},{"line_number":106,"context_line":"            version_manifest \u003d versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":107,"context_line":"            pinned_version \u003d version_manifest.get(self.obj_name())"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        return pinned_version or self.VERSION"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"class IronicDBObject(IronicObject):"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_192380aa","line":109,"range":{"start_line":84,"start_character":0,"end_line":109,"end_character":45},"updated":"2017-05-20 18:03:35.000000000","message":"ditto, I suppose we should not care about these methods for not-in-DB objects, why not just move it to IronicDBObject? And don\u0027t use the VERSION at all, only version.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"30317271b7b84f9236346ba8748b12fb351a75e5","unresolved":false,"context_lines":[{"line_number":81,"context_line":"                    self[field] !\u003d loaded_object[field]):"},{"line_number":82,"context_line":"                self[field] \u003d loaded_object[field]"},{"line_number":83,"context_line":""},{"line_number":84,"context_line":"    def convert_to_version(self, target_version):"},{"line_number":85,"context_line":"        \"\"\"Convert this object to the target version."},{"line_number":86,"context_line":""},{"line_number":87,"context_line":"        Subclasses should make any changes to the object (e.g. do any"},{"line_number":88,"context_line":"        migrations) to convert the object to the specified version."},{"line_number":89,"context_line":""},{"line_number":90,"context_line":"        :param target_version: the desired version of the object"},{"line_number":91,"context_line":"        \"\"\""},{"line_number":92,"context_line":"        self.VERSION \u003d target_version"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    def get_target_version(self):"},{"line_number":95,"context_line":"        \"\"\"Returns the target version for this object."},{"line_number":96,"context_line":""},{"line_number":97,"context_line":"        This is the version in which the object should be manipulated, e.g."},{"line_number":98,"context_line":"        sent over the wire via RPC."},{"line_number":99,"context_line":""},{"line_number":100,"context_line":"        :returns: if pinned, returns the version of this object corresponding"},{"line_number":101,"context_line":"                  to the pin. Otherwise, returns the version of the object."},{"line_number":102,"context_line":"        \"\"\""},{"line_number":103,"context_line":"        pinned_version \u003d None"},{"line_number":104,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":105,"context_line":"        if pin:"},{"line_number":106,"context_line":"            version_manifest \u003d versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":107,"context_line":"            pinned_version \u003d version_manifest.get(self.obj_name())"},{"line_number":108,"context_line":""},{"line_number":109,"context_line":"        return pinned_version or self.VERSION"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"class IronicDBObject(IronicObject):"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_ae084891","line":109,"range":{"start_line":84,"start_character":0,"end_line":109,"end_character":45},"in_reply_to":"ff0f0b1f_192380aa","updated":"2017-05-25 02:16:58.000000000","message":"I think the versioning is separate from the DB. Eg, The object versions used by one service (regardless of whether the objects are saved in a DB or not) can be different from the versions used by another service.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"351611a00f07942dcf45db494bc012a931eefbd1","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        return pinned_version or self.VERSION"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"class IronicDBObject(IronicObject):"},{"line_number":113,"context_line":"    \"\"\"An ironic object that is stored in a DB."},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    This is a versioned Ironic object that may be stored in a DB. it supports"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_de3e0ed9","line":112,"updated":"2017-05-20 18:03:35.000000000","message":"So, this object should contain the created_at and updated_at, not the IronicObject I suppose, as if an object is not created in the DB they won\u0027t be set anyway. (e.g. there is no DB object for any of notifications, so this mixin won\u0027t get triggered https://github.com/openstack/oslo.db/blob/8be136528e99d8ea6ae8f1ae8767738b4c876759/oslo_db/sqlalchemy/models.py#L137)","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6b72138e8e9a8cd913c2074565f6f5a7ff0f8bac","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        return pinned_version or self.VERSION"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"class IronicDBObject(IronicObject):"},{"line_number":113,"context_line":"    \"\"\"An ironic object that is stored in a DB."},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    This is a versioned Ironic object that may be stored in a DB. it supports"}],"source_content_type":"text/x-python","patch_set":46,"id":"df140735_e63cb0c9","line":112,"in_reply_to":"ff0f0b1f_cebedcac","updated":"2017-05-31 23:17:34.000000000","message":"Ok, i simplified things; no IronicDBObject class any more.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"30317271b7b84f9236346ba8748b12fb351a75e5","unresolved":false,"context_lines":[{"line_number":109,"context_line":"        return pinned_version or self.VERSION"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":""},{"line_number":112,"context_line":"class IronicDBObject(IronicObject):"},{"line_number":113,"context_line":"    \"\"\"An ironic object that is stored in a DB."},{"line_number":114,"context_line":""},{"line_number":115,"context_line":"    This is a versioned Ironic object that may be stored in a DB. it supports"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_cebedcac","line":112,"in_reply_to":"ff0f0b1f_de3e0ed9","updated":"2017-05-25 02:16:58.000000000","message":"Right. There\u0027s a comment/TODO L60-61 about changing that; not sure whether they should be moved to this object, or whether they should be moved to a separate mixin class, or whether any of it should be in this patch or a separate one.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":19593,"name":"Anup Navare","email":"anup.d.navare@intel.com","username":"anup"},"change_message_id":"77d9fa9dddafd97af3f8e77fc2fd2e950d284152","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        :param obj: An object of the class."},{"line_number":138,"context_line":"        :param db_object: A DB model of the object"},{"line_number":139,"context_line":"        :return: The object of the class with the database entity added"},{"line_number":140,"context_line":"        \"\"\""},{"line_number":141,"context_line":"        objname \u003d obj.obj_name()"},{"line_number":142,"context_line":"        version \u003d db_object[\u0027version\u0027]"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_fc5a084e","line":140,"range":{"start_line":140,"start_character":8,"end_line":140,"end_character":11},"updated":"2017-05-19 21:38:04.000000000","message":"Missing raises in doc string","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6b72138e8e9a8cd913c2074565f6f5a7ff0f8bac","unresolved":false,"context_lines":[{"line_number":137,"context_line":"        :param obj: An object of the class."},{"line_number":138,"context_line":"        :param db_object: A DB model of the object"},{"line_number":139,"context_line":"        :return: The object of the class with the database entity added"},{"line_number":140,"context_line":"        \"\"\""},{"line_number":141,"context_line":"        objname \u003d obj.obj_name()"},{"line_number":142,"context_line":"        version \u003d db_object[\u0027version\u0027]"},{"line_number":143,"context_line":""}],"source_content_type":"text/x-python","patch_set":46,"id":"df140735_863174ef","line":140,"range":{"start_line":140,"start_character":8,"end_line":140,"end_character":11},"in_reply_to":"ff0f0b1f_fc5a084e","updated":"2017-05-31 23:17:34.000000000","message":"Done","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"351611a00f07942dcf45db494bc012a931eefbd1","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            version \u003d versions.RELEASE_MAPPING[\u0027ocata\u0027][\u0027objects\u0027].get("},{"line_number":158,"context_line":"                objname, \u00271.0\u0027)"},{"line_number":159,"context_line":"        if not versionutils.is_compatible(version, obj.VERSION):"},{"line_number":160,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":161,"context_line":"                objname\u003dobjname, objver\u003dversion, supported\u003dobj.VERSION)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        for field in obj.fields:"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_debbae6f","line":160,"updated":"2017-05-20 18:03:35.000000000","message":"This means that something has gone wrong during the migration? I wonder if we should add some log message stating this.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6b72138e8e9a8cd913c2074565f6f5a7ff0f8bac","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            version \u003d versions.RELEASE_MAPPING[\u0027ocata\u0027][\u0027objects\u0027].get("},{"line_number":158,"context_line":"                objname, \u00271.0\u0027)"},{"line_number":159,"context_line":"        if not versionutils.is_compatible(version, obj.VERSION):"},{"line_number":160,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":161,"context_line":"                objname\u003dobjname, objver\u003dversion, supported\u003dobj.VERSION)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        for field in obj.fields:"}],"source_content_type":"text/x-python","patch_set":46,"id":"df140735_b164b4e5","line":160,"in_reply_to":"ff0f0b1f_ceec9cbc","updated":"2017-05-31 23:17:34.000000000","message":"I logged an error for this.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"30317271b7b84f9236346ba8748b12fb351a75e5","unresolved":false,"context_lines":[{"line_number":157,"context_line":"            version \u003d versions.RELEASE_MAPPING[\u0027ocata\u0027][\u0027objects\u0027].get("},{"line_number":158,"context_line":"                objname, \u00271.0\u0027)"},{"line_number":159,"context_line":"        if not versionutils.is_compatible(version, obj.VERSION):"},{"line_number":160,"context_line":"            raise exception.IncompatibleObjectVersion("},{"line_number":161,"context_line":"                objname\u003dobjname, objver\u003dversion, supported\u003dobj.VERSION)"},{"line_number":162,"context_line":""},{"line_number":163,"context_line":"        for field in obj.fields:"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_ceec9cbc","line":160,"in_reply_to":"ff0f0b1f_debbae6f","updated":"2017-05-25 02:16:58.000000000","message":"Yeah, it could be an internal error (although I don\u0027t think so, I think there are enough unit tests to catch incompatibilities), but more likely user error (eg, mucking with the DB directly or maybe even incorrectly pinning the OLDER release instead of the newer release. Maybe... I\u0027m wondering about it...)","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"351611a00f07942dcf45db494bc012a931eefbd1","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        for field in obj.fields:"},{"line_number":164,"context_line":"            obj[field] \u003d db_object[field]"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        obj.VERSION \u003d version"},{"line_number":167,"context_line":"        obj.version \u003d version"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        obj._context \u003d context"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_7e81428f","line":166,"updated":"2017-05-20 18:03:35.000000000","message":"Not sure I get why do we need to store the same info in both variables, why not just use version?","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"30317271b7b84f9236346ba8748b12fb351a75e5","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        for field in obj.fields:"},{"line_number":164,"context_line":"            obj[field] \u003d db_object[field]"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        obj.VERSION \u003d version"},{"line_number":167,"context_line":"        obj.version \u003d version"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        obj._context \u003d context"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_ee4d20c4","line":166,"in_reply_to":"ff0f0b1f_7e81428f","updated":"2017-05-25 02:16:58.000000000","message":"Because I wanted to keep the different versions separate. Maybe it is too confusing; let me see if I can make it clearer.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6b72138e8e9a8cd913c2074565f6f5a7ff0f8bac","unresolved":false,"context_lines":[{"line_number":163,"context_line":"        for field in obj.fields:"},{"line_number":164,"context_line":"            obj[field] \u003d db_object[field]"},{"line_number":165,"context_line":""},{"line_number":166,"context_line":"        obj.VERSION \u003d version"},{"line_number":167,"context_line":"        obj.version \u003d version"},{"line_number":168,"context_line":""},{"line_number":169,"context_line":"        obj._context \u003d context"}],"source_content_type":"text/x-python","patch_set":46,"id":"df140735_e641503b","line":166,"in_reply_to":"ff0f0b1f_ee4d20c4","updated":"2017-05-31 23:17:34.000000000","message":"ok, I got rid of obj.version","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":19593,"name":"Anup Navare","email":"anup.d.navare@intel.com","username":"anup"},"change_message_id":"77d9fa9dddafd97af3f8e77fc2fd2e950d284152","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        return [cls._from_db_object(context, cls(), db_obj)"},{"line_number":186,"context_line":"                for db_obj in db_objects]"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def do_version_changes_for_db(self):"},{"line_number":189,"context_line":"        \"\"\"Do any changes to the object before saving it in the DB."},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        This determines which version of the object should be saved to the"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_5ec8c920","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":34},"updated":"2017-05-19 21:38:04.000000000","message":"nit: add raises AttributeError in doc string which is raised on obj_attr_is_set()","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6b72138e8e9a8cd913c2074565f6f5a7ff0f8bac","unresolved":false,"context_lines":[{"line_number":185,"context_line":"        return [cls._from_db_object(context, cls(), db_obj)"},{"line_number":186,"context_line":"                for db_obj in db_objects]"},{"line_number":187,"context_line":""},{"line_number":188,"context_line":"    def do_version_changes_for_db(self):"},{"line_number":189,"context_line":"        \"\"\"Do any changes to the object before saving it in the DB."},{"line_number":190,"context_line":""},{"line_number":191,"context_line":"        This determines which version of the object should be saved to the"}],"source_content_type":"text/x-python","patch_set":46,"id":"df140735_667780a2","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":34},"in_reply_to":"ff0f0b1f_5ec8c920","updated":"2017-05-31 23:17:34.000000000","message":"got rid of obj_attr_is_set :-)","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"351611a00f07942dcf45db494bc012a931eefbd1","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        * If the object isn\u0027t pinned, we save the object in the latest version."},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        Because the object may be converted to a different object version,"},{"line_number":211,"context_line":"        this method should only be called just before saving the object to"},{"line_number":212,"context_line":"        the DB."},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        :returns: a dictionary of changed fields and their new values"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_be60fa02","line":211,"updated":"2017-05-20 18:03:35.000000000","message":"I\u0027d vote for having a base save method in this class calling this function, but maybe this has more implications so I guess can be done separately.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"6b72138e8e9a8cd913c2074565f6f5a7ff0f8bac","unresolved":false,"context_lines":[{"line_number":208,"context_line":"        * If the object isn\u0027t pinned, we save the object in the latest version."},{"line_number":209,"context_line":""},{"line_number":210,"context_line":"        Because the object may be converted to a different object version,"},{"line_number":211,"context_line":"        this method should only be called just before saving the object to"},{"line_number":212,"context_line":"        the DB."},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"        :returns: a dictionary of changed fields and their new values"}],"source_content_type":"text/x-python","patch_set":46,"id":"df140735_c6aaccd0","line":211,"in_reply_to":"ff0f0b1f_be60fa02","updated":"2017-05-31 23:17:34.000000000","message":"I\u0027m afraid to do more refactoring; this isn\u0027t called at the same time as the actual DB save, and the DB save is not the same method name for all the objects.\n\nI thought about just redefining obj_get_changes(), but I\u0027m a bit hesitant in case that causes issues in the future. (cuz eg, we\u0027re actually modifying the object here, and obj_get_changes() might be called for other reasons besides saving (even though that\u0027s all we\u0027re doing with it for now).","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"351611a00f07942dcf45db494bc012a931eefbd1","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            # Convert the object so we can save it in the target version."},{"line_number":231,"context_line":"            self.convert_to_version(target_version)"},{"line_number":232,"context_line":"        elif (CONF.pin_release_version is None and"},{"line_number":233,"context_line":"              self.VERSION !\u003d self.__class__.VERSION):"},{"line_number":234,"context_line":"            # NOTE(rloo): Not pinned and the object is in an older version."},{"line_number":235,"context_line":"            # We want to save this in the latest version, so upgrade it."},{"line_number":236,"context_line":"            self.convert_to_version(self.__class__.VERSION)"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_79315cec","line":233,"range":{"start_line":233,"start_character":14,"end_line":233,"end_character":52},"updated":"2017-05-20 18:03:35.000000000","message":"Why would this happen? Apart from someone setting the object\u0027s VERSION manually?","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"30317271b7b84f9236346ba8748b12fb351a75e5","unresolved":false,"context_lines":[{"line_number":230,"context_line":"            # Convert the object so we can save it in the target version."},{"line_number":231,"context_line":"            self.convert_to_version(target_version)"},{"line_number":232,"context_line":"        elif (CONF.pin_release_version is None and"},{"line_number":233,"context_line":"              self.VERSION !\u003d self.__class__.VERSION):"},{"line_number":234,"context_line":"            # NOTE(rloo): Not pinned and the object is in an older version."},{"line_number":235,"context_line":"            # We want to save this in the latest version, so upgrade it."},{"line_number":236,"context_line":"            self.convert_to_version(self.__class__.VERSION)"}],"source_content_type":"text/x-python","patch_set":46,"id":"ff0f0b1f_ae4e0894","line":233,"range":{"start_line":233,"start_character":14,"end_line":233,"end_character":52},"in_reply_to":"ff0f0b1f_79315cec","updated":"2017-05-25 02:16:58.000000000","message":"Let\u0027s say an object can be in version N-1 or N. The object is saved in version N-1. The code is upgraded and the rolling upgrade is done, nothing is pinned any more, but the db migration script hasn\u0027t been run). Now the object is being modified. L166 above will set self.VERSION to be N-1. When we save the object, we don\u0027t want to save it in version N-1, we want to save it in N. self.__class__.VERSION is N; self.VERSION is N-1.","commit_id":"cb3c1fec74fadc98ab2caec162dac2a1705c2239"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c3b45ec38f7fd787e45bb096e40df1af8e0881b3","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        :raises: ovo_exception.IncompatibleObjectVersion"},{"line_number":121,"context_line":"        \"\"\""},{"line_number":122,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":123,"context_line":"        if pin:"},{"line_number":124,"context_line":"            version_manifest \u003d versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":125,"context_line":"            pinned_version \u003d version_manifest.get(self.obj_name())"},{"line_number":126,"context_line":"            if pinned_version:"}],"source_content_type":"text/x-python","patch_set":49,"id":"9f0e0f27_262688b9","line":123,"updated":"2017-06-09 09:51:10.000000000","message":"nit:\n\n if not pin:\n   return self.__class__.VERSION","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"213227856b3c0710c9d5eec37b2a4b73e4fe69eb","unresolved":false,"context_lines":[{"line_number":120,"context_line":"        :raises: ovo_exception.IncompatibleObjectVersion"},{"line_number":121,"context_line":"        \"\"\""},{"line_number":122,"context_line":"        pin \u003d CONF.pin_release_version"},{"line_number":123,"context_line":"        if pin:"},{"line_number":124,"context_line":"            version_manifest \u003d versions.RELEASE_MAPPING[pin][\u0027objects\u0027]"},{"line_number":125,"context_line":"            pinned_version \u003d version_manifest.get(self.obj_name())"},{"line_number":126,"context_line":"            if pinned_version:"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_6b99e11d","line":123,"in_reply_to":"9f0e0f27_262688b9","updated":"2017-06-09 22:24:09.000000000","message":"+1","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"213227856b3c0710c9d5eec37b2a4b73e4fe69eb","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            obj[field] \u003d db_object[field]"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        obj._context \u003d context"},{"line_number":188,"context_line":"        obj.obj_reset_changes()"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        if db_version !\u003d obj.__class__.VERSION:"},{"line_number":191,"context_line":"            # convert to the latest version"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_4b13c567","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":31},"updated":"2017-06-09 22:24:09.000000000","message":"It feels wrong to reset here and potentially do it if the db version !\u003d object version below. I wonder if this could all be refactored at some point... but not today.","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a765829eea932f7e644e1d0aa8235e7039fe9539","unresolved":false,"context_lines":[{"line_number":185,"context_line":"            obj[field] \u003d db_object[field]"},{"line_number":186,"context_line":""},{"line_number":187,"context_line":"        obj._context \u003d context"},{"line_number":188,"context_line":"        obj.obj_reset_changes()"},{"line_number":189,"context_line":""},{"line_number":190,"context_line":"        if db_version !\u003d obj.__class__.VERSION:"},{"line_number":191,"context_line":"            # convert to the latest version"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_beb14492","line":188,"range":{"start_line":188,"start_character":8,"end_line":188,"end_character":31},"in_reply_to":"7f231b9d_4b13c567","updated":"2017-06-12 20:07:52.000000000","message":"Hmm. I don\u0027t actually want to retain these diffs/changes. If we didn\u0027t do any modifications due to versioning, this would be the normal thing to do -- we\u0027d return an object that represents the DB info; no changes in flight for the object.\n\nBelow -- if we end up changing the object so that it is in a different version than what it had been stored in the DB, I DO want the changes -- only the changes that are needed for changing the version. The reason being that some time later, it is possible that this object gets saved in the changed version -- in which case, I want to be sure that we are saving the converted changes.","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c3b45ec38f7fd787e45bb096e40df1af8e0881b3","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        if db_version !\u003d obj.__class__.VERSION:"},{"line_number":191,"context_line":"            # convert to the latest version"},{"line_number":192,"context_line":"            obj.convert_to_version(obj.__class__.VERSION)"},{"line_number":193,"context_line":"            if obj.get_target_version() \u003d\u003d db_version:"},{"line_number":194,"context_line":"                # pinned, so no need to keep these changes (we\u0027ll end up"},{"line_number":195,"context_line":"                # converting back to db_version if obj is saved)"},{"line_number":196,"context_line":"                obj.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":49,"id":"9f0e0f27_063b84d1","line":193,"updated":"2017-06-09 09:51:10.000000000","message":"the ordering here surprises me. Why not do this check first? Then you won\u0027t have to convert-then-discard.","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f87bbe208f256f83481dea67cf6aa7433a52ff80","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        if db_version !\u003d obj.__class__.VERSION:"},{"line_number":191,"context_line":"            # convert to the latest version"},{"line_number":192,"context_line":"            obj.convert_to_version(obj.__class__.VERSION)"},{"line_number":193,"context_line":"            if obj.get_target_version() \u003d\u003d db_version:"},{"line_number":194,"context_line":"                # pinned, so no need to keep these changes (we\u0027ll end up"},{"line_number":195,"context_line":"                # converting back to db_version if obj is saved)"},{"line_number":196,"context_line":"                obj.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_8172597a","line":193,"in_reply_to":"7f231b9d_990fc22d","updated":"2017-06-14 19:21:56.000000000","message":"Discussed with Dmitry on IRC [1]; the confusion was with obj.obj_reset_changes() -- it doesn\u0027t undo the changes to the object; it just clears its internal \u0027_changed_fields\u0027 attribute.\n\n[1] http://eavesdrop.openstack.org/irclogs/%23openstack-ironic/%23openstack-ironic.2017-06-13.log.html#t2017-06-13T13:49:26","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a765829eea932f7e644e1d0aa8235e7039fe9539","unresolved":false,"context_lines":[{"line_number":190,"context_line":"        if db_version !\u003d obj.__class__.VERSION:"},{"line_number":191,"context_line":"            # convert to the latest version"},{"line_number":192,"context_line":"            obj.convert_to_version(obj.__class__.VERSION)"},{"line_number":193,"context_line":"            if obj.get_target_version() \u003d\u003d db_version:"},{"line_number":194,"context_line":"                # pinned, so no need to keep these changes (we\u0027ll end up"},{"line_number":195,"context_line":"                # converting back to db_version if obj is saved)"},{"line_number":196,"context_line":"                obj.obj_reset_changes()"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_990fc22d","line":193,"in_reply_to":"9f0e0f27_063b84d1","updated":"2017-06-12 20:07:52.000000000","message":"I\u0027m not sure what you mean here, do this check before what? L190? I *do* want the object converted; the question is whether I want to keep around the changes or not. (Only the changes are saved in any future DB save.)\n\nI\u0027ll try to ask you in IRC tomorrow. Maybe I need to change the code and/or add more comments...","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c3b45ec38f7fd787e45bb096e40df1af8e0881b3","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        return [cls._from_db_object(context, cls(), db_obj)"},{"line_number":217,"context_line":"                for db_obj in db_objects]"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def do_version_changes_for_db(self):"},{"line_number":220,"context_line":"        \"\"\"Do any changes to the object before saving it in the DB."},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        This determines which version of the object should be saved to the"}],"source_content_type":"text/x-python","patch_set":49,"id":"9f0e0f27_e63770e3","line":219,"updated":"2017-06-09 09:51:10.000000000","message":"this function does not \"do\" any changes, I\u0027d call it get_...","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a765829eea932f7e644e1d0aa8235e7039fe9539","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        return [cls._from_db_object(context, cls(), db_obj)"},{"line_number":217,"context_line":"                for db_obj in db_objects]"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def do_version_changes_for_db(self):"},{"line_number":220,"context_line":"        \"\"\"Do any changes to the object before saving it in the DB."},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        This determines which version of the object should be saved to the"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_3e62d43f","line":219,"in_reply_to":"7f231b9d_16ba9cdd","updated":"2017-06-12 20:07:52.000000000","message":"It actually can change the object (the self.convert_to_version() call at L245), which is why I was afraid if I called it get_*, someone would say it wasn\u0027t just returning info but also changing something.\n\nI can change it to get_* though.","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"f87bbe208f256f83481dea67cf6aa7433a52ff80","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        return [cls._from_db_object(context, cls(), db_obj)"},{"line_number":217,"context_line":"                for db_obj in db_objects]"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def do_version_changes_for_db(self):"},{"line_number":220,"context_line":"        \"\"\"Do any changes to the object before saving it in the DB."},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        This determines which version of the object should be saved to the"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_81c49992","line":219,"in_reply_to":"7f231b9d_3e62d43f","updated":"2017-06-14 19:21:56.000000000","message":"I started to change this to get_version_changes_for_db(), but I feel that is wrong, since a get_*() shouldn\u0027t modify anything. I *could* change the code to copy the object and make/get the changes of the copied object but I\u0027m not sure it is worth it. Let me know...\n\nI did modify the docstring to be more explicit that the object might change, maybe that will help.","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6637,"name":"Sam Betts","email":"sam@code-smash.net","username":"sambetts"},"change_message_id":"a6a785f2f6935a93196fe725c95f7046401a1db4","unresolved":false,"context_lines":[{"line_number":216,"context_line":"        return [cls._from_db_object(context, cls(), db_obj)"},{"line_number":217,"context_line":"                for db_obj in db_objects]"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def do_version_changes_for_db(self):"},{"line_number":220,"context_line":"        \"\"\"Do any changes to the object before saving it in the DB."},{"line_number":221,"context_line":""},{"line_number":222,"context_line":"        This determines which version of the object should be saved to the"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_16ba9cdd","line":219,"in_reply_to":"9f0e0f27_e63770e3","updated":"2017-06-09 16:55:52.000000000","message":"+1 to get_version_changes_for_db TBH as this is replacing a very similar function that was used before,","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"213227856b3c0710c9d5eec37b2a4b73e4fe69eb","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        The version used to save the object is determined as follows:"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        * If the object is pinned, we save the object in the pinned version."},{"line_number":229,"context_line":"          Since it is pinned, we don\u0027t want to save in a newer version, in case"},{"line_number":230,"context_line":"          a rolling upgrade is happening and some services are still using the"},{"line_number":231,"context_line":"          older version of ironic, with no knowledge of this newer version."},{"line_number":232,"context_line":"        * If the object isn\u0027t pinned, we save the object in the latest version."}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_fdd81880","line":229,"range":{"start_line":229,"start_character":33,"end_line":229,"end_character":43},"updated":"2017-06-09 22:24:09.000000000","message":"nit: I feel like this is a must not?","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"a765829eea932f7e644e1d0aa8235e7039fe9539","unresolved":false,"context_lines":[{"line_number":226,"context_line":"        The version used to save the object is determined as follows:"},{"line_number":227,"context_line":""},{"line_number":228,"context_line":"        * If the object is pinned, we save the object in the pinned version."},{"line_number":229,"context_line":"          Since it is pinned, we don\u0027t want to save in a newer version, in case"},{"line_number":230,"context_line":"          a rolling upgrade is happening and some services are still using the"},{"line_number":231,"context_line":"          older version of ironic, with no knowledge of this newer version."},{"line_number":232,"context_line":"        * If the object isn\u0027t pinned, we save the object in the latest version."}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_9938e2d3","line":229,"range":{"start_line":229,"start_character":33,"end_line":229,"end_character":43},"in_reply_to":"7f231b9d_fdd81880","updated":"2017-06-12 20:07:52.000000000","message":"yup","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":11655,"name":"Julia Kreger","email":"juliaashleykreger@gmail.com","username":"jkreger","status":"Flying to the moon with a Jetpack!"},"change_message_id":"213227856b3c0710c9d5eec37b2a4b73e4fe69eb","unresolved":false,"context_lines":[{"line_number":232,"context_line":"        * If the object isn\u0027t pinned, we save the object in the latest version."},{"line_number":233,"context_line":""},{"line_number":234,"context_line":"        Because the object may be converted to a different object version,"},{"line_number":235,"context_line":"        this method should only be called just before saving the object to"},{"line_number":236,"context_line":"        the DB."},{"line_number":237,"context_line":""},{"line_number":238,"context_line":"        :returns: a dictionary of changed fields and their new values"}],"source_content_type":"text/x-python","patch_set":49,"id":"7f231b9d_9d36c4b4","line":235,"range":{"start_line":235,"start_character":20,"end_line":235,"end_character":26},"updated":"2017-06-09 22:24:09.000000000","message":"nit: s/should/must/","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"},{"author":{"_account_id":10239,"name":"Dmitry Tantsur","email":"dtantsur@protonmail.com","username":"dtantsur"},"change_message_id":"c3b45ec38f7fd787e45bb096e40df1af8e0881b3","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        \"\"\""},{"line_number":241,"context_line":"        target_version \u003d self.get_target_version()"},{"line_number":242,"context_line":""},{"line_number":243,"context_line":"        if (target_version !\u003d self.VERSION):"},{"line_number":244,"context_line":"            # Convert the object so we can save it in the target version."},{"line_number":245,"context_line":"            self.convert_to_version(target_version)"},{"line_number":246,"context_line":"            db_version \u003d target_version"}],"source_content_type":"text/x-python","patch_set":49,"id":"9f0e0f27_c6346ce1","line":243,"updated":"2017-06-09 09:51:10.000000000","message":"nit: redundant brackets","commit_id":"ebfc4fe4c4c3910bf8b1229cb75259befa530877"}],"ironic/tests/unit/api/utils.py":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"71d538f4dfd98d3627096cd34a7a8efb457b7356","unresolved":false,"context_lines":[{"line_number":24,"context_line":"from ironic.api.controllers.v1 import port as port_controller"},{"line_number":25,"context_line":"from ironic.api.controllers.v1 import portgroup as portgroup_controller"},{"line_number":26,"context_line":"from ironic.drivers import base as drivers_base"},{"line_number":27,"context_line":"from ironic.tests.unit.db import utils as dbutils"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"ADMIN_TOKEN \u003d \u00274562138218392831\u0027"},{"line_number":30,"context_line":"MEMBER_TOKEN \u003d \u00274562138218392832\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_6fb6f18b","line":27,"updated":"2016-12-30 21:02:04.000000000","message":"nit: i\u0027m fine with this, but this change seems unrelated","commit_id":"e263a1458505960ed90cc7326680466d7a4d9018"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"057d1129124110d280dee6cef147fbea3c934f99","unresolved":false,"context_lines":[{"line_number":24,"context_line":"from ironic.api.controllers.v1 import port as port_controller"},{"line_number":25,"context_line":"from ironic.api.controllers.v1 import portgroup as portgroup_controller"},{"line_number":26,"context_line":"from ironic.drivers import base as drivers_base"},{"line_number":27,"context_line":"from ironic.tests.unit.db import utils as dbutils"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"ADMIN_TOKEN \u003d \u00274562138218392831\u0027"},{"line_number":30,"context_line":"MEMBER_TOKEN \u003d \u00274562138218392832\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_dfaec780","line":27,"in_reply_to":"ba5201f7_6fb6f18b","updated":"2017-01-09 14:40:43.000000000","message":"I changed it because at first glance I didn\u0027t understand why the HTTP API validation failed when changing the db models. I think it is more intuitive to have some information about what\u0027s going on in the name.","commit_id":"e263a1458505960ed90cc7326680466d7a4d9018"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"24da769a6fbc3d03708ef2631f3831c36b6b93c0","unresolved":false,"context_lines":[{"line_number":24,"context_line":"from ironic.api.controllers.v1 import port as port_controller"},{"line_number":25,"context_line":"from ironic.api.controllers.v1 import portgroup as portgroup_controller"},{"line_number":26,"context_line":"from ironic.drivers import base as drivers_base"},{"line_number":27,"context_line":"from ironic.tests.unit.db import utils as dbutils"},{"line_number":28,"context_line":""},{"line_number":29,"context_line":"ADMIN_TOKEN \u003d \u00274562138218392831\u0027"},{"line_number":30,"context_line":"MEMBER_TOKEN \u003d \u00274562138218392832\u0027"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba5201f7_06d07cf5","line":27,"in_reply_to":"ba5201f7_dfaec780","updated":"2017-01-11 20:21:10.000000000","message":"This change is fine, but I would like it in a separate patch. I think we need to focus on the rolling upgrade changes only.","commit_id":"e263a1458505960ed90cc7326680466d7a4d9018"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"743b64de502b47fb724dc99f581c942d0e5e634b","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def node_post_data(**kw):"},{"line_number":94,"context_line":"    node \u003d utils.get_test_node(**kw)"},{"line_number":95,"context_line":"    del node[\u0027version\u0027]"},{"line_number":96,"context_line":"    # These values are not part of the API object"},{"line_number":97,"context_line":"    node.pop(\u0027conductor_affinity\u0027)"},{"line_number":98,"context_line":"    node.pop(\u0027chassis_id\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a3c09a3_662196b4","line":95,"updated":"2017-01-14 05:57:54.000000000","message":"I think it should either follow the pattern of using pop to delete attribute, or del for all the fields that are not part of the API object.","commit_id":"1d53b0cbe413aa1044896d142596745a31a7a52b"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"691842b5e148a4751a58133121c79be9ceeb8283","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def node_post_data(**kw):"},{"line_number":94,"context_line":"    node \u003d utils.get_test_node(**kw)"},{"line_number":95,"context_line":"    del node[\u0027version\u0027]"},{"line_number":96,"context_line":"    # These values are not part of the API object"},{"line_number":97,"context_line":"    node.pop(\u0027conductor_affinity\u0027)"},{"line_number":98,"context_line":"    node.pop(\u0027chassis_id\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a3c09a3_b482af50","line":95,"in_reply_to":"7a3c09a3_662196b4","updated":"2017-01-16 07:42:02.000000000","message":"Ok, I think del makes more sense, since we are not using the value afterwards.","commit_id":"1d53b0cbe413aa1044896d142596745a31a7a52b"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"c7d90460f7f0b62165dc3434a7fc62ffc91e6436","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def node_post_data(**kw):"},{"line_number":94,"context_line":"    node \u003d utils.get_test_node(**kw)"},{"line_number":95,"context_line":"    del node[\u0027version\u0027]"},{"line_number":96,"context_line":"    # These values are not part of the API object"},{"line_number":97,"context_line":"    node.pop(\u0027conductor_affinity\u0027)"},{"line_number":98,"context_line":"    node.pop(\u0027chassis_id\u0027)"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a3c09a3_ea50c060","line":95,"in_reply_to":"7a3c09a3_b482af50","updated":"2017-01-17 14:46:44.000000000","message":"On the other hand, I see pop() is used in other places, so I\u0027ll change it to that.","commit_id":"1d53b0cbe413aa1044896d142596745a31a7a52b"},{"author":{"_account_id":23883,"name":"Joanna Taryma","email":"joanna.taryma@intel.com","username":"jtaryma"},"change_message_id":"743b64de502b47fb724dc99f581c942d0e5e634b","unresolved":false,"context_lines":[{"line_number":114,"context_line":""},{"line_number":115,"context_line":"def port_post_data(**kw):"},{"line_number":116,"context_line":"    port \u003d utils.get_test_port(**kw)"},{"line_number":117,"context_line":"    del port[\u0027version\u0027]"},{"line_number":118,"context_line":"    # node_id is not part of the API object"},{"line_number":119,"context_line":"    port.pop(\u0027node_id\u0027)"},{"line_number":120,"context_line":"    # portgroup_id is not part of the API object"}],"source_content_type":"text/x-python","patch_set":19,"id":"7a3c09a3_86304203","line":117,"updated":"2017-01-14 05:57:54.000000000","message":"ditto","commit_id":"1d53b0cbe413aa1044896d142596745a31a7a52b"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def node_post_data(**kw):"},{"line_number":94,"context_line":"    node \u003d utils.get_test_node(**kw)"},{"line_number":95,"context_line":"    node.pop(\u0027version\u0027)"},{"line_number":96,"context_line":"    # These values are not part of the API object"},{"line_number":97,"context_line":"    node.pop(\u0027conductor_affinity\u0027)"},{"line_number":98,"context_line":"    node.pop(\u0027chassis_id\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_cbdd0d72","line":95,"updated":"2017-01-19 16:44:28.000000000","message":"I think it may be clearer if we put this after L96, since this isn\u0027t part of the API object either.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"1ba0afef9d4a69f2179472426d4ba417608886cb","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def node_post_data(**kw):"},{"line_number":94,"context_line":"    node \u003d utils.get_test_node(**kw)"},{"line_number":95,"context_line":"    node.pop(\u0027version\u0027)"},{"line_number":96,"context_line":"    # These values are not part of the API object"},{"line_number":97,"context_line":"    node.pop(\u0027conductor_affinity\u0027)"},{"line_number":98,"context_line":"    node.pop(\u0027chassis_id\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_dae5d8a8","line":95,"updated":"2017-01-19 16:55:48.000000000","message":"please move this one line down :) same for other places","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":92,"context_line":""},{"line_number":93,"context_line":"def node_post_data(**kw):"},{"line_number":94,"context_line":"    node \u003d utils.get_test_node(**kw)"},{"line_number":95,"context_line":"    node.pop(\u0027version\u0027)"},{"line_number":96,"context_line":"    # These values are not part of the API object"},{"line_number":97,"context_line":"    node.pop(\u0027conductor_affinity\u0027)"},{"line_number":98,"context_line":"    node.pop(\u0027chassis_id\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_938f9622","line":95,"in_reply_to":"5a3905b3_dae5d8a8","updated":"2017-01-20 09:31:54.000000000","message":"Done","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":114,"context_line":""},{"line_number":115,"context_line":"def port_post_data(**kw):"},{"line_number":116,"context_line":"    port \u003d utils.get_test_port(**kw)"},{"line_number":117,"context_line":"    port.pop(\u0027version\u0027)"},{"line_number":118,"context_line":"    # node_id is not part of the API object"},{"line_number":119,"context_line":"    port.pop(\u0027node_id\u0027)"},{"line_number":120,"context_line":"    # portgroup_id is not part of the API object"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_8b41450e","line":117,"updated":"2017-01-19 16:44:28.000000000","message":"ditto, probably combine this with the other two below.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":114,"context_line":""},{"line_number":115,"context_line":"def port_post_data(**kw):"},{"line_number":116,"context_line":"    port \u003d utils.get_test_port(**kw)"},{"line_number":117,"context_line":"    port.pop(\u0027version\u0027)"},{"line_number":118,"context_line":"    # node_id is not part of the API object"},{"line_number":119,"context_line":"    port.pop(\u0027node_id\u0027)"},{"line_number":120,"context_line":"    # portgroup_id is not part of the API object"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_f3ae1a97","line":117,"in_reply_to":"5a3905b3_8b41450e","updated":"2017-01-20 09:31:54.000000000","message":"Done","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"def chassis_post_data(**kw):"},{"line_number":127,"context_line":"    chassis \u003d utils.get_test_chassis(**kw)"},{"line_number":128,"context_line":"    chassis.pop(\u0027version\u0027)"},{"line_number":129,"context_line":"    internal \u003d chassis_controller.ChassisPatchType.internal_attrs()"},{"line_number":130,"context_line":"    return remove_internal(chassis, internal)"},{"line_number":131,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_4b5e7dea","line":128,"updated":"2017-01-19 16:44:28.000000000","message":"to be consistent, add # version is not part of the API object","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":125,"context_line":""},{"line_number":126,"context_line":"def chassis_post_data(**kw):"},{"line_number":127,"context_line":"    chassis \u003d utils.get_test_chassis(**kw)"},{"line_number":128,"context_line":"    chassis.pop(\u0027version\u0027)"},{"line_number":129,"context_line":"    internal \u003d chassis_controller.ChassisPatchType.internal_attrs()"},{"line_number":130,"context_line":"    return remove_internal(chassis, internal)"},{"line_number":131,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_1396464c","line":128,"in_reply_to":"5a3905b3_4b5e7dea","updated":"2017-01-20 09:31:54.000000000","message":"Done","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":135,"context_line":"    #                    we have to use chassis_uuid"},{"line_number":136,"context_line":"    node \u003d node_post_data(**kw)"},{"line_number":137,"context_line":"    chassis \u003d utils.get_test_chassis()"},{"line_number":138,"context_line":"    chassis.pop(\u0027version\u0027)"},{"line_number":139,"context_line":"    node[\u0027chassis_uuid\u0027] \u003d kw.get(\u0027chassis_uuid\u0027, chassis[\u0027uuid\u0027])"},{"line_number":140,"context_line":"    return node"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_ab5d21eb","line":138,"updated":"2017-01-19 16:44:28.000000000","message":"this is not needed; it is only used for getting the chassis\u0027 uuid below.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":135,"context_line":"    #                    we have to use chassis_uuid"},{"line_number":136,"context_line":"    node \u003d node_post_data(**kw)"},{"line_number":137,"context_line":"    chassis \u003d utils.get_test_chassis()"},{"line_number":138,"context_line":"    chassis.pop(\u0027version\u0027)"},{"line_number":139,"context_line":"    node[\u0027chassis_uuid\u0027] \u003d kw.get(\u0027chassis_uuid\u0027, chassis[\u0027uuid\u0027])"},{"line_number":140,"context_line":"    return node"},{"line_number":141,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_73722a4b","line":138,"in_reply_to":"5a3905b3_ab5d21eb","updated":"2017-01-20 09:31:54.000000000","message":"Done","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    \"\"\"Return a Portgroup object without internal attributes.\"\"\""},{"line_number":145,"context_line":"    portgroup \u003d utils.get_test_portgroup(**kw)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    portgroup.pop(\u0027version\u0027)"},{"line_number":148,"context_line":"    # node_id is not a part of the API object"},{"line_number":149,"context_line":"    portgroup.pop(\u0027node_id\u0027)"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_0bc5553b","line":147,"updated":"2017-01-19 16:44:28.000000000","message":"move this after L148 and update L148.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":144,"context_line":"    \"\"\"Return a Portgroup object without internal attributes.\"\"\""},{"line_number":145,"context_line":"    portgroup \u003d utils.get_test_portgroup(**kw)"},{"line_number":146,"context_line":""},{"line_number":147,"context_line":"    portgroup.pop(\u0027version\u0027)"},{"line_number":148,"context_line":"    # node_id is not a part of the API object"},{"line_number":149,"context_line":"    portgroup.pop(\u0027node_id\u0027)"},{"line_number":150,"context_line":""}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_d382feff","line":147,"in_reply_to":"5a3905b3_0bc5553b","updated":"2017-01-20 09:31:54.000000000","message":"Done","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":164,"context_line":"    \"\"\"Return a Portgroup object with appropriate attributes.\"\"\""},{"line_number":165,"context_line":"    portgroup \u003d portgroup_post_data(**kw)"},{"line_number":166,"context_line":"    node \u003d utils.get_test_node()"},{"line_number":167,"context_line":"    node.pop(\u0027version\u0027)"},{"line_number":168,"context_line":"    portgroup[\u0027node_uuid\u0027] \u003d kw.get(\u0027node_uuid\u0027, node[\u0027uuid\u0027])"},{"line_number":169,"context_line":"    return portgroup"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_8bddc544","line":167,"updated":"2017-01-19 16:44:28.000000000","message":"this is not needed; it is only used for getting the node\u0027s uuid below.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"}],"ironic/tests/unit/db/sqlalchemy/test_migrations.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":652,"context_line":"        self.assertIsInstance(ifaces.c.interface_name.type,"},{"line_number":653,"context_line":"                              sqlalchemy.types.String)"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"    def _check_dbefd6bdaa2c(self, engine, data):"},{"line_number":657,"context_line":"        ifaces \u003d db_utils.get_table(engine, \u0027conductor_hardware_interfaces\u0027)"},{"line_number":658,"context_line":"        col_names \u003d [column.name for column in ifaces.c]"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_2b1e91ba","line":655,"updated":"2017-01-19 16:44:28.000000000","message":"don\u0027t need this extra blank line","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":652,"context_line":"        self.assertIsInstance(ifaces.c.interface_name.type,"},{"line_number":653,"context_line":"                              sqlalchemy.types.String)"},{"line_number":654,"context_line":""},{"line_number":655,"context_line":""},{"line_number":656,"context_line":"    def _check_dbefd6bdaa2c(self, engine, data):"},{"line_number":657,"context_line":"        ifaces \u003d db_utils.get_table(engine, \u0027conductor_hardware_interfaces\u0027)"},{"line_number":658,"context_line":"        col_names \u003d [column.name for column in ifaces.c]"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_d3f3de08","line":655,"in_reply_to":"5a3905b3_2b1e91ba","updated":"2017-01-20 09:31:54.000000000","message":"Done","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"}],"ironic/tests/unit/db/utils.py":[{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"440fd02ee85e13cae18fc02f9b46789cfd470be4","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        \"private_state\": \"secret value\""},{"line_number":207,"context_line":"    }"},{"line_number":208,"context_line":"    result \u003d {"},{"line_number":209,"context_line":"        \u0027version\u0027: node.Node.VERSION,"},{"line_number":210,"context_line":"        \u0027id\u0027: kw.get(\u0027id\u0027, 123),"},{"line_number":211,"context_line":"        \u0027name\u0027: kw.get(\u0027name\u0027, None),"},{"line_number":212,"context_line":"        \u0027uuid\u0027: kw.get(\u0027uuid\u0027, \u00271be26c0b-03f2-4d2e-ae87-c02d7f33c123\u0027),"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_f26a6080","line":209,"updated":"2017-01-16 21:05:42.000000000","message":"I was wondering whether it might make sense to do a similar thing as the others, eg kw.get(\u0027version\u0027, node.Node.VERSION). if we want to add a unit test that tests some specific (or different) versions of a node. And also, cuz it is more consistent.","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"cc6492157f4f4a1d9750198748fef2231e83fa1b","unresolved":false,"context_lines":[{"line_number":206,"context_line":"        \"private_state\": \"secret value\""},{"line_number":207,"context_line":"    }"},{"line_number":208,"context_line":"    result \u003d {"},{"line_number":209,"context_line":"        \u0027version\u0027: node.Node.VERSION,"},{"line_number":210,"context_line":"        \u0027id\u0027: kw.get(\u0027id\u0027, 123),"},{"line_number":211,"context_line":"        \u0027name\u0027: kw.get(\u0027name\u0027, None),"},{"line_number":212,"context_line":"        \u0027uuid\u0027: kw.get(\u0027uuid\u0027, \u00271be26c0b-03f2-4d2e-ae87-c02d7f33c123\u0027),"}],"source_content_type":"text/x-python","patch_set":21,"id":"7a3c09a3_c231eb5b","line":209,"in_reply_to":"7a3c09a3_f26a6080","updated":"2017-01-17 14:46:52.000000000","message":"Done","commit_id":"847caa0935e9b5a31897efb89600631a48847d74"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":486,"context_line":""},{"line_number":487,"context_line":"def get_test_node_tag(**kw):"},{"line_number":488,"context_line":"    return {"},{"line_number":489,"context_line":"        \"tag\": kw.get(\"tag\", \"tag1\"),"},{"line_number":490,"context_line":"        \"node_id\": kw.get(\"node_id\", \"123\"),"},{"line_number":491,"context_line":"        \u0027created_at\u0027: kw.get(\u0027created_at\u0027),"},{"line_number":492,"context_line":"        \u0027updated_at\u0027: kw.get(\u0027updated_at\u0027),"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_46d3d438","line":489,"updated":"2017-01-19 16:44:28.000000000","message":"Did we decide to skip this? I think we should add version here because it exists at this level.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":486,"context_line":""},{"line_number":487,"context_line":"def get_test_node_tag(**kw):"},{"line_number":488,"context_line":"    return {"},{"line_number":489,"context_line":"        \"tag\": kw.get(\"tag\", \"tag1\"),"},{"line_number":490,"context_line":"        \"node_id\": kw.get(\"node_id\", \"123\"),"},{"line_number":491,"context_line":"        \u0027created_at\u0027: kw.get(\u0027created_at\u0027),"},{"line_number":492,"context_line":"        \u0027updated_at\u0027: kw.get(\u0027updated_at\u0027),"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_13d2c600","line":489,"in_reply_to":"5a3905b3_46d3d438","updated":"2017-01-20 09:31:54.000000000","message":"Done","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"}],"ironic/tests/unit/objects/test_objects.py":[{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"864bbd8c402777be1b7af64bb54cc813367c844b","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":402,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":403,"context_line":"                 \u0027version\u0027: \u00271.4\u0027,"},{"line_number":404,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":405,"context_line":"        MyObj._from_db_object(obj, dbobj)"},{"line_number":406,"context_line":"        self.assertEqual(obj.VERSION, \u00271.4\u0027)"},{"line_number":407,"context_line":"        self.assertEqual(obj.foo, 123)"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_c172b710","line":404,"range":{"start_line":404,"start_character":44,"end_line":404,"end_character":60},"updated":"2017-01-17 19:31:46.000000000","message":"I don\u0027t quite understand what real scenario this is testing. If \"missing\" is not supposed to exist in 1.4, why would there ever be a 1.4 MyObj in the DB with the \"missing\" attribute not null?\n\nFrom https://bugs.launchpad.net/ironic/+bug/1585141, I thought that pinned services would still be saving objects as though the object was of version 1.4, so they wouldn\u0027t be saving it with a \"missing\" field","commit_id":"845ff5997d11116b22d4e8acb2dc68c64598ebf1"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"5af2d55dc5d2e282ca1aed225a04fd215a1c7587","unresolved":false,"context_lines":[{"line_number":401,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":402,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":403,"context_line":"                 \u0027version\u0027: \u00271.4\u0027,"},{"line_number":404,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":405,"context_line":"        MyObj._from_db_object(obj, dbobj)"},{"line_number":406,"context_line":"        self.assertEqual(obj.VERSION, \u00271.4\u0027)"},{"line_number":407,"context_line":"        self.assertEqual(obj.foo, 123)"}],"source_content_type":"text/x-python","patch_set":23,"id":"5a3905b3_29578203","line":404,"range":{"start_line":404,"start_character":44,"end_line":404,"end_character":60},"in_reply_to":"7a3c09a3_c172b710","updated":"2017-01-19 14:10:39.000000000","message":"I\u0027m checking here that the implementation by itself doesn\u0027t actually touch this field - this is the job for obj_make_compatible methods. I could test with None, but then obj.missing will always be None.","commit_id":"845ff5997d11116b22d4e8acb2dc68c64598ebf1"},{"author":{"_account_id":13295,"name":"Mario Villaplana","email":"mario.villaplana@gmail.com","username":"mariojv"},"change_message_id":"864bbd8c402777be1b7af64bb54cc813367c844b","unresolved":false,"context_lines":[{"line_number":420,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":421,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":422,"context_line":"                 \u0027version\u0027: \u00271.4\u0027,"},{"line_number":423,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":424,"context_line":"        MyObj._from_db_object(obj, dbobj)"},{"line_number":425,"context_line":"        self.assertEqual(obj.VERSION, \u00271.4\u0027)"},{"line_number":426,"context_line":"        self.assertEqual(obj.foo, 123)"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a3c09a3_01675f4d","line":423,"range":{"start_line":423,"start_character":44,"end_line":423,"end_character":62},"updated":"2017-01-17 19:31:46.000000000","message":"same question here","commit_id":"845ff5997d11116b22d4e8acb2dc68c64598ebf1"},{"author":{"_account_id":12356,"name":"Vladyslav Drok","email":"vdrok@mirantis.com","username":"vdrok"},"change_message_id":"1ba0afef9d4a69f2179472426d4ba417608886cb","unresolved":false,"context_lines":[{"line_number":42,"context_line":"              \u0027missing\u0027: fields.StringField(),"},{"line_number":43,"context_line":"              }"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":46,"context_line":"        super(MyObj, self).obj_make_compatible(primitive, target_version)"},{"line_number":47,"context_line":"        if target_version \u003d\u003d \u00271.4\u0027 and \u0027missing\u0027 in primitive:"},{"line_number":48,"context_line":"            del primitive[\u0027missing\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def obj_load_attr(self, attrname):"},{"line_number":51,"context_line":"        setattr(self, attrname, \u0027loaded!\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_b0daed76","line":48,"range":{"start_line":45,"start_character":4,"end_line":48,"end_character":36},"updated":"2017-01-19 16:55:48.000000000","message":"Maybe dumb question - this is going to be similar for other objects? Need to read some ovo docs on that.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":42,"context_line":"              \u0027missing\u0027: fields.StringField(),"},{"line_number":43,"context_line":"              }"},{"line_number":44,"context_line":""},{"line_number":45,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":46,"context_line":"        super(MyObj, self).obj_make_compatible(primitive, target_version)"},{"line_number":47,"context_line":"        if target_version \u003d\u003d \u00271.4\u0027 and \u0027missing\u0027 in primitive:"},{"line_number":48,"context_line":"            del primitive[\u0027missing\u0027]"},{"line_number":49,"context_line":""},{"line_number":50,"context_line":"    def obj_load_attr(self, attrname):"},{"line_number":51,"context_line":"        setattr(self, attrname, \u0027loaded!\u0027)"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_bc77ff9d","line":48,"range":{"start_line":45,"start_character":4,"end_line":48,"end_character":36},"in_reply_to":"5a3905b3_b0daed76","updated":"2017-01-20 09:31:54.000000000","message":"Yes. You could also convert versions to tuples to compare them. Dan Smith has a good writeup here: http://superuser.openstack.org/articles/upgrades-in-openstack-nova-objects/\n\nPlease also check the update to the dev documentation in the later patch, it should explain these things.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":6618,"name":"Ruby Loo","email":"opensrloo@gmail.com","username":"rloo"},"change_message_id":"96d58a11f3f0a22dd0ab08ee71370f40afedcc68","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                         set(TestSubclassedObject.fields.keys()))"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    def test_get_changes(self):"},{"line_number":333,"context_line":"        obj \u003d MyObj(self.context)"},{"line_number":334,"context_line":"        self.assertEqual({\u0027version\u0027: \u00271.5\u0027}, obj.obj_get_changes())"},{"line_number":335,"context_line":"        obj.foo \u003d 123"},{"line_number":336,"context_line":"        self.assertEqual({\u0027version\u0027: \u00271.5\u0027, \u0027foo\u0027: 123}, obj.obj_get_changes())"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_5320f39b","line":333,"updated":"2017-01-19 16:44:28.000000000","message":"I guess I don\u0027t understand ._unpinned. I looked at this obj._unpinned and it is False. Without looking at the code for ._unpinned, I would have thought it was True. Is it pinned?\n\nMaybe it would be good to have some small unit test(s) that just show what obj._unpinned is expected to be. It would help me.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":14250,"name":"Grzegorz Grasza","email":"xek@redhat.com","username":"xek"},"change_message_id":"16831dccbc04b902d60b208f43084fd7efcb3f09","unresolved":false,"context_lines":[{"line_number":330,"context_line":"                         set(TestSubclassedObject.fields.keys()))"},{"line_number":331,"context_line":""},{"line_number":332,"context_line":"    def test_get_changes(self):"},{"line_number":333,"context_line":"        obj \u003d MyObj(self.context)"},{"line_number":334,"context_line":"        self.assertEqual({\u0027version\u0027: \u00271.5\u0027}, obj.obj_get_changes())"},{"line_number":335,"context_line":"        obj.foo \u003d 123"},{"line_number":336,"context_line":"        self.assertEqual({\u0027version\u0027: \u00271.5\u0027, \u0027foo\u0027: 123}, obj.obj_get_changes())"}],"source_content_type":"text/x-python","patch_set":25,"id":"5a3905b3_59e8ed10","line":333,"in_reply_to":"5a3905b3_5320f39b","updated":"2017-01-20 09:31:54.000000000","message":"It is false, because a newly created object cannot be unpinned, since it should be sent via RPC or saved in the DB in the pinned version, specified in the pin_release_version config.\n\nOnly objects which were received (via RPC) or retrieved (from DB) in the newest version can be called \"unpinned\".\n\nI removed this attribute (as we previously discussed), and wrote some more tests for the _get_target_version() method.","commit_id":"bdd4d66db54a9c345fe66fc7913fe6ba621e57d5"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":387,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":388,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":389,"context_line":"                 \u0027version\u0027: \u00271.5\u0027,"},{"line_number":390,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":391,"context_line":"        MyObj._from_db_object(obj, dbobj)"},{"line_number":392,"context_line":"        self.assertEqual(obj.VERSION, \u00271.5\u0027)"},{"line_number":393,"context_line":"        self.assertEqual(obj.foo, 123)"}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_b1dfa138","line":390,"updated":"2017-02-08 18:40:48.000000000","message":"Can we define dbobj at the start of this file and reuse it across all the tests?","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":399,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":400,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":401,"context_line":"                 \u0027version\u0027: \u00271.4\u0027,"},{"line_number":402,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":403,"context_line":"        MyObj._from_db_object(obj, dbobj)"},{"line_number":404,"context_line":"        self.assertEqual(obj.VERSION, \u00271.4\u0027)"},{"line_number":405,"context_line":"        self.assertEqual(obj.foo, 123)"}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_11f155cd","line":402,"range":{"start_line":402,"start_character":23,"end_line":402,"end_character":24},"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":441,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":442,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":443,"context_line":"                 \u0027version\u0027: \u00271.3\u0027,"},{"line_number":444,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":445,"context_line":"        obj \u003d MyObj._from_db_object(MyObj(), dbobj)"},{"line_number":446,"context_line":"        self.assertEqual(\u00271.3\u0027, obj._get_target_version())"},{"line_number":447,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_91b38509","line":444,"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":457,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":458,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":459,"context_line":"                 \u0027version\u0027: \u00270.3\u0027,"},{"line_number":460,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":461,"context_line":"        self.assertRaises(object_exception.IncompatibleObjectVersion,"},{"line_number":462,"context_line":"                          MyObj._from_db_object, MyObj(), dbobj)"},{"line_number":463,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_d1d18d24","line":460,"range":{"start_line":460,"start_character":14,"end_line":460,"end_character":16},"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":480,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":481,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":482,"context_line":"                 \u0027version\u0027: \u00271.5\u0027,"},{"line_number":483,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":484,"context_line":"        obj \u003d MyObj._from_db_object(MyObj(), dbobj)"},{"line_number":485,"context_line":"        self.assertEqual(\u00271.5\u0027, obj._get_target_version())"},{"line_number":486,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_f17a0923","line":483,"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":511,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":512,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":513,"context_line":"                 \u0027version\u0027: \u00271.3\u0027,"},{"line_number":514,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":515,"context_line":"        obj \u003d MyObj._from_db_object(MyObj(), dbobj)"},{"line_number":516,"context_line":"        self.assertEqual(\u00271.3\u0027, obj._get_target_version())"},{"line_number":517,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_1176b53d","line":514,"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":543,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":544,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":545,"context_line":"                 \u0027version\u0027: \u00271.4\u0027,"},{"line_number":546,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":547,"context_line":"        MyObj._from_db_object(obj, dbobj)"},{"line_number":548,"context_line":"        self.assertEqual(obj.VERSION, \u00271.4\u0027)"},{"line_number":549,"context_line":"        self.assertEqual(obj.foo, 123)"}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_5192fd4c","line":546,"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":562,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":563,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":564,"context_line":"                 \u0027version\u0027: \u00271.5\u0027,"},{"line_number":565,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":566,"context_line":"        MyObj._from_db_object(obj, dbobj)"},{"line_number":567,"context_line":"        self.assertEqual(obj.VERSION, \u00271.5\u0027)"},{"line_number":568,"context_line":"        self.assertEqual(obj.foo, 123)"}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_71953931","line":565,"range":{"start_line":565,"start_character":25,"end_line":565,"end_character":26},"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":581,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":582,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":583,"context_line":"                 \u0027version\u0027: None,"},{"line_number":584,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":585,"context_line":"        MyObj._from_db_object(obj, dbobj)"},{"line_number":586,"context_line":"        self.assertEqual(obj.VERSION, \u00271.4\u0027)"},{"line_number":587,"context_line":"        self.assertEqual(obj.foo, 123)"}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_9198e56b","line":584,"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"},{"author":{"_account_id":11878,"name":"Rushil Chugh","email":"rushil.chugh@gmail.com","username":"rushil"},"change_message_id":"fc616cb570d4e10709e2abb7d38612ef1d0fcce2","unresolved":false,"context_lines":[{"line_number":593,"context_line":"        dbobj \u003d {\u0027created_at\u0027: timeutils.utcnow(),"},{"line_number":594,"context_line":"                 \u0027updated_at\u0027: timeutils.utcnow(),"},{"line_number":595,"context_line":"                 \u0027version\u0027: \u00271.6\u0027,"},{"line_number":596,"context_line":"                 \u0027foo\u0027: 123, \u0027bar\u0027: \u0027test\u0027, \u0027missing\u0027: \u0027test\u0027}"},{"line_number":597,"context_line":"        self.assertRaises(object_exception.IncompatibleObjectVersion,"},{"line_number":598,"context_line":"                          MyObj._from_db_object, obj, dbobj)"},{"line_number":599,"context_line":""}],"source_content_type":"text/x-python","patch_set":34,"id":"1a430d35_f1a12995","line":596,"updated":"2017-02-08 18:40:48.000000000","message":"Same comment as above","commit_id":"460554c255cf77bf312f3d10a2fa56cb3e9bd19e"}]}