)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"3e3eb599597ee69f183a29821c1b65e1eb2444cf","unresolved":false,"context_lines":[{"line_number":20,"context_line":"NOTE: I feel like what I\u0027ve done with _do_compute_node() and"},{"line_number":21,"context_line":"_from_db_object() is a bit of a hack, but it lets me avoid modifying"},{"line_number":22,"context_line":"many of the unit tests and I think it might help when upgrading."},{"line_number":23,"context_line":"Thoughts?"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Closes-Bug: #1420848"},{"line_number":26,"context_line":"Change-Id: Ied7d47363d0489bca3cf2c711217e1a3b7d24a03"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a80dd14_a0456b44","line":23,"updated":"2015-03-12 21:20:07.000000000","message":"when you say _do_compute_node,  I think it\u0027s \nobj_load_attr ? guess hack code to avoid UT issue is not a good way ...but not very experience on this area ....","commit_id":"4bfc5e89d061bdb7aba74f0160baa0af53ee6cc9"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"dcf70afd5d4f2691cc1688a0563bf0fe61b21067","unresolved":false,"context_lines":[{"line_number":20,"context_line":"NOTE: I feel like what I\u0027ve done with _do_compute_node() and"},{"line_number":21,"context_line":"_from_db_object() is a bit of a hack, but it lets me avoid modifying"},{"line_number":22,"context_line":"many of the unit tests and I think it might help when upgrading."},{"line_number":23,"context_line":"Thoughts?"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Closes-Bug: #1420848"},{"line_number":26,"context_line":"Change-Id: Ied7d47363d0489bca3cf2c711217e1a3b7d24a03"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"9a80dd14_a83f00b3","line":23,"in_reply_to":"9a80dd14_a0456b44","updated":"2015-03-13 06:43:09.000000000","message":"Yes, should be obj_load_attr.  Will fix that.\n\nI\u0027m wondering if it might be useful in order to handle calling _from_db_object() on data coming from an older node that doesn\u0027t have the \"reported_at\" field.  Could be totally useless though, I\u0027m clearly no expert either.\n\nThe fact that without that a whole bunch of unit tests blew up made me wonder whether it might have implications for upgrading...would be nice if someone with more experience could chime in.","commit_id":"4bfc5e89d061bdb7aba74f0160baa0af53ee6cc9"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"48d251fe5891bae9b7292b5a0521483085585801","unresolved":false,"context_lines":[{"line_number":11,"context_line":"field to be updated by setting the service as \"down\" or \"up\", thus"},{"line_number":12,"context_line":"potentially causing a service that is unavailable to be detected"},{"line_number":13,"context_line":"as \"up\".  This could result in the scheduler trying to assign"},{"line_number":14,"context_line":"instances to an unavailable compute node."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The fix is to add a new field to explicitly track when the service"},{"line_number":17,"context_line":"sends in a status report and use that if available when testing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a80dd14_bab534ee","line":14,"updated":"2015-03-16 10:12:53.000000000","message":"The scheduler is only taking a list of nodes given by what it has on the compute_nodes table and checks each by calling the ServiceGroup API on the service_is_up() method.\n\nDepending on which SG driver you\u0027re using, it can either check the DB service info by indeed looking at updated_at or can also call other drivers (Zookeeper and Memcached at the moment) for knowing if it\u0027s up.\n\nThat way, if using ZK or MC, it doesn\u0027t check the service info... So I would prefer to you work on the ServiceGroup DB driver to fix the bug instead of more than necessary IMHO.\n\nI agree that it could need to modify the disabling possibility to either *not* update the updated_at field, or create another DB field called \"reported_at\" but which would be *persisted*","commit_id":"33d3d0845360796db13c5052569834d130dc0243"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"6934a6d041f1c84e84dfbb153c58ac7d1957a52e","unresolved":false,"context_lines":[{"line_number":11,"context_line":"field to be updated by setting the service as \"down\" or \"up\", thus"},{"line_number":12,"context_line":"potentially causing a service that is unavailable to be detected"},{"line_number":13,"context_line":"as \"up\".  This could result in the scheduler trying to assign"},{"line_number":14,"context_line":"instances to an unavailable compute node."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The fix is to add a new field to explicitly track when the service"},{"line_number":17,"context_line":"sends in a status report and use that if available when testing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a80dd14_95865fda","line":14,"in_reply_to":"9a80dd14_1a2a9c90","updated":"2015-03-16 22:13:00.000000000","message":"I don\u0027t think that\u0027s sufficient.  Suppose the service stops reporting state.  A minute later it will be considered \"down\" if using the DB backend.  If I then set it to \"disabled\" that will update the \"updated_at\" field.  If we then go to enable the service the DB backend sees that the \"updated_at\" field has been recently updated, so it thinks the service is up.\n\nWhat we actually care about is the last time the service called _report_state(), not the last time the row in the \"services\" table was changed.","commit_id":"33d3d0845360796db13c5052569834d130dc0243"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"25a78ecfb9da1262c09a03718bab53e5e773308e","unresolved":false,"context_lines":[{"line_number":11,"context_line":"field to be updated by setting the service as \"down\" or \"up\", thus"},{"line_number":12,"context_line":"potentially causing a service that is unavailable to be detected"},{"line_number":13,"context_line":"as \"up\".  This could result in the scheduler trying to assign"},{"line_number":14,"context_line":"instances to an unavailable compute node."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The fix is to add a new field to explicitly track when the service"},{"line_number":17,"context_line":"sends in a status report and use that if available when testing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a80dd14_1a2a9c90","line":14,"in_reply_to":"9a80dd14_4de62db2","updated":"2015-03-16 21:56:08.000000000","message":"Just to be clear, I\u0027m not sure we need to add a new field for reporting how long the service is updated. Instead, we should make sure that when enabling back a service, it would be calling the SG API to see if it\u0027s still up or no.\n\nIf yes, it doesn\u0027t matter if updated_at is updated, because we already did an healthcheck at this time which will be considered later on.\nIf no, we can safely consider to warn the operator that while he wants to enabling back the service, it\u0027s still not up, and we could also change back the updated_at field to the previous value we had (before enabling back the service, which was basically the time when the operator disabled the service)","commit_id":"33d3d0845360796db13c5052569834d130dc0243"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"11ee3593504c73b6599561bb62731a556a63fe91","unresolved":false,"context_lines":[{"line_number":11,"context_line":"field to be updated by setting the service as \"down\" or \"up\", thus"},{"line_number":12,"context_line":"potentially causing a service that is unavailable to be detected"},{"line_number":13,"context_line":"as \"up\".  This could result in the scheduler trying to assign"},{"line_number":14,"context_line":"instances to an unavailable compute node."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The fix is to add a new field to explicitly track when the service"},{"line_number":17,"context_line":"sends in a status report and use that if available when testing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a80dd14_41d5e3b7","line":14,"in_reply_to":"9a80dd14_95865fda","updated":"2015-03-17 16:38:00.000000000","message":"I think we could minimize the nova Service object changes by adding a new field to the DB schema but not to the nova Service object.\n\nIf we were to go that route the only practical option I see would be to load the \"reported_at\" DB value in the nova Service \"updated_at\" field.  Otherwise we\u0027d need to do additional DB accesses in servicegroup.drivers.db.DbDriver.is_up(), and that gets complicated if we don\u0027t have local DB access.","commit_id":"33d3d0845360796db13c5052569834d130dc0243"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"5468ffff9266fac7f1f2c798f0c60f6b45c3da27","unresolved":false,"context_lines":[{"line_number":11,"context_line":"field to be updated by setting the service as \"down\" or \"up\", thus"},{"line_number":12,"context_line":"potentially causing a service that is unavailable to be detected"},{"line_number":13,"context_line":"as \"up\".  This could result in the scheduler trying to assign"},{"line_number":14,"context_line":"instances to an unavailable compute node."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The fix is to add a new field to explicitly track when the service"},{"line_number":17,"context_line":"sends in a status report and use that if available when testing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a80dd14_4de62db2","line":14,"in_reply_to":"9a80dd14_ada969df","updated":"2015-03-16 17:53:08.000000000","message":"Never mind, I was getting confused between the servicegroup db backend and the Service object in the database.\n\nIt might be possible to avoid changing the Service object. The problem is that service_is_up() is often passed a Service object, so either I need to include the \"reported_at\" field in that object or else do another DB query to retrieve the \"reported_at\" field.  Thoughts?\n\nIs it possible to have fields defined in the sqlalchemy Service model that aren\u0027t in the Service object?\n\nI think I\u0027m still going to need a way to distinguish between calls to _report_state() and other calls to service_update().","commit_id":"33d3d0845360796db13c5052569834d130dc0243"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"37d9bffd2e8e3e6658707b05164246007d371bff","unresolved":false,"context_lines":[{"line_number":11,"context_line":"field to be updated by setting the service as \"down\" or \"up\", thus"},{"line_number":12,"context_line":"potentially causing a service that is unavailable to be detected"},{"line_number":13,"context_line":"as \"up\".  This could result in the scheduler trying to assign"},{"line_number":14,"context_line":"instances to an unavailable compute node."},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"The fix is to add a new field to explicitly track when the service"},{"line_number":17,"context_line":"sends in a status report and use that if available when testing"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"9a80dd14_ada969df","line":14,"in_reply_to":"9a80dd14_bab534ee","updated":"2015-03-16 17:30:09.000000000","message":"Could you elaborate a bit on where you think I\u0027m modifying more than necessary?  I think all of my changes should only affect the DB driver, not ZK or MC.\n\nI should perhaps update the commit comment to indicate that this is all specific to the ServiceGroup DB driver.","commit_id":"33d3d0845360796db13c5052569834d130dc0243"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1fd5f92f9784554864fdfc8834d9bc8d969b2b3e","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently we use the auto-updated \"updated_at\" field to determine"},{"line_number":10,"context_line":"whether a service is \"up\".  An end-user can cause the \"updated_at\""},{"line_number":11,"context_line":"field to be updated by setting the service as \"down\" or \"up\", thus"},{"line_number":12,"context_line":"potentially causing a service that is unavailable to be detected"},{"line_number":13,"context_line":"as \"up\".  This could result in the scheduler trying to assign"},{"line_number":14,"context_line":"instances to an unavailable compute node, or in the system"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a016987_52fa9076","line":11,"updated":"2015-05-16 22:50:04.000000000","message":"A user cannot set a service as \"down\" or \"up\". They can only set a service as \"disabled\" or \"enabled\". The \"down\" or \"up\" state is determined by the servicegroup API by, as you say, checking the updated_at state.\n\nSo, really, it would be good to change the wording in this commit message to be a bit more accurate.","commit_id":"3a40df87fd7428942364a1f3073abc55c9763ae5"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"9d7033a17ec3320de3077696cf5ae121c452b315","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently we use the auto-updated \"updated_at\" field to determine"},{"line_number":10,"context_line":"whether a service is \"up\".  An end-user can cause the \"updated_at\""},{"line_number":11,"context_line":"field to be updated by setting the service as \"down\" or \"up\", thus"},{"line_number":12,"context_line":"potentially causing a service that is unavailable to be detected"},{"line_number":13,"context_line":"as \"up\".  This could result in the scheduler trying to assign"},{"line_number":14,"context_line":"instances to an unavailable compute node, or in the system"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a016987_1d6bb5f2","line":11,"in_reply_to":"7a016987_52fa9076","updated":"2015-05-21 07:37:13.000000000","message":"Good catch.  Fixed.","commit_id":"3a40df87fd7428942364a1f3073abc55c9763ae5"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1fd5f92f9784554864fdfc8834d9bc8d969b2b3e","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This does mean that \"nova service-list\" could show the service as"},{"line_number":22,"context_line":"\"down\" even if it shows a recent \"updated_at\", but that could"},{"line_number":23,"context_line":"happen for the other backends already."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Closes-Bug: #1420848"},{"line_number":26,"context_line":"Change-Id: Ied7d47363d0489bca3cf2c711217e1a3b7d24a03"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a016987_12f41884","line":23,"updated":"2015-05-16 22:50:04.000000000","message":"Please add a DocImpact line with a brief explanation of how this patch changes Nova\u0027s behaviour.","commit_id":"3a40df87fd7428942364a1f3073abc55c9763ae5"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"9d7033a17ec3320de3077696cf5ae121c452b315","unresolved":false,"context_lines":[{"line_number":20,"context_line":""},{"line_number":21,"context_line":"This does mean that \"nova service-list\" could show the service as"},{"line_number":22,"context_line":"\"down\" even if it shows a recent \"updated_at\", but that could"},{"line_number":23,"context_line":"happen for the other backends already."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"Closes-Bug: #1420848"},{"line_number":26,"context_line":"Change-Id: Ied7d47363d0489bca3cf2c711217e1a3b7d24a03"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":23,"id":"7a016987_fd6fb9f9","line":23,"in_reply_to":"7a016987_12f41884","updated":"2015-05-21 07:37:13.000000000","message":"Done","commit_id":"3a40df87fd7428942364a1f3073abc55c9763ae5"}],"nova/api/openstack/api_version_request.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"3e3eb599597ee69f183a29821c1b65e1eb2444cf","unresolved":false,"context_lines":[{"line_number":43,"context_line":"    * 2.3 - Exposes additional os-extended-server-attributes"},{"line_number":44,"context_line":"            Exposes delete_on_termination for os-extended-volumes"},{"line_number":45,"context_line":"    * 2.4 - Exposes additional \"reported_at\" attribute to os-services to"},{"line_number":46,"context_line":"            give a more accurate view of service status."},{"line_number":47,"context_line":"\"\"\""},{"line_number":48,"context_line":""},{"line_number":49,"context_line":"# The minimum and maximum versions of the API supported"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_6cc52f5b","line":46,"updated":"2015-03-12 21:20:07.000000000","message":"only FYI, I saw someone used this 2.4\nmaybe you need rebase later :)","commit_id":"4bfc5e89d061bdb7aba74f0160baa0af53ee6cc9"}],"nova/api/openstack/compute/contrib/services.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"d095686c5ef28e66a99651ff17dd6187fde04fe3","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from nova import servicegroup"},{"line_number":23,"context_line":"from nova import utils"},{"line_number":24,"context_line":"from oslo_log import log as logging"},{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":26,"context_line":"authorize \u003d extensions.extension_authorizer(\u0027compute\u0027, \u0027services\u0027)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_765e3514","line":25,"updated":"2015-03-11 10:46:51.000000000","message":"Where it used?","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"74d5a02ca75f16080a9af68241cea0a452ebca65","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from nova import servicegroup"},{"line_number":23,"context_line":"from nova import utils"},{"line_number":24,"context_line":"from oslo_log import log as logging"},{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":26,"context_line":"authorize \u003d extensions.extension_authorizer(\u0027compute\u0027, \u0027services\u0027)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_b7197d7b","line":25,"in_reply_to":"9a80dd14_64959a87","updated":"2015-03-11 15:13:52.000000000","message":"Yes, indeed. :)  I\u0027ll remove it in patch set 4.","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"65b0303f6385d39ba8536a5034a790740784aa9f","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from nova import servicegroup"},{"line_number":23,"context_line":"from nova import utils"},{"line_number":24,"context_line":"from oslo_log import log as logging"},{"line_number":25,"context_line":"LOG \u003d logging.getLogger(__name__)"},{"line_number":26,"context_line":"authorize \u003d extensions.extension_authorizer(\u0027compute\u0027, \u0027services\u0027)"},{"line_number":27,"context_line":""},{"line_number":28,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_64959a87","line":25,"in_reply_to":"9a80dd14_765e3514","updated":"2015-03-11 13:08:27.000000000","message":"guess for debug purpose and forgot to remove :)","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"65b0303f6385d39ba8536a5034a790740784aa9f","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        if self.ext_mgr.is_loaded(\u0027os-extended-services-delete\u0027):"},{"line_number":66,"context_line":"            service_detail[\u0027id\u0027] \u003d svc[\u0027id\u0027]"},{"line_number":67,"context_line":"        if svc.get(\u0027reported_at\u0027):"},{"line_number":68,"context_line":"            service_detail[\u0027reported_at\u0027] \u003d svc[\u0027reported_at\u0027]"},{"line_number":69,"context_line":"        if detailed:"},{"line_number":70,"context_line":"            service_detail[\u0027disabled_reason\u0027] \u003d svc[\u0027disabled_reason\u0027]"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_e42b4a31","line":68,"updated":"2015-03-11 13:08:27.000000000","message":"I am not sure adding more field can be done like this at V2 API\n\nhttps://wiki.openstack.org/wiki/APIChangeGuidelines#Generally_Considered_OK","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"74d5a02ca75f16080a9af68241cea0a452ebca65","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        if self.ext_mgr.is_loaded(\u0027os-extended-services-delete\u0027):"},{"line_number":66,"context_line":"            service_detail[\u0027id\u0027] \u003d svc[\u0027id\u0027]"},{"line_number":67,"context_line":"        if svc.get(\u0027reported_at\u0027):"},{"line_number":68,"context_line":"            service_detail[\u0027reported_at\u0027] \u003d svc[\u0027reported_at\u0027]"},{"line_number":69,"context_line":"        if detailed:"},{"line_number":70,"context_line":"            service_detail[\u0027disabled_reason\u0027] \u003d svc[\u0027disabled_reason\u0027]"},{"line_number":71,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_5724c1c5","line":68,"in_reply_to":"9a80dd14_e42b4a31","updated":"2015-03-11 15:13:52.000000000","message":"Thanks for that link.  Based on that, am I right that it would count as \"Adding a property to a resource representation\" and so would need to be done as an API extension?\n\nI had considered returning the \"reported_at\" timestamp in the \"updated_at\" field (since it logically means the last time it was updated intentionally) but I thought that might be confusing.","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"}],"nova/api/openstack/rest_api_version_history.rst":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"3e3eb599597ee69f183a29821c1b65e1eb2444cf","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"  Exposes additional \"reported_at\" attribute to os-services to give a more"},{"line_number":53,"context_line":"  accurate view of service status."},{"line_number":54,"context_line":"  "},{"line_number":55,"context_line":"  The \"updated_at\" attribute can be spuriously updated by the end-user doing"},{"line_number":56,"context_line":"  a \"service disable\" followed by a \"service enable\".  The \"reported_at\""},{"line_number":57,"context_line":"  attribute shows the last time the service explicitly reported its state,"}],"source_content_type":"text/x-rst","patch_set":6,"id":"9a80dd14_4ca14b93","line":54,"updated":"2015-03-12 21:20:07.000000000","message":"blanks :)","commit_id":"4bfc5e89d061bdb7aba74f0160baa0af53ee6cc9"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"dcf70afd5d4f2691cc1688a0563bf0fe61b21067","unresolved":false,"context_lines":[{"line_number":51,"context_line":""},{"line_number":52,"context_line":"  Exposes additional \"reported_at\" attribute to os-services to give a more"},{"line_number":53,"context_line":"  accurate view of service status."},{"line_number":54,"context_line":"  "},{"line_number":55,"context_line":"  The \"updated_at\" attribute can be spuriously updated by the end-user doing"},{"line_number":56,"context_line":"  a \"service disable\" followed by a \"service enable\".  The \"reported_at\""},{"line_number":57,"context_line":"  attribute shows the last time the service explicitly reported its state,"}],"source_content_type":"text/x-rst","patch_set":6,"id":"9a80dd14_c85fd40d","line":54,"in_reply_to":"9a80dd14_4ca14b93","updated":"2015-03-13 06:43:09.000000000","message":"will fix","commit_id":"4bfc5e89d061bdb7aba74f0160baa0af53ee6cc9"}],"nova/conductor/manager.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"16a835ef765582f822244dd749080d5d56308a6f","unresolved":false,"context_lines":[{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    @messaging.expected_exceptions(exception.ServiceNotFound)"},{"line_number":334,"context_line":"    def service_update(self, context, service, values):"},{"line_number":335,"context_line":"        # If the sender included a report timestamp then override"},{"line_number":336,"context_line":"        # it with ours (to avoid inter-host time mismatch issues)."},{"line_number":337,"context_line":"        if values and values.get(\u0027reported_at\u0027):"},{"line_number":338,"context_line":"            values[\u0027reported_at\u0027] \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa963961_08ff8b02","line":335,"updated":"2015-03-26 14:06:22.000000000","message":"This makes sense even if looks a hack.\n\nWe should raise an exception if sender try to update \"reported_at\" from this method and create a specific service to update that value.\n\n  def service_update_report(self, ...):\n    ...","commit_id":"a2170354153ddf01fd0d448becec99154886833c"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"3253769ebfa09d2e1078c41d7b201f5526bf7fe5","unresolved":false,"context_lines":[{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    @messaging.expected_exceptions(exception.ServiceNotFound)"},{"line_number":334,"context_line":"    def service_update(self, context, service, values):"},{"line_number":335,"context_line":"        # If the sender included a report timestamp then override"},{"line_number":336,"context_line":"        # it with ours (to avoid inter-host time mismatch issues)."},{"line_number":337,"context_line":"        if values and values.get(\u0027reported_at\u0027):"},{"line_number":338,"context_line":"            values[\u0027reported_at\u0027] \u003d timeutils.utcnow()"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa963961_d7fdbe6f","line":335,"in_reply_to":"fa963961_08ff8b02","updated":"2015-03-26 16:10:10.000000000","message":"Okay, will do.","commit_id":"a2170354153ddf01fd0d448becec99154886833c"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c1b602dbd42d9eb66b7381be1fb401a54a0638ef","unresolved":false,"context_lines":[{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    @messaging.expected_exceptions(exception.ServiceNotFound)"},{"line_number":334,"context_line":"    def service_update(self, context, service, values):"},{"line_number":335,"context_line":"        svc \u003d self.db.service_update(context, service[\u0027id\u0027], values)"},{"line_number":336,"context_line":"        return jsonutils.to_primitive(svc)"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"    def task_log_get(self, context, task_name, begin, end, host, state):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa963961_a1609c36","side":"PARENT","line":335,"updated":"2015-03-27 07:51:05.000000000","message":"We can probably raise an exception here if we try to update \"last_seen_up\" without to pass by \"service_report\"","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"542c7e01ce7ddf03d7bd2daa9d16f38086b21b7f","unresolved":false,"context_lines":[{"line_number":332,"context_line":""},{"line_number":333,"context_line":"    @messaging.expected_exceptions(exception.ServiceNotFound)"},{"line_number":334,"context_line":"    def service_update(self, context, service, values):"},{"line_number":335,"context_line":"        svc \u003d self.db.service_update(context, service[\u0027id\u0027], values)"},{"line_number":336,"context_line":"        return jsonutils.to_primitive(svc)"},{"line_number":337,"context_line":""},{"line_number":338,"context_line":"    def task_log_get(self, context, task_name, begin, end, host, state):"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa963961_8a74849c","side":"PARENT","line":335,"in_reply_to":"fa963961_a1609c36","updated":"2015-03-27 14:42:46.000000000","message":"Right, you had suggested that before.  Will add.","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"8a71923b6ae77788229b0a454c30499e61fe9ca9","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        # \u0027report_count\u0027, so if that value changes then store the timestamp"},{"line_number":558,"context_line":"        # as the last time we got a state report."},{"line_number":559,"context_line":"        if \u0027report_count\u0027 in values:"},{"line_number":560,"context_line":"            if service_ref.report_count !\u003d values[\u0027report_count\u0027]:"},{"line_number":561,"context_line":"                service_ref.last_seen_up \u003d timeutils.utcnow()"},{"line_number":562,"context_line":"        service_ref.update(values)"},{"line_number":563,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"7aaa499b_f8ad2a32","line":560,"updated":"2015-04-13 14:41:58.000000000","message":"just nit:\n\n  if service_ref.report_count \u003c values[\u0027report_count\u0027]:","commit_id":"3c9cf06ff6508e30bb3a3dceba58d9964a2f3b41"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"091fa4bc04a60f57c944d8968b5ab56bf5fed987","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        # \u0027report_count\u0027, so if that value changes then store the timestamp"},{"line_number":558,"context_line":"        # as the last time we got a state report."},{"line_number":559,"context_line":"        if \u0027report_count\u0027 in values:"},{"line_number":560,"context_line":"            if values[\u0027report_count\u0027] \u003e service_ref.report_count:"},{"line_number":561,"context_line":"                service_ref.last_seen_up \u003d timeutils.utcnow()"},{"line_number":562,"context_line":"        service_ref.update(values)"},{"line_number":563,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7aaa499b_ae5f580d","line":560,"updated":"2015-04-13 15:10:23.000000000","message":"That sounds a littly hacky to know if service_update was called by the SG DB driver IMHO, as the semantics wasn\u0027t so clear. Imagine that someone else updates the report_count field elsewhere (like in the API) and then your beauty logic is just false...\n\nCouldn\u0027t we make it more explicit ?  I wonder if it wouldn\u0027t be better to set another method for explicitely updating report_count that would be called by SG and drop the value here.","commit_id":"86785819c175e7094ebf310a63bc38c335b31435"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"e1c88b8f736b903dde4a3dd40e041b68bebc5e83","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        # \u0027report_count\u0027, so if that value changes then store the timestamp"},{"line_number":558,"context_line":"        # as the last time we got a state report."},{"line_number":559,"context_line":"        if \u0027report_count\u0027 in values:"},{"line_number":560,"context_line":"            if values[\u0027report_count\u0027] \u003e service_ref.report_count:"},{"line_number":561,"context_line":"                service_ref.last_seen_up \u003d timeutils.utcnow()"},{"line_number":562,"context_line":"        service_ref.update(values)"},{"line_number":563,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7aaa499b_70b00e44","line":560,"in_reply_to":"7aaa499b_8e2b7ceb","updated":"2015-04-13 15:30:22.000000000","message":"There is a big difference in between what was proposed in the PS you mentioned and what\u0027s here. Here, we\u0027re praising that anybody will use the report_count field, or it will false the last_seen_up change. Imagine that for some reason, we add the possibility in the API to update the report_count field (because of a miss or because of a side effect), then any user could change the behaviour.\n\nI would be in favor of providing a clear separation at the object level, not by extending the conductor or the method signature.","commit_id":"86785819c175e7094ebf310a63bc38c335b31435"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"e36bd42c0a0bed371cb684c6901b21c99ba011f9","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        # \u0027report_count\u0027, so if that value changes then store the timestamp"},{"line_number":558,"context_line":"        # as the last time we got a state report."},{"line_number":559,"context_line":"        if \u0027report_count\u0027 in values:"},{"line_number":560,"context_line":"            if values[\u0027report_count\u0027] \u003e service_ref.report_count:"},{"line_number":561,"context_line":"                service_ref.last_seen_up \u003d timeutils.utcnow()"},{"line_number":562,"context_line":"        service_ref.update(values)"},{"line_number":563,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7aaa499b_1032eaf1","line":560,"in_reply_to":"7aaa499b_ae5f580d","updated":"2015-04-13 15:48:42.000000000","message":"As of patch set 12 I had added a separate service_report() routine.  Hans suggested this check instead.\n\nMaybe if/when https://review.openstack.org/#/c/133663/ goes in we could make something like service_report() a remotable object method.\n\nI do like the fact that the last_seen_up timestamp is set by the DB, since that avoids any issues with time mismatch between nodes.","commit_id":"86785819c175e7094ebf310a63bc38c335b31435"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"93d3ed4c045adb4c1409df49196f2ffb4e3a3ad8","unresolved":false,"context_lines":[{"line_number":557,"context_line":"        # \u0027report_count\u0027, so if that value changes then store the timestamp"},{"line_number":558,"context_line":"        # as the last time we got a state report."},{"line_number":559,"context_line":"        if \u0027report_count\u0027 in values:"},{"line_number":560,"context_line":"            if values[\u0027report_count\u0027] \u003e service_ref.report_count:"},{"line_number":561,"context_line":"                service_ref.last_seen_up \u003d timeutils.utcnow()"},{"line_number":562,"context_line":"        service_ref.update(values)"},{"line_number":563,"context_line":""}],"source_content_type":"text/x-python","patch_set":18,"id":"7aaa499b_8e2b7ceb","line":560,"in_reply_to":"7aaa499b_ae5f580d","updated":"2015-04-13 15:15:40.000000000","message":"Was proposed here but there are some complain. https://review.openstack.org/#/c/163060/11/nova/conductor/manager.py,cm","commit_id":"86785819c175e7094ebf310a63bc38c335b31435"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"1fd5f92f9784554864fdfc8834d9bc8d969b2b3e","unresolved":false,"context_lines":[{"line_number":539,"context_line":"        # as the last time we got a state report."},{"line_number":540,"context_line":"        if \u0027report_count\u0027 in values:"},{"line_number":541,"context_line":"            if values[\u0027report_count\u0027] \u003e service_ref.report_count:"},{"line_number":542,"context_line":"                service_ref.last_seen_up \u003d timeutils.utcnow()"},{"line_number":543,"context_line":"        service_ref.update(values)"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"    return service_ref"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_b22f84f5","line":542,"updated":"2015-05-16 22:50:04.000000000","message":"Instead of relying on the report_count thing, why not just check to see if \u0027disabled\u0027 is in the values dict? If it is, then we know it\u0027s a user, not the heartbeat mechanism, that is executing the DB update.","commit_id":"3a40df87fd7428942364a1f3073abc55c9763ae5"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"224407d5826167cdd02feb1ba1409da0be0edd78","unresolved":false,"context_lines":[{"line_number":539,"context_line":"        # as the last time we got a state report."},{"line_number":540,"context_line":"        if \u0027report_count\u0027 in values:"},{"line_number":541,"context_line":"            if values[\u0027report_count\u0027] \u003e service_ref.report_count:"},{"line_number":542,"context_line":"                service_ref.last_seen_up \u003d timeutils.utcnow()"},{"line_number":543,"context_line":"        service_ref.update(values)"},{"line_number":544,"context_line":""},{"line_number":545,"context_line":"    return service_ref"}],"source_content_type":"text/x-python","patch_set":23,"id":"7a016987_3b65afe0","line":542,"in_reply_to":"7a016987_b22f84f5","updated":"2015-05-17 06:07:18.000000000","message":"That would work now, but would it still work if we add a \"report service down\" message (like from https://review.openstack.org/#/c/169836/) ?  I could see us just setting the service to \"forced down\" or something without touching the \"disabled\" state.\n\nI\u0027m fine either way, just want to make sure we don\u0027t paint ourselves into a corner.","commit_id":"3a40df87fd7428942364a1f3073abc55c9763ae5"}],"nova/db/sqlalchemy/migrate_repo/versions/278_add_service_heartbeat.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"3ee90dc19485d8cd08b5b01a2d285c03fb6dee77","unresolved":false,"context_lines":[{"line_number":10,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"# under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from oslo_log import log as logging"},{"line_number":14,"context_line":"from sqlalchemy import MetaData, Table, Column, DateTime"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"BASE_TABLE_NAME \u003d \u0027services\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_7b8ffecc","line":13,"updated":"2015-03-11 10:38:03.000000000","message":"F401 \u0027logging\u0027 imported but unused","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"74d5a02ca75f16080a9af68241cea0a452ebca65","unresolved":false,"context_lines":[{"line_number":10,"context_line":"# License for the specific language governing permissions and limitations"},{"line_number":11,"context_line":"# under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"from oslo_log import log as logging"},{"line_number":14,"context_line":"from sqlalchemy import MetaData, Table, Column, DateTime"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"BASE_TABLE_NAME \u003d \u0027services\u0027"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_32259bab","line":13,"in_reply_to":"9a80dd14_7b8ffecc","updated":"2015-03-11 15:13:52.000000000","message":"fixed","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"3ee90dc19485d8cd08b5b01a2d285c03fb6dee77","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        if hasattr(table.c, NEW_COLUMN_NAME):"},{"line_number":38,"context_line":"            getattr(table.c, NEW_COLUMN_NAME).drop()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_dbb54a7f","line":40,"updated":"2015-03-11 10:38:03.000000000","message":"W391 blank line at end of file","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"74d5a02ca75f16080a9af68241cea0a452ebca65","unresolved":false,"context_lines":[{"line_number":37,"context_line":"        if hasattr(table.c, NEW_COLUMN_NAME):"},{"line_number":38,"context_line":"            getattr(table.c, NEW_COLUMN_NAME).drop()"},{"line_number":39,"context_line":""},{"line_number":40,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_723633e9","line":40,"in_reply_to":"9a80dd14_dbb54a7f","updated":"2015-03-11 15:13:52.000000000","message":"fixed","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"}],"nova/objects/service.py":[{"author":{"_account_id":6062,"name":"jichenjc","email":"jichenjc@cn.ibm.com","username":"jichenjc"},"change_message_id":"3e3eb599597ee69f183a29821c1b65e1eb2444cf","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    # Version 1.7: Service version 1.9"},{"line_number":191,"context_line":"    # Version 1.8: Service version 1.10"},{"line_number":192,"context_line":"    # Version 1.9: Added get_by_binary() and Service version 1.11"},{"line_number":193,"context_line":"    # Version 1.10: Service version 1.11"},{"line_number":194,"context_line":"    VERSION \u003d \u00271.10\u0027"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_0c01136a","line":193,"updated":"2015-03-12 21:20:07.000000000","message":"is 1.11 -\u003e 1.12?","commit_id":"4bfc5e89d061bdb7aba74f0160baa0af53ee6cc9"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"dcf70afd5d4f2691cc1688a0563bf0fe61b21067","unresolved":false,"context_lines":[{"line_number":190,"context_line":"    # Version 1.7: Service version 1.9"},{"line_number":191,"context_line":"    # Version 1.8: Service version 1.10"},{"line_number":192,"context_line":"    # Version 1.9: Added get_by_binary() and Service version 1.11"},{"line_number":193,"context_line":"    # Version 1.10: Service version 1.11"},{"line_number":194,"context_line":"    VERSION \u003d \u00271.10\u0027"},{"line_number":195,"context_line":""},{"line_number":196,"context_line":"    fields \u003d {"}],"source_content_type":"text/x-python","patch_set":6,"id":"9a80dd14_0839eca3","line":193,"in_reply_to":"9a80dd14_0c01136a","updated":"2015-03-13 06:43:09.000000000","message":"will fix","commit_id":"4bfc5e89d061bdb7aba74f0160baa0af53ee6cc9"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"16a835ef765582f822244dd749080d5d56308a6f","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        \u0027disabled_reason\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":55,"context_line":"        \u0027availability_zone\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":56,"context_line":"        \u0027compute_node\u0027: fields.ObjectField(\u0027ComputeNode\u0027),"},{"line_number":57,"context_line":"        \u0027reported_at\u0027: fields.DateTimeField(nullable\u003dTrue),"},{"line_number":58,"context_line":"        }"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    obj_relationships \u003d {"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa963961_e5081094","line":57,"updated":"2015-03-26 14:06:22.000000000","message":"s/reported_at/last_seen_up ?","commit_id":"a2170354153ddf01fd0d448becec99154886833c"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"3253769ebfa09d2e1078c41d7b201f5526bf7fe5","unresolved":false,"context_lines":[{"line_number":54,"context_line":"        \u0027disabled_reason\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":55,"context_line":"        \u0027availability_zone\u0027: fields.StringField(nullable\u003dTrue),"},{"line_number":56,"context_line":"        \u0027compute_node\u0027: fields.ObjectField(\u0027ComputeNode\u0027),"},{"line_number":57,"context_line":"        \u0027reported_at\u0027: fields.DateTimeField(nullable\u003dTrue),"},{"line_number":58,"context_line":"        }"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    obj_relationships \u003d {"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa963961_1779e6fc","line":57,"in_reply_to":"fa963961_e5081094","updated":"2015-03-26 16:10:10.000000000","message":"Sure, can do.","commit_id":"a2170354153ddf01fd0d448becec99154886833c"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"4da742e11dc8477eb83a6f7317afa1bfbffb0b9b","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    }"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":66,"context_line":"        _target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":67,"context_line":"        if _target_version \u003c (1, 10):"},{"line_number":68,"context_line":"            target_compute_version \u003d self.obj_calculate_child_version("},{"line_number":69,"context_line":"                target_version, \u0027compute_node\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_1607771d","line":66,"updated":"2015-03-27 19:04:53.000000000","message":"Need an entry here for backwards compat.\n\n  if _target_version \u003c (1, 12):\n      del primitive[\u0027last_seen_up\u0027]","commit_id":"e7a2d29f658ca101e4abf70b047dba3b1b404713"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"f3bc3a354ce7f86b708cdb2c26e3056d9ebfa107","unresolved":false,"context_lines":[{"line_number":63,"context_line":"    }"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":66,"context_line":"        _target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":67,"context_line":"        if _target_version \u003c (1, 10):"},{"line_number":68,"context_line":"            target_compute_version \u003d self.obj_calculate_child_version("},{"line_number":69,"context_line":"                target_version, \u0027compute_node\u0027)"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_0c7886f1","line":66,"in_reply_to":"fa963961_1607771d","updated":"2015-03-27 19:59:15.000000000","message":"Okay.","commit_id":"e7a2d29f658ca101e4abf70b047dba3b1b404713"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"4da742e11dc8477eb83a6f7317afa1bfbffb0b9b","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    @staticmethod"},{"line_number":89,"context_line":"    def _from_db_object(context, service, db_service):"},{"line_number":90,"context_line":"        allow_missing \u003d (\u0027availability_zone\u0027, \u0027last_seen_up\u0027)"},{"line_number":91,"context_line":"        for key in service.fields:"},{"line_number":92,"context_line":"            if key in allow_missing and key not in db_service:"},{"line_number":93,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_960ca795","line":90,"updated":"2015-03-27 19:04:53.000000000","message":"This shouldn\u0027t be needed. With the new db column in place, every service query will return a value or None for this, which should be sufficient.","commit_id":"e7a2d29f658ca101e4abf70b047dba3b1b404713"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"f3bc3a354ce7f86b708cdb2c26e3056d9ebfa107","unresolved":false,"context_lines":[{"line_number":87,"context_line":""},{"line_number":88,"context_line":"    @staticmethod"},{"line_number":89,"context_line":"    def _from_db_object(context, service, db_service):"},{"line_number":90,"context_line":"        allow_missing \u003d (\u0027availability_zone\u0027, \u0027last_seen_up\u0027)"},{"line_number":91,"context_line":"        for key in service.fields:"},{"line_number":92,"context_line":"            if key in allow_missing and key not in db_service:"},{"line_number":93,"context_line":"                continue"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_ec4b22af","line":90,"in_reply_to":"fa963961_960ca795","updated":"2015-03-27 19:59:15.000000000","message":"Do we ensure that the DB is updated before anything else?  If we update nova-compute/nova-scheduler/nova-manage/etc. before the DB, then the DB object won\u0027t have the new field.  Or is that not allowed?\n\nIf we are guaranteed to update the DB first, can I unconditionally use service_ref[\u0027last_seen_up\u0027] in is_up() in servicegroup/drivers/db.py and drop the existing checks?","commit_id":"e7a2d29f658ca101e4abf70b047dba3b1b404713"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"4da742e11dc8477eb83a6f7317afa1bfbffb0b9b","unresolved":false,"context_lines":[{"line_number":110,"context_line":"                   \u0027name\u0027: self.obj_name(),"},{"line_number":111,"context_line":"                   \u0027id\u0027: self.id,"},{"line_number":112,"context_line":"                   })"},{"line_number":113,"context_line":"        if attrname \u003d\u003d \u0027last_seen_up\u0027:"},{"line_number":114,"context_line":"            self.last_seen_up \u003d None"},{"line_number":115,"context_line":"            return"},{"line_number":116,"context_line":"        if attrname !\u003d \u0027compute_node\u0027:"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_56273f12","line":113,"updated":"2015-03-27 19:04:53.000000000","message":"Not needed for the same reason as above.","commit_id":"e7a2d29f658ca101e4abf70b047dba3b1b404713"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"091fa4bc04a60f57c944d8968b5ab56bf5fed987","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            self._do_compute_node(self._context, primitive,"},{"line_number":75,"context_line":"                                  target_compute_version)"},{"line_number":76,"context_line":"        super(Service, self).obj_make_compatible(primitive, target_version)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _do_compute_node(self, context, primitive, target_version):"},{"line_number":79,"context_line":"        try:"},{"line_number":80,"context_line":"            # NOTE(sbauza): Some drivers (VMware, Ironic) can have multiple"}],"source_content_type":"text/x-python","patch_set":18,"id":"7aaa499b_4ea57402","line":77,"updated":"2015-04-13 15:10:23.000000000","message":"Here, you\u0027re adding a new field, I would also take the opportunity of the version bump to add a new method like \u0027increment_report_count()\u0027 which could be remotable.","commit_id":"86785819c175e7094ebf310a63bc38c335b31435"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"e36bd42c0a0bed371cb684c6901b21c99ba011f9","unresolved":false,"context_lines":[{"line_number":74,"context_line":"            self._do_compute_node(self._context, primitive,"},{"line_number":75,"context_line":"                                  target_compute_version)"},{"line_number":76,"context_line":"        super(Service, self).obj_make_compatible(primitive, target_version)"},{"line_number":77,"context_line":""},{"line_number":78,"context_line":"    def _do_compute_node(self, context, primitive, target_version):"},{"line_number":79,"context_line":"        try:"},{"line_number":80,"context_line":"            # NOTE(sbauza): Some drivers (VMware, Ironic) can have multiple"}],"source_content_type":"text/x-python","patch_set":18,"id":"7aaa499b_3004c6be","line":77,"in_reply_to":"7aaa499b_4ea57402","updated":"2015-04-13 15:48:42.000000000","message":"This might make sense after https://review.openstack.org/#/c/133663/ goes in.","commit_id":"86785819c175e7094ebf310a63bc38c335b31435"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"7085aa7c56543d1eb2a83a341fdcadcc436a8f46","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":69,"context_line":"        _target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":70,"context_line":"        if _target_version \u003c (1, 12) and \u0027last_seen_up\u0027 in primitive:"},{"line_number":71,"context_line":"            del primitive[\u0027last_seen_up\u0027]"},{"line_number":72,"context_line":"        if _target_version \u003c (1, 10):"},{"line_number":73,"context_line":"            target_compute_version \u003d self.obj_calculate_child_version("}],"source_content_type":"text/x-python","patch_set":25,"id":"3afb71cf_2bce0863","line":70,"range":{"start_line":70,"start_character":30,"end_line":70,"end_character":35},"updated":"2015-06-10 18:49:57.000000000","message":"This should be \u003c1.13 because you\u0027re adding it in 1.13. This will leave it for 1.12, which is not your revision.","commit_id":"861ddc4b186b59ec7008dcf4bd7b9929567b915d"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"b327793117871d32177ec7f8ac5de7e84b2cbba3","unresolved":false,"context_lines":[{"line_number":67,"context_line":""},{"line_number":68,"context_line":"    def obj_make_compatible(self, primitive, target_version):"},{"line_number":69,"context_line":"        _target_version \u003d utils.convert_version_to_tuple(target_version)"},{"line_number":70,"context_line":"        if _target_version \u003c (1, 12) and \u0027last_seen_up\u0027 in primitive:"},{"line_number":71,"context_line":"            del primitive[\u0027last_seen_up\u0027]"},{"line_number":72,"context_line":"        if _target_version \u003c (1, 10):"},{"line_number":73,"context_line":"            target_compute_version \u003d self.obj_calculate_child_version("}],"source_content_type":"text/x-python","patch_set":25,"id":"3afb71cf_8e506a33","line":70,"in_reply_to":"3afb71cf_2bce0863","updated":"2015-06-10 19:16:38.000000000","message":"Good catch. This was fallout from a rebase.  Will respin.","commit_id":"861ddc4b186b59ec7008dcf4bd7b9929567b915d"}],"nova/servicegroup/drivers/db.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"d095686c5ef28e66a99651ff17dd6187fde04fe3","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":66,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        last_heartbeat \u003d service_ref.get(\u0027reported_at\u0027) or \\"},{"line_number":69,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027]"},{"line_number":70,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":71,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_56bdb101","line":68,"range":{"start_line":68,"start_character":59,"end_line":68,"end_character":60},"updated":"2015-03-11 10:46:51.000000000","message":"I think is preferable to use `()` instead `\\`.","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"74d5a02ca75f16080a9af68241cea0a452ebca65","unresolved":false,"context_lines":[{"line_number":65,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":66,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":67,"context_line":"        \"\"\""},{"line_number":68,"context_line":"        last_heartbeat \u003d service_ref.get(\u0027reported_at\u0027) or \\"},{"line_number":69,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027]"},{"line_number":70,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":71,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_32b03bf4","line":68,"in_reply_to":"9a80dd14_56bdb101","updated":"2015-03-11 15:13:52.000000000","message":"Sure.","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d12016d13c3bf046f4f7061df8ee6c48209420d4","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":70,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        last_heartbeat \u003d service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027]"},{"line_number":73,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":74,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":75,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a6ced46_5ee3ec32","side":"PARENT","line":72,"updated":"2015-03-24 11:58:03.000000000","message":"What you want to do is to avoid the case when operator set the compute to \"down\" and update_at is updated right? - What about to quit early if service_ref[\u0027disabled\u0027] \u003d\u003d True?\n  \n  def is_up(self, ...):\n    if service_ref[\u0027disabled\u0027] \u003d\u003d True:\n      return False\n    last_heartbeat \u003d service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027]\n    ...","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"80b7934bb48b9d471b4afbc9771d7bceebbc0b1f","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":70,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        last_heartbeat \u003d service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027]"},{"line_number":73,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":74,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":75,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a6ced46_e63c09c2","side":"PARENT","line":72,"in_reply_to":"1a6ced46_5ee3ec32","updated":"2015-03-24 15:03:59.000000000","message":"I don\u0027t think that helps us any.  Suppose I have a nova-compute service that is \"down\" due to communications loss.  If I do a \"nova service-disable\" followed by a \"nova service-enable\" that will cause the service_ref[\u0027updated_at\u0027] to be updated with the current timestamp and the service will be detected as \"up\" for the next \"self.service_down_time\" seconds.\n\nWe need to explicitly distinguish between \"when was the row updated\" and \"when did the service report in\" because row updates can happen for multiple reasons but is_up() really only cares about when the service reported its state.","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"d3c9a3be34fe1e915bc711557ee479be41463078","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":70,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        last_heartbeat \u003d service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027]"},{"line_number":73,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":74,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":75,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa963961_25bb9810","side":"PARENT","line":72,"in_reply_to":"1a6ced46_8c3602e6","updated":"2015-03-26 13:43:04.000000000","message":"so if you do a \"nova service-enable\" updated_at will be updated with the current timestamp and yes the service will be detect it as up until the next self.service_down, what is wrong with that?","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"259b42aebedb7c7b0a5a712a367bd2f599786e6b","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":70,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        last_heartbeat \u003d service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027]"},{"line_number":73,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":74,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":75,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":11,"id":"1a6ced46_8c3602e6","side":"PARENT","line":72,"in_reply_to":"1a6ced46_e63c09c2","updated":"2015-03-24 23:08:23.000000000","message":"The above was a bit ambiguous.  It should really be \"...is_up() *should* only care about when the service reported its state.\"","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"3253769ebfa09d2e1078c41d7b201f5526bf7fe5","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":70,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        last_heartbeat \u003d service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027]"},{"line_number":73,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":74,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":75,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa963961_89c71a45","side":"PARENT","line":72,"in_reply_to":"fa963961_25bb9810","updated":"2015-03-26 16:10:10.000000000","message":"The scheduler will think that compute node is available to place instances on, but of course it\u0027s not, so we could end up with long delays until the RPC requests time out.  Also, because the system thinks the compute node is up it won\u0027t let us evacuate any instances that were on that compute node.","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"16a835ef765582f822244dd749080d5d56308a6f","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        try:"},{"line_number":109,"context_line":"            report_count \u003d service.service_ref[\u0027report_count\u0027] + 1"},{"line_number":110,"context_line":"            state_catalog[\u0027report_count\u0027] \u003d report_count"},{"line_number":111,"context_line":"            state_catalog[\u0027reported_at\u0027] \u003d timeutils.utcnow()"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"            service.service_ref \u003d self.conductor_api.service_update(ctxt,"},{"line_number":114,"context_line":"                    service.service_ref, state_catalog)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa963961_c80e43e5","line":111,"updated":"2015-03-26 14:06:22.000000000","message":"Like that, this value will be erased each time when passing in service_update (since you have added a condition if reported_at is defined) - That make me think we should to have a specific \"api\" to update the report and remove line 110 and 111 then call conductor_api.service_update_report(ctxt, ...)","commit_id":"a2170354153ddf01fd0d448becec99154886833c"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"3253769ebfa09d2e1078c41d7b201f5526bf7fe5","unresolved":false,"context_lines":[{"line_number":108,"context_line":"        try:"},{"line_number":109,"context_line":"            report_count \u003d service.service_ref[\u0027report_count\u0027] + 1"},{"line_number":110,"context_line":"            state_catalog[\u0027report_count\u0027] \u003d report_count"},{"line_number":111,"context_line":"            state_catalog[\u0027reported_at\u0027] \u003d timeutils.utcnow()"},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"            service.service_ref \u003d self.conductor_api.service_update(ctxt,"},{"line_number":114,"context_line":"                    service.service_ref, state_catalog)"}],"source_content_type":"text/x-python","patch_set":11,"id":"fa963961_e90b467c","line":111,"in_reply_to":"fa963961_c80e43e5","updated":"2015-03-26 16:10:10.000000000","message":"Sounds reasonable.","commit_id":"a2170354153ddf01fd0d448becec99154886833c"},{"author":{"_account_id":7730,"name":"Sahid Orentino Ferdjaoui","email":"sahid.ferdjaoui@industrialdiscipline.com","username":"sahid"},"change_message_id":"c1b602dbd42d9eb66b7381be1fb401a54a0638ef","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":70,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":73,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":74,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":75,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa963961_8117d8a2","line":72,"updated":"2015-03-27 07:51:05.000000000","message":"So during the update \u0027last_seen_up\u0027 will be set to None, and you will use the old solution with \u0027updated_at\u0027, seems good for me.\n\nCan we add a note about when we could remove this condition? probably for M*","commit_id":"c464548cf09925c3536606ba861128162551d759"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"542c7e01ce7ddf03d7bd2daa9d16f38086b21b7f","unresolved":false,"context_lines":[{"line_number":69,"context_line":"        \"\"\"Moved from nova.utils"},{"line_number":70,"context_line":"        Check whether a service is up based on last heartbeat."},{"line_number":71,"context_line":"        \"\"\""},{"line_number":72,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":73,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":74,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":75,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa963961_6aa020f5","line":72,"in_reply_to":"fa963961_8117d8a2","updated":"2015-03-27 14:42:46.000000000","message":"Sure.","commit_id":"c464548cf09925c3536606ba861128162551d759"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"b919aaaee803e6e72f35c1ca833d97f72f9c63ea","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":107,"context_line":"        state_catalog \u003d {}"},{"line_number":108,"context_line":"        try:"},{"line_number":109,"context_line":"            report_count \u003d service.service_ref[\u0027report_count\u0027] + 1"},{"line_number":110,"context_line":"            state_catalog[\u0027report_count\u0027] \u003d report_count"},{"line_number":111,"context_line":"            service.service_ref \u003d self.conductor_api.service_report(ctxt,"},{"line_number":112,"context_line":"                    service.service_ref, state_catalog)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa963961_d31db2be","line":109,"updated":"2015-03-27 11:07:45.000000000","message":"This might be a stupid idea but from what I can tell this is the only place we ever update service[\u0027report_count\u0027]. What if we pick that up at the db-level and set last_seen_up to the current timestamp based on that. This would require minimal changes, basically just db changes and the above change to is_up() to read the new value.\n\nComments?","commit_id":"c464548cf09925c3536606ba861128162551d759"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"542c7e01ce7ddf03d7bd2daa9d16f38086b21b7f","unresolved":false,"context_lines":[{"line_number":106,"context_line":"        ctxt \u003d context.get_admin_context()"},{"line_number":107,"context_line":"        state_catalog \u003d {}"},{"line_number":108,"context_line":"        try:"},{"line_number":109,"context_line":"            report_count \u003d service.service_ref[\u0027report_count\u0027] + 1"},{"line_number":110,"context_line":"            state_catalog[\u0027report_count\u0027] \u003d report_count"},{"line_number":111,"context_line":"            service.service_ref \u003d self.conductor_api.service_report(ctxt,"},{"line_number":112,"context_line":"                    service.service_ref, state_catalog)"}],"source_content_type":"text/x-python","patch_set":12,"id":"fa963961_ca6ccc9e","line":109,"in_reply_to":"fa963961_d31db2be","updated":"2015-03-27 14:42:46.000000000","message":"I like it!  That\u0027s really nice actually as far as an efficient solution.","commit_id":"c464548cf09925c3536606ba861128162551d759"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"4da742e11dc8477eb83a6f7317afa1bfbffb0b9b","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            report_count \u003d service.service_ref[\u0027report_count\u0027] + 1"},{"line_number":109,"context_line":"            state_catalog[\u0027report_count\u0027] \u003d report_count"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"            service.service_ref \u003d self.conductor_api.service_update(ctxt,"},{"line_number":112,"context_line":"                    service.service_ref, state_catalog)"},{"line_number":113,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_c9ba90e1","side":"PARENT","line":110,"updated":"2015-03-27 19:04:53.000000000","message":"remove this","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"f3bc3a354ce7f86b708cdb2c26e3056d9ebfa107","unresolved":false,"context_lines":[{"line_number":107,"context_line":"        try:"},{"line_number":108,"context_line":"            report_count \u003d service.service_ref[\u0027report_count\u0027] + 1"},{"line_number":109,"context_line":"            state_catalog[\u0027report_count\u0027] \u003d report_count"},{"line_number":110,"context_line":""},{"line_number":111,"context_line":"            service.service_ref \u003d self.conductor_api.service_update(ctxt,"},{"line_number":112,"context_line":"                    service.service_ref, state_catalog)"},{"line_number":113,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_cc185e87","side":"PARENT","line":110,"in_reply_to":"fa963961_c9ba90e1","updated":"2015-03-27 19:59:15.000000000","message":"will do","commit_id":"d1bba0d1f088808c6fa43f36cf591568b8513231"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"26c910d2aa6edbafa54428233d0235172082b07c","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # Keep checking \u0027updated_at\u0027 if \u0027last_seen_up\u0027 isn\u0027t set."},{"line_number":73,"context_line":"        # Should be able to use only \u0027last_seen_up\u0027 in the M release"},{"line_number":74,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":75,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":76,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":77,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":78,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa963961_5db07af4","line":75,"updated":"2015-03-27 21:20:20.000000000","message":"This should be in the object, not in the thing(s) using it.","commit_id":"086e221facf37bf056ef17b18662d05d6c95ac09"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"2d19b6d4ebb930909ed5b294b6d57ed35f092f2b","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # Keep checking \u0027updated_at\u0027 if \u0027last_seen_up\u0027 isn\u0027t set."},{"line_number":73,"context_line":"        # Should be able to use only \u0027last_seen_up\u0027 in the M release"},{"line_number":74,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":75,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":76,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":77,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":78,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":14,"id":"da9b358b_262b7b32","line":75,"in_reply_to":"da9b358b_074d3a72","updated":"2015-03-30 14:03:23.000000000","message":"Right, so maybe it does belong here. If so, then there is no point in you adding it to the service object I think. If this code were using objects, then it would make sense, but AFAICT, you add it  the object and no other code consumes it.\n\nAlso, I don\u0027t think that a single datestamp is worth the hassle of subclassing the object. It also wouldn\u0027t surprise me if the other drivers could make use of this field in some way.","commit_id":"086e221facf37bf056ef17b18662d05d6c95ac09"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"eefdb3d6f00520d801ee0b5ba4b1bbcacb558d2a","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # Keep checking \u0027updated_at\u0027 if \u0027last_seen_up\u0027 isn\u0027t set."},{"line_number":73,"context_line":"        # Should be able to use only \u0027last_seen_up\u0027 in the M release"},{"line_number":74,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":75,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":76,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":77,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":78,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":14,"id":"da9b358b_8937f8af","line":75,"in_reply_to":"da9b358b_262b7b32","updated":"2015-03-30 14:25:57.000000000","message":"The services are often queried from the DB by calling objects.Service.get_by_*() or objects.ServiceList.get_*() and then is_up() gets called on the resulting object, so I think I do actually need to add it to the service object.\n\nIf I don\u0027t add it to the service object, then I\u0027d need another DB access in here to get the \"last_seen_up\" value.","commit_id":"086e221facf37bf056ef17b18662d05d6c95ac09"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"de4093f44045c924187ed5358933cd3e0d920e70","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # Keep checking \u0027updated_at\u0027 if \u0027last_seen_up\u0027 isn\u0027t set."},{"line_number":73,"context_line":"        # Should be able to use only \u0027last_seen_up\u0027 in the M release"},{"line_number":74,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":75,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":76,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":77,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":78,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":14,"id":"da9b358b_8931380e","line":75,"in_reply_to":"da9b358b_8937f8af","updated":"2015-03-30 14:42:26.000000000","message":"Okay, fair enough. This whole module is in need of some serious cleanup I think, as there is still a lot of conductor_api usage and, obviously, a mix of object and db reference bits.","commit_id":"086e221facf37bf056ef17b18662d05d6c95ac09"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"4c4b6589c902ca2dfa49a002a44d9f3050e6ba7f","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # Keep checking \u0027updated_at\u0027 if \u0027last_seen_up\u0027 isn\u0027t set."},{"line_number":73,"context_line":"        # Should be able to use only \u0027last_seen_up\u0027 in the M release"},{"line_number":74,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":75,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":76,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":77,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":78,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":14,"id":"da9b358b_074d3a72","line":75,"in_reply_to":"fa963961_5db07af4","updated":"2015-03-28 00:21:29.000000000","message":"1) Are all callers guaranteed to be using the service object rather than just a dict?  If so should we change this code to use object members directly instead of using the dict syntax?\n\n2) The whole \"last_heartbeat\" check really only applies to the DB driver, but all the various drivers share the same Service object.  Conceptually it would be cleaner if the DB backend could subclass the Service object and make a different object with an extra field in it and pass that around to everything expecting a Service object, then in this routine treat it as the new object.  Is that possible?","commit_id":"086e221facf37bf056ef17b18662d05d6c95ac09"},{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"85f56138e9f899a19270886b7fd580eeb5000bf8","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # Keep checking \u0027updated_at\u0027 if \u0027last_seen_up\u0027 isn\u0027t set."},{"line_number":73,"context_line":"        # Should be able to use only \u0027last_seen_up\u0027 in the M release"},{"line_number":74,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":75,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":76,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":77,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":78,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":14,"id":"fa963961_7826ac69","line":75,"in_reply_to":"fa963961_5db07af4","updated":"2015-03-30 18:04:51.000000000","message":"Dan, see my attempt at objectifying this module at https://review.openstack.org/133663","commit_id":"086e221facf37bf056ef17b18662d05d6c95ac09"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"091fa4bc04a60f57c944d8968b5ab56bf5fed987","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # Keep checking \u0027updated_at\u0027 if \u0027last_seen_up\u0027 isn\u0027t set."},{"line_number":73,"context_line":"        # Should be able to use only \u0027last_seen_up\u0027 in the M release"},{"line_number":74,"context_line":"        last_heartbeat \u003d (service_ref.get(\u0027last_seen_up\u0027) or"},{"line_number":75,"context_line":"            service_ref[\u0027updated_at\u0027] or service_ref[\u0027created_at\u0027])"},{"line_number":76,"context_line":"        if isinstance(last_heartbeat, six.string_types):"},{"line_number":77,"context_line":"            # NOTE(russellb) If this service_ref came in over rpc via"},{"line_number":78,"context_line":"            # conductor, then the timestamp will be a string and needs to be"}],"source_content_type":"text/x-python","patch_set":18,"id":"7aaa499b_aeb6383b","line":75,"updated":"2015-04-13 15:10:23.000000000","message":"++ I like this","commit_id":"86785819c175e7094ebf310a63bc38c335b31435"}],"nova/tests/unit/db/test_migrations.py":[{"author":{"_account_id":8412,"name":"Mike Durnosvistov","email":"glacierr.dev@gmail.com","username":"mdurnosvistov"},"change_message_id":"b5d384857245573a71cb4a388243e5d429d5848c","unresolved":false,"context_lines":[{"line_number":843,"context_line":"        self.assertIsInstance(shadow_services.c.reported_at.type,"},{"line_number":844,"context_line":"                              sqlalchemy.types.DateTime)"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    def _post_downgrade_276(self, engine):"},{"line_number":847,"context_line":"        self.assertColumnNotExists(engine, \u0027services\u0027, \u0027reported_at\u0027)"},{"line_number":848,"context_line":"        self.assertColumnNotExists(engine, \u0027shadow_services\u0027,"},{"line_number":849,"context_line":"                                   \u0027reported_at\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_360f5d30","line":846,"updated":"2015-03-11 10:39:37.000000000","message":"F811 redefinition of unused \u0027_post_downgrade_276\u0027 from line 820","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"74d5a02ca75f16080a9af68241cea0a452ebca65","unresolved":false,"context_lines":[{"line_number":843,"context_line":"        self.assertIsInstance(shadow_services.c.reported_at.type,"},{"line_number":844,"context_line":"                              sqlalchemy.types.DateTime)"},{"line_number":845,"context_line":""},{"line_number":846,"context_line":"    def _post_downgrade_276(self, engine):"},{"line_number":847,"context_line":"        self.assertColumnNotExists(engine, \u0027services\u0027, \u0027reported_at\u0027)"},{"line_number":848,"context_line":"        self.assertColumnNotExists(engine, \u0027shadow_services\u0027,"},{"line_number":849,"context_line":"                                   \u0027reported_at\u0027)"}],"source_content_type":"text/x-python","patch_set":2,"id":"9a80dd14_d2b51fd1","line":846,"in_reply_to":"9a80dd14_360f5d30","updated":"2015-03-11 15:13:52.000000000","message":"Fixed","commit_id":"78af24105634212cf06341d3e7eb68c2af5718a6"}],"nova/tests/unit/objects/test_service.py":[{"author":{"_account_id":6450,"name":"Hans Lindgren","email":"hanlind@kth.se","username":"hanlind"},"change_message_id":"4da742e11dc8477eb83a6f7317afa1bfbffb0b9b","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    \u0027topic\u0027: \u0027fake-service-topic\u0027,"},{"line_number":37,"context_line":"    \u0027report_count\u0027: 1,"},{"line_number":38,"context_line":"    \u0027disabled\u0027: False,"},{"line_number":39,"context_line":"    \u0027disabled_reason\u0027: None,"},{"line_number":40,"context_line":"    }"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"OPTIONAL \u003d [\u0027availability_zone\u0027, \u0027compute_node\u0027, \u0027last_seen_up\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_16fd373a","line":39,"updated":"2015-03-27 19:04:53.000000000","message":"Add the column here instead of making it optional below.","commit_id":"e7a2d29f658ca101e4abf70b047dba3b1b404713"},{"author":{"_account_id":8768,"name":"Chris Friesen","email":"chris.friesen@windriver.com","username":"cbf123"},"change_message_id":"f3bc3a354ce7f86b708cdb2c26e3056d9ebfa107","unresolved":false,"context_lines":[{"line_number":36,"context_line":"    \u0027topic\u0027: \u0027fake-service-topic\u0027,"},{"line_number":37,"context_line":"    \u0027report_count\u0027: 1,"},{"line_number":38,"context_line":"    \u0027disabled\u0027: False,"},{"line_number":39,"context_line":"    \u0027disabled_reason\u0027: None,"},{"line_number":40,"context_line":"    }"},{"line_number":41,"context_line":""},{"line_number":42,"context_line":"OPTIONAL \u003d [\u0027availability_zone\u0027, \u0027compute_node\u0027, \u0027last_seen_up\u0027]"}],"source_content_type":"text/x-python","patch_set":13,"id":"fa963961_c74dd773","line":39,"in_reply_to":"fa963961_16fd373a","updated":"2015-03-27 19:59:15.000000000","message":"okay","commit_id":"e7a2d29f658ca101e4abf70b047dba3b1b404713"}]}