)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"d94a9592e0cb587bcdf51655e7323bc379c06b23","unresolved":false,"context_lines":[{"line_number":10,"context_line":"(\u0027instance_type_id\u0027, \u0027key\u0027, \u0027deleted\u0027) to InstanceTypeExtraSpecs model and migrate"},{"line_number":11,"context_line":"sripts."},{"line_number":12,"context_line":"Implementation instance_type_extra_specs_update_or_create in sqlalchemy.api"},{"line_number":13,"context_line":"rewrited."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"blueprint db-enforce-unique-keys"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":6,"id":"AAAAP3%2F%2F%2BBM%3D","line":13,"updated":"2013-07-08 11:42:19.000000000","message":"s/rewrited/rewritten/","commit_id":"7455c02ed6714bf2f2a3f0615863d1136c837e72"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ea43ca36165145ccc23b945090240009b06b42f2","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Added unique constraint \u0027uniq_instance_type_extra_specs0instance_type_id0key0deleted\u0027"},{"line_number":10,"context_line":"(\u0027instance_type_id\u0027, \u0027key\u0027, \u0027deleted\u0027) to InstanceTypeExtraSpecs model and migrate"},{"line_number":11,"context_line":"sripts."},{"line_number":12,"context_line":"Implementation instance_type_extra_specs_update_or_create in sqlalchemy.api"},{"line_number":13,"context_line":"rewritten."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAP3%2F%2F0vY%3D","line":11,"updated":"2013-07-10 21:10:42.000000000","message":"s/sripts/scripts/","commit_id":"6a185614f02c1e123f1f471879dea26f127e167f"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"48758b6280e3f8fe81a2d1a29bf4ac1bde396c2b","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Added unique constraint \u0027uniq_instance_type_extra_specs0instance_type_id0key0deleted\u0027"},{"line_number":10,"context_line":"(\u0027instance_type_id\u0027, \u0027key\u0027, \u0027deleted\u0027) to InstanceTypeExtraSpecs model and migrate"},{"line_number":11,"context_line":"sripts."},{"line_number":12,"context_line":"Implementation instance_type_extra_specs_update_or_create in sqlalchemy.api"},{"line_number":13,"context_line":"rewritten."},{"line_number":14,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAP3%2F%2FzIo%3D","line":11,"in_reply_to":"AAAAP3%2F%2F0vY%3D","updated":"2013-07-11 08:29:49.000000000","message":"Done","commit_id":"6a185614f02c1e123f1f471879dea26f127e167f"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"ea43ca36165145ccc23b945090240009b06b42f2","unresolved":false,"context_lines":[{"line_number":10,"context_line":"(\u0027instance_type_id\u0027, \u0027key\u0027, \u0027deleted\u0027) to InstanceTypeExtraSpecs model and migrate"},{"line_number":11,"context_line":"sripts."},{"line_number":12,"context_line":"Implementation instance_type_extra_specs_update_or_create in sqlalchemy.api"},{"line_number":13,"context_line":"rewritten."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"blueprint db-enforce-unique-keys"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAP3%2F%2F0vI%3D","line":13,"updated":"2013-07-10 21:10:42.000000000","message":"Can you add a little about how and why it was rewritten?\n\nAlso, all of these lines seem oddly broken.","commit_id":"6a185614f02c1e123f1f471879dea26f127e167f"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"48758b6280e3f8fe81a2d1a29bf4ac1bde396c2b","unresolved":false,"context_lines":[{"line_number":10,"context_line":"(\u0027instance_type_id\u0027, \u0027key\u0027, \u0027deleted\u0027) to InstanceTypeExtraSpecs model and migrate"},{"line_number":11,"context_line":"sripts."},{"line_number":12,"context_line":"Implementation instance_type_extra_specs_update_or_create in sqlalchemy.api"},{"line_number":13,"context_line":"rewritten."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"blueprint db-enforce-unique-keys"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":9,"id":"AAAAP3%2F%2FzIk%3D","line":13,"in_reply_to":"AAAAP3%2F%2F0vI%3D","updated":"2013-07-11 08:29:49.000000000","message":"Done","commit_id":"6a185614f02c1e123f1f471879dea26f127e167f"}],"nova/db/sqlalchemy/api.py":[{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"98d186aa371dfd9b762b1411e20b5965e496b4b9","unresolved":false,"context_lines":[{"line_number":4014,"context_line":"                return key"},{"line_number":4015,"context_line":"        return None"},{"line_number":4016,"context_line":""},{"line_number":4017,"context_line":"    while True:"},{"line_number":4018,"context_line":"        dup \u003d specs_create()"},{"line_number":4019,"context_line":"        if dup is None:"},{"line_number":4020,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2Fa1w%3D","line":4017,"updated":"2013-07-02 12:11:46.000000000","message":"while True:\n    for key, value in specs.items():\n         try:\n             with get_session().begin():\n                  specs_ref \u003d models.InstanceTypeExtraSpecs()\n                  specs_ref.update(...)\n                  session.add(specs_ref)\n          except db_exc.DBDuplicateEntry:\n                  failed_keys.append(key)","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"e4e5366b372f5d9ae2c213662c797626b662aac8","unresolved":false,"context_lines":[{"line_number":4014,"context_line":"                return key"},{"line_number":4015,"context_line":"        return None"},{"line_number":4016,"context_line":""},{"line_number":4017,"context_line":"    while True:"},{"line_number":4018,"context_line":"        dup \u003d specs_create()"},{"line_number":4019,"context_line":"        if dup is None:"},{"line_number":4020,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2FXac%3D","line":4017,"in_reply_to":"AAAAPn%2F%2FXdw%3D","updated":"2013-07-03 08:25:21.000000000","message":"get_session() ensures you have a separate session/transaction for each insert","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"0dee4c10f0b99987dd68268905d40c149715a5af","unresolved":false,"context_lines":[{"line_number":4014,"context_line":"                return key"},{"line_number":4015,"context_line":"        return None"},{"line_number":4016,"context_line":""},{"line_number":4017,"context_line":"    while True:"},{"line_number":4018,"context_line":"        dup \u003d specs_create()"},{"line_number":4019,"context_line":"        if dup is None:"},{"line_number":4020,"context_line":"            break"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2FXdw%3D","line":4017,"in_reply_to":"AAAAPn%2F%2Fa1w%3D","updated":"2013-07-03 07:55:40.000000000","message":"After exception previous specs_refs are expunged, we need restart cycle.\n\nSqlalchemy code:\ndef __exit__(self, type, value, traceback):\n       ....\n        if type is None:\n            try:\n                self.commit()\n            except:\n                self.rollback()\n                raise\n        else:\n            self.rollback()\n\nSqlalchemy doc:\nRolling Back\n        ...\n        Objects which were initially in the pending state\nwhen they were added to the Session within the lifespan\nof the transaction are expunged","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"98d186aa371dfd9b762b1411e20b5965e496b4b9","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"@require_context"},{"line_number":4036,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4037,"context_line":"    session \u003d get_session()"},{"line_number":4038,"context_line":"    session.begin()"},{"line_number":4039,"context_line":"    try:"},{"line_number":4040,"context_line":"        instance_type_id \u003d \\"},{"line_number":4041,"context_line":"                _instance_type_get_id_from_flavor(context, flavor_id, session)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2Fa2s%3D","line":4038,"updated":"2013-07-02 12:11:46.000000000","message":"session.begin() returns a transaction context manager.\n\nUse it like this:\n   with session.begin():\n       ...\n\nThe transaction will be committed on success or rolled back on error automatically.","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"2f0c5120c91359283df256d32ee66fb15c43ea1c","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"@require_context"},{"line_number":4036,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4037,"context_line":"    session \u003d get_session()"},{"line_number":4038,"context_line":"    session.begin()"},{"line_number":4039,"context_line":"    try:"},{"line_number":4040,"context_line":"        instance_type_id \u003d \\"},{"line_number":4041,"context_line":"                _instance_type_get_id_from_flavor(context, flavor_id, session)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2FTVM%3D","line":4038,"in_reply_to":"AAAAPn%2F%2FTWo%3D","updated":"2013-07-04 14:40:45.000000000","message":"Ahh. Sorry, missed that. Code is really complicated...\n\nI would suggest you to raise an exception  in _instance_type_extra_specs_safe_create() on IntegrityError and handle it in instance_type_extra_specs_update_or_create() to ensure you can try the insertion again omitting the duplicating entry.\n\nI believe, that handling begin()/rollback()/\"try insertion again\" at the top level (in instance_type_extra_specs_update_or_create() function) will make the code easier and more readable.","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"b3edd80baf7d970ae9856c0ffbecae28aa543c05","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"@require_context"},{"line_number":4036,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4037,"context_line":"    session \u003d get_session()"},{"line_number":4038,"context_line":"    session.begin()"},{"line_number":4039,"context_line":"    try:"},{"line_number":4040,"context_line":"        instance_type_id \u003d \\"},{"line_number":4041,"context_line":"                _instance_type_get_id_from_flavor(context, flavor_id, session)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2FTWo%3D","line":4038,"in_reply_to":"AAAAPn%2F%2FUJo%3D","updated":"2013-07-04 14:17:34.000000000","message":"\"but first 50 entries are lost\"\nNot lost, look at my code\n\nwhile True:\n    dup \u003d specs_create()\n    if dup is None:\n        break\n    failed_keys.append(dup)\n\nall entires re-inserted again, only w/o 51 entry.\n\nI see three ways to implement this method:\n\n1. Do not use transaction - bad way in my opinion.\n\n2. Raise exception from this method in case UC violation.\n\n3. Use logic based on UC inside method.\n\nMay be 2 preferred way.","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"09c6c0122347e80c037805abb0b26b92a91fbc75","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"@require_context"},{"line_number":4036,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4037,"context_line":"    session \u003d get_session()"},{"line_number":4038,"context_line":"    session.begin()"},{"line_number":4039,"context_line":"    try:"},{"line_number":4040,"context_line":"        instance_type_id \u003d \\"},{"line_number":4041,"context_line":"                _instance_type_get_id_from_flavor(context, flavor_id, session)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2FUJo%3D","line":4038,"in_reply_to":"AAAAPn%2F%2FWy4%3D","updated":"2013-07-04 08:01:10.000000000","message":"Ok. Let\u0027s consider the following situation:\n\nYou are doing insertion of 100 new entries in _instance_type_extra_specs_safe_create(). You have already inserted 50 entries, and at this moment a concurrent transaction is committed. The 51st insert fails on flush due to integrity error (unique constraint violation). rollback() call at line 4012 will roll back the current transaction deleting 50 rows that have been already inserted. Left 49 entries can be successfully inserted and you report an error of insertion of 51st entry, but first 50 entries are lost.\n\nSo these inserts must be done in a single transaction. This can actually fail due to IntegrityError, but it\u0027s easy to do a global rollback this way (and e.g. repeat the insertion without duplicating entry in a loop until it finally succeeds). I would rather you detected the IntegrityError at the commit() moment.\n\nBesides, every DB API method has an implicit contract of being executed transactionally - if it fails, the DB state must be rolled back to the original state (before the method was called, no \u0027partial\u0027 results are allowed). It is crucial to obey this contract to e.g. make it possible to implement blueprint db-reconnect right.","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"e60a423a725c8ce812515e2f055d9ca0b99105cf","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"@require_context"},{"line_number":4036,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4037,"context_line":"    session \u003d get_session()"},{"line_number":4038,"context_line":"    session.begin()"},{"line_number":4039,"context_line":"    try:"},{"line_number":4040,"context_line":"        instance_type_id \u003d \\"},{"line_number":4041,"context_line":"                _instance_type_get_id_from_flavor(context, flavor_id, session)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2FWy4%3D","line":4038,"in_reply_to":"AAAAPn%2F%2FXZc%3D","updated":"2013-07-03 13:09:02.000000000","message":"All inserts/updates are done in a single transaction - good, but\nsession.flush() required each time for check what insert violate UC.\nSqlalchemy doc: \"Any failures during flush will always result in a rollback of whatever transaction is present.\"\nI try also without result session.begin_nested().","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"e4e5366b372f5d9ae2c213662c797626b662aac8","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"@require_context"},{"line_number":4036,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4037,"context_line":"    session \u003d get_session()"},{"line_number":4038,"context_line":"    session.begin()"},{"line_number":4039,"context_line":"    try:"},{"line_number":4040,"context_line":"        instance_type_id \u003d \\"},{"line_number":4041,"context_line":"                _instance_type_get_id_from_flavor(context, flavor_id, session)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2FXZc%3D","line":4038,"in_reply_to":"AAAAPn%2F%2FXgg%3D","updated":"2013-07-03 08:25:21.000000000","message":"My point is - don\u0027t close transactions manually. Either create a separate transaction using get_session().begin() explicitly or (better) make sure that all inserts/updates are done in a single transaction (you actually can get an IntegrityError, if some of those values violate the unique constraint, but it\u0027s easy to make a ROLLBACK and try again).","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"0dee4c10f0b99987dd68268905d40c149715a5af","unresolved":false,"context_lines":[{"line_number":4035,"context_line":"@require_context"},{"line_number":4036,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4037,"context_line":"    session \u003d get_session()"},{"line_number":4038,"context_line":"    session.begin()"},{"line_number":4039,"context_line":"    try:"},{"line_number":4040,"context_line":"        instance_type_id \u003d \\"},{"line_number":4041,"context_line":"                _instance_type_get_id_from_flavor(context, flavor_id, session)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2FXgg%3D","line":4038,"in_reply_to":"AAAAPn%2F%2Fa2s%3D","updated":"2013-07-03 07:55:40.000000000","message":"If transaction closed manually we cannot use context manager.\nCode from sqlalchemy:\ndef __exit__(self, type, value, traceback):\n        self._assert_is_open(\"Cannot end transaction context. The transaction \"\n                                    \"was closed from within the context\")","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"98d186aa371dfd9b762b1411e20b5965e496b4b9","unresolved":false,"context_lines":[{"line_number":4048,"context_line":"                            if k not in existing_keys])"},{"line_number":4049,"context_line":"        failed \u003d _instance_type_extra_specs_safe_create(instance_type_id,"},{"line_number":4050,"context_line":"                                                        not_existing, session)"},{"line_number":4051,"context_line":"        if len(failed) !\u003d 0:"},{"line_number":4052,"context_line":"            fail_refs \u003d _instance_type_extra_specs_get_by_keys(context,"},{"line_number":4053,"context_line":"                                        instance_type_id, failed, session)"},{"line_number":4054,"context_line":"            spec_refs.extend(fail_refs)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAPn%2F%2Fa20%3D","line":4051,"updated":"2013-07-02 12:11:46.000000000","message":"if failed:\n   ...","commit_id":"c8c4e6c5f718364cb27ec7b96f3efcca73703992"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"42c0de5704dd65a64403d12fa0345d539878fe06","unresolved":false,"context_lines":[{"line_number":4083,"context_line":""},{"line_number":4084,"context_line":""},{"line_number":4085,"context_line":"@require_context"},{"line_number":4086,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4087,"context_line":"    session \u003d get_session()"},{"line_number":4088,"context_line":"    session.begin()"},{"line_number":4089,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAP3%2F%2F6pY%3D","line":4086,"updated":"2013-07-09 09:09:44.000000000","message":"I would really like to see a simpler solution, something like this:\n\n@require_context\ndef instance_type_extra_specs_update_or_create(context, flavor_id, specs, max_retries\u003d10):\n    for i in xrange(max_retries):\n        try:\n            session \u003d get_session()\n            with session.begin():\n                instance_type_id \u003d _instance_type_get_id_from_flavor(\n                    context, flavor_id, session)\n\n                spec_refs \u003d model_query(context, models.InstanceTypeExtraSpecs,\n                                        session\u003dsession, read_deleted\u003d\"no\").\\\n                    filter_by(instance_type_id\u003dinstance_type_id).\\\n                    filter(models.InstanceTypeExtraSpecs.key.in_(specs.keys())).\\\n                    all()\n\n                existing_keys \u003d set()\n                for spec_ref in spec_refs:\n                    key \u003d spec_ref[\"key\"]\n                    existing_keys.add(key)\n                    spec_ref.update({\"value\": specs[key]})\n\n                for key, value in specs.iteritems():\n                    if key in existing_keys:\n                        continue\n\n                    spec_ref \u003d models.InstanceTypeExtraSpecs()\n                    spec_ref.update({\"key\": key, \"value\": value,\n                                     \"instance_type_id\": instance_type_id})\n                    session.add(spec_ref)\n        except db_exc.DBDuplicateEntry:\n            # a concurrent transaction has been commited, try again\n            pass\n\n    return specs\n\n\nwhat do you think?","commit_id":"7455c02ed6714bf2f2a3f0615863d1136c837e72"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"03b4bc61f7469cdada78a51bd577779f77e389cf","unresolved":false,"context_lines":[{"line_number":4083,"context_line":""},{"line_number":4084,"context_line":""},{"line_number":4085,"context_line":"@require_context"},{"line_number":4086,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs):"},{"line_number":4087,"context_line":"    session \u003d get_session()"},{"line_number":4088,"context_line":"    session.begin()"},{"line_number":4089,"context_line":"    try:"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAP3%2F%2F6Qw%3D","line":4086,"in_reply_to":"AAAAP3%2F%2F6pY%3D","updated":"2013-07-09 11:29:07.000000000","message":"I think we need additional exception for top level in this case:\n\nfor i in xrange(max_retries):\n    try:\n        ...\n    except db_exc.DBDuplicateEntry:\n        # a concurrent transaction has been commited, try again\n        pass\n    else:\n        return specs\nraise InstanceTypeExtraSpecsExists","commit_id":"7455c02ed6714bf2f2a3f0615863d1136c837e72"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"1e95d039f875de1cbdc88ec042fd878031bdb2d4","unresolved":false,"context_lines":[{"line_number":4089,"context_line":"                                     \"instance_type_id\": instance_type_id})"},{"line_number":4090,"context_line":"                    session.add(spec_ref)"},{"line_number":4091,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":4092,"context_line":"            # a concurrent transaction has been commited"},{"line_number":4093,"context_line":"            if attempt \u003d\u003d max_retries - 1:"},{"line_number":4094,"context_line":"                raise"},{"line_number":4095,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F2kw%3D","line":4092,"updated":"2013-07-10 12:15:49.000000000","message":"This comment is a bit confusing. It probably makes sense to rephrase it,to something like this:\n\n# a concurrent transaction has been committed,\n\n# try again unless this was the last attempt","commit_id":"4d2ffdf94fd82cc0da1a5273e12ba3eabec45618"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"9fa536f18ca19da3d63b11363b26396da000ce23","unresolved":false,"context_lines":[{"line_number":4089,"context_line":"                                     \"instance_type_id\": instance_type_id})"},{"line_number":4090,"context_line":"                    session.add(spec_ref)"},{"line_number":4091,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":4092,"context_line":"            # a concurrent transaction has been commited"},{"line_number":4093,"context_line":"            if attempt \u003d\u003d max_retries - 1:"},{"line_number":4094,"context_line":"                raise"},{"line_number":4095,"context_line":"        else:"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F2F8%3D","line":4092,"in_reply_to":"AAAAP3%2F%2F2kw%3D","updated":"2013-07-10 14:31:02.000000000","message":"Done","commit_id":"4d2ffdf94fd82cc0da1a5273e12ba3eabec45618"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"1e95d039f875de1cbdc88ec042fd878031bdb2d4","unresolved":false,"context_lines":[{"line_number":4093,"context_line":"            if attempt \u003d\u003d max_retries - 1:"},{"line_number":4094,"context_line":"                raise"},{"line_number":4095,"context_line":"        else:"},{"line_number":4096,"context_line":"            return specs"},{"line_number":4097,"context_line":""},{"line_number":4098,"context_line":""},{"line_number":4099,"context_line":"####################"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F2ik%3D","line":4096,"updated":"2013-07-10 12:15:49.000000000","message":"nit: this can be put inside with block","commit_id":"4d2ffdf94fd82cc0da1a5273e12ba3eabec45618"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"9fa536f18ca19da3d63b11363b26396da000ce23","unresolved":false,"context_lines":[{"line_number":4093,"context_line":"            if attempt \u003d\u003d max_retries - 1:"},{"line_number":4094,"context_line":"                raise"},{"line_number":4095,"context_line":"        else:"},{"line_number":4096,"context_line":"            return specs"},{"line_number":4097,"context_line":""},{"line_number":4098,"context_line":""},{"line_number":4099,"context_line":"####################"}],"source_content_type":"text/x-python","patch_set":7,"id":"AAAAP3%2F%2F2F0%3D","line":4096,"in_reply_to":"AAAAP3%2F%2F2ik%3D","updated":"2013-07-10 14:31:02.000000000","message":"Done","commit_id":"4d2ffdf94fd82cc0da1a5273e12ba3eabec45618"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"05b736090a99e08efd3b729eb05843819296536f","unresolved":false,"context_lines":[{"line_number":4081,"context_line":""},{"line_number":4082,"context_line":"@require_context"},{"line_number":4083,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs,"},{"line_number":4084,"context_line":"                                           max_retries\u003d10):"},{"line_number":4085,"context_line":"    for attempt in xrange(max_retries):"},{"line_number":4086,"context_line":"        try:"},{"line_number":4087,"context_line":"            session \u003d get_session()"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAP3%2F%2FuBc%3D","line":4084,"updated":"2013-07-12 14:30:25.000000000","message":"shouldn\u0027t this line be aligned with the previous one?","commit_id":"ce202726a389dceedc0fb2f7441caf7f4f8e85c6"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"7028600c8f890927eeb6fff4cf820e3641614ae3","unresolved":false,"context_lines":[{"line_number":4081,"context_line":""},{"line_number":4082,"context_line":"@require_context"},{"line_number":4083,"context_line":"def instance_type_extra_specs_update_or_create(context, flavor_id, specs,"},{"line_number":4084,"context_line":"                                           max_retries\u003d10):"},{"line_number":4085,"context_line":"    for attempt in xrange(max_retries):"},{"line_number":4086,"context_line":"        try:"},{"line_number":4087,"context_line":"            session \u003d get_session()"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAP3%2F%2FtTI%3D","line":4084,"in_reply_to":"AAAAP3%2F%2FuBc%3D","updated":"2013-07-12 17:06:08.000000000","message":"Done","commit_id":"ce202726a389dceedc0fb2f7441caf7f4f8e85c6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"85950becdd34646a919bf5fb35009bba6e781542","unresolved":false,"context_lines":[{"line_number":4110,"context_line":"                    session.add(spec_ref)"},{"line_number":4111,"context_line":""},{"line_number":4112,"context_line":"            return specs"},{"line_number":4113,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":4114,"context_line":"            # a concurrent transaction has been committed,"},{"line_number":4115,"context_line":"            # try again unless this was the last attempt"},{"line_number":4116,"context_line":"            if attempt \u003d\u003d max_retries - 1:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAP3%2F%2Fq7c%3D","line":4113,"updated":"2013-07-14 11:55:16.000000000","message":"I am sorry but I seem to be missing something here. Why would a retry solve the issue unless another process/thread is modifying the entry.\nCan you please clarify","commit_id":"0d9cf62bfc929c7bd844b7722c380e029ffc50d4"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"3a20e1621069d8c782929b452ad075533bdc625a","unresolved":false,"context_lines":[{"line_number":4110,"context_line":"                    session.add(spec_ref)"},{"line_number":4111,"context_line":""},{"line_number":4112,"context_line":"            return specs"},{"line_number":4113,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":4114,"context_line":"            # a concurrent transaction has been committed,"},{"line_number":4115,"context_line":"            # try again unless this was the last attempt"},{"line_number":4116,"context_line":"            if attempt \u003d\u003d max_retries - 1:"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAP3%2F%2Fp1I%3D","line":4113,"in_reply_to":"AAAAP3%2F%2Fq7c%3D","updated":"2013-07-15 08:37:41.000000000","message":"This method does the following:\n\n1. Read existing keys.\n\n2. Update existing keys.\n\n3. Insert new keys.\n\nIf concurrent transaction insert the same key w/o unique constraint a duplicate record is created. With unique constraint exception raised, next retry on step 1 read this key as existed and it will be updated.","commit_id":"0d9cf62bfc929c7bd844b7722c380e029ffc50d4"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"85950becdd34646a919bf5fb35009bba6e781542","unresolved":false,"context_lines":[{"line_number":4113,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":4114,"context_line":"            # a concurrent transaction has been committed,"},{"line_number":4115,"context_line":"            # try again unless this was the last attempt"},{"line_number":4116,"context_line":"            if attempt \u003d\u003d max_retries - 1:"},{"line_number":4117,"context_line":"                raise"},{"line_number":4118,"context_line":""},{"line_number":4119,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAP3%2F%2Fq7Y%3D","line":4116,"updated":"2013-07-14 11:55:16.000000000","message":"why not do the raise outside of the loop?","commit_id":"0d9cf62bfc929c7bd844b7722c380e029ffc50d4"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"3a20e1621069d8c782929b452ad075533bdc625a","unresolved":false,"context_lines":[{"line_number":4113,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":4114,"context_line":"            # a concurrent transaction has been committed,"},{"line_number":4115,"context_line":"            # try again unless this was the last attempt"},{"line_number":4116,"context_line":"            if attempt \u003d\u003d max_retries - 1:"},{"line_number":4117,"context_line":"                raise"},{"line_number":4118,"context_line":""},{"line_number":4119,"context_line":""}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAP3%2F%2Fp0g%3D","line":4116,"in_reply_to":"AAAAP3%2F%2Fq7Y%3D","updated":"2013-07-15 08:37:41.000000000","message":"We must pass exception max_retries times.","commit_id":"0d9cf62bfc929c7bd844b7722c380e029ffc50d4"}],"nova/tests/db/test_db_api.py":[{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"9aec642ecfff7b525433387d50eb5f66fcf91b23","unresolved":false,"context_lines":[{"line_number":2600,"context_line":"        db.instance_type_extra_specs_update_or_create(self.ctxt, \u0027f1\u0027,"},{"line_number":2601,"context_line":"                                                     {\u0027x\u0027: \u0027z\u0027})"},{"line_number":2602,"context_line":"        spec \u003d db.instance_type_extra_specs_get(self.ctxt, \u0027f1\u0027)"},{"line_number":2603,"context_line":"        self.assertEqual(spec[\u0027x\u0027], \u0027z\u0027)"},{"line_number":2604,"context_line":""},{"line_number":2605,"context_line":""},{"line_number":2606,"context_line":"class InstanceTypeAccessTestCase(BaseInstanceTypeTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2FlWw%3D","line":2603,"updated":"2013-06-27 10:20:35.000000000","message":"Can we get test coverage of creating a duplicate without a race as well please?","commit_id":"33586f7ef4160dba71a6ab3dbc5f85ce8a219f25"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"7a0ca40b0159a574de001a30a318f32513657c6c","unresolved":false,"context_lines":[{"line_number":2600,"context_line":"        db.instance_type_extra_specs_update_or_create(self.ctxt, \u0027f1\u0027,"},{"line_number":2601,"context_line":"                                                     {\u0027x\u0027: \u0027z\u0027})"},{"line_number":2602,"context_line":"        spec \u003d db.instance_type_extra_specs_get(self.ctxt, \u0027f1\u0027)"},{"line_number":2603,"context_line":"        self.assertEqual(spec[\u0027x\u0027], \u0027z\u0027)"},{"line_number":2604,"context_line":""},{"line_number":2605,"context_line":""},{"line_number":2606,"context_line":"class InstanceTypeAccessTestCase(BaseInstanceTypeTestCase):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAPn%2F%2FlKY%3D","line":2603,"in_reply_to":"AAAAPn%2F%2FlWw%3D","updated":"2013-06-27 12:33:01.000000000","message":"This case covered in test_instance_type_extra_specs_update_or_create\na\u003d\u0027a\u0027, b\u003d\u0027b\u0027, c\u003d\u0027c\u0027 -\u003e b\u003d\u0027b1\u0027, c\u003d\u0027c1\u0027, d\u003d\u0027d1\u0027, e\u003d\u0027e1 and\nd\u003d\u0027d\u0027, e\u003d\u0027e\u0027, f\u003d\u0027f\u0027 -\u003e b\u003d\u0027b1\u0027, c\u003d\u0027c1\u0027, d\u003d\u0027d1\u0027, e\u003d\u0027e1\u0027","commit_id":"33586f7ef4160dba71a6ab3dbc5f85ce8a219f25"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"9c4491999e613fedb98da77e7a51465cd5cb2ebb","unresolved":false,"context_lines":[{"line_number":2590,"context_line":"        def safe_create(instance_type_id, specs, session):"},{"line_number":2591,"context_line":"            ref \u003d models.InstanceTypeExtraSpecs()"},{"line_number":2592,"context_line":"            ref.update({\"key\": \u0027x\u0027, \"value\": \u0027y\u0027,"},{"line_number":2593,"context_line":"                             \"instance_type_id\": instance_type_id})"},{"line_number":2594,"context_line":"            ref.save()"},{"line_number":2595,"context_line":"            return orig_safe_create(instance_type_id, specs, session)"},{"line_number":2596,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAPn%2F%2FfSU%3D","line":2593,"updated":"2013-06-30 02:43:21.000000000","message":"You should pull this quote inline with the one above.","commit_id":"0ed351949d635ab222427ede58f2bc6e39e4efae"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"9c4491999e613fedb98da77e7a51465cd5cb2ebb","unresolved":false,"context_lines":[{"line_number":2595,"context_line":"            return orig_safe_create(instance_type_id, specs, session)"},{"line_number":2596,"context_line":""},{"line_number":2597,"context_line":"        self.stubs.Set(sqlalchemy_api,"},{"line_number":2598,"context_line":"                      \u0027_instance_type_extra_specs_safe_create\u0027,"},{"line_number":2599,"context_line":"                       safe_create)"},{"line_number":2600,"context_line":"        db.instance_type_extra_specs_update_or_create(self.ctxt, \u0027f1\u0027,"},{"line_number":2601,"context_line":"                                                     {\u0027x\u0027: \u0027z\u0027})"}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAPn%2F%2FfSQ%3D","line":2598,"updated":"2013-06-30 02:43:21.000000000","message":"This should be intended to line up with the inside of the brace above.","commit_id":"0ed351949d635ab222427ede58f2bc6e39e4efae"},{"author":{"_account_id":2271,"name":"Michael Still","email":"mikal@stillhq.com","username":"mikalstill"},"change_message_id":"9c4491999e613fedb98da77e7a51465cd5cb2ebb","unresolved":false,"context_lines":[{"line_number":2598,"context_line":"                      \u0027_instance_type_extra_specs_safe_create\u0027,"},{"line_number":2599,"context_line":"                       safe_create)"},{"line_number":2600,"context_line":"        db.instance_type_extra_specs_update_or_create(self.ctxt, \u0027f1\u0027,"},{"line_number":2601,"context_line":"                                                     {\u0027x\u0027: \u0027z\u0027})"},{"line_number":2602,"context_line":"        spec \u003d db.instance_type_extra_specs_get(self.ctxt, \u0027f1\u0027)"},{"line_number":2603,"context_line":"        self.assertEqual(spec[\u0027x\u0027], \u0027z\u0027)"},{"line_number":2604,"context_line":""}],"source_content_type":"text/x-python","patch_set":4,"id":"AAAAPn%2F%2FfSM%3D","line":2601,"updated":"2013-06-30 02:43:21.000000000","message":"One more space here please.","commit_id":"0ed351949d635ab222427ede58f2bc6e39e4efae"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"05b736090a99e08efd3b729eb05843819296536f","unresolved":false,"context_lines":[{"line_number":2720,"context_line":"        get_id \u003d counted()"},{"line_number":2721,"context_line":"        self.stubs.Set(sqlalchemy_api,"},{"line_number":2722,"context_line":"                       \u0027_instance_type_get_id_from_flavor\u0027, get_id)"},{"line_number":2723,"context_line":"        try:"},{"line_number":2724,"context_line":"            sqlalchemy_api.\\"},{"line_number":2725,"context_line":"               instance_type_extra_specs_update_or_create(self.ctxt, 1, {}, 5)"},{"line_number":2726,"context_line":"        except db_exc.DBDuplicateEntry:"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAP3%2F%2Ft7I%3D","line":2723,"updated":"2013-07-12 14:30:25.000000000","message":"maybe it makes sense to use assertRaises() instead of try/except here?","commit_id":"ce202726a389dceedc0fb2f7441caf7f4f8e85c6"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"7028600c8f890927eeb6fff4cf820e3641614ae3","unresolved":false,"context_lines":[{"line_number":2720,"context_line":"        get_id \u003d counted()"},{"line_number":2721,"context_line":"        self.stubs.Set(sqlalchemy_api,"},{"line_number":2722,"context_line":"                       \u0027_instance_type_get_id_from_flavor\u0027, get_id)"},{"line_number":2723,"context_line":"        try:"},{"line_number":2724,"context_line":"            sqlalchemy_api.\\"},{"line_number":2725,"context_line":"               instance_type_extra_specs_update_or_create(self.ctxt, 1, {}, 5)"},{"line_number":2726,"context_line":"        except db_exc.DBDuplicateEntry:"}],"source_content_type":"text/x-python","patch_set":11,"id":"AAAAP3%2F%2FtTA%3D","line":2723,"in_reply_to":"AAAAP3%2F%2Ft7I%3D","updated":"2013-07-12 17:06:08.000000000","message":"Done","commit_id":"ce202726a389dceedc0fb2f7441caf7f4f8e85c6"},{"author":{"_account_id":1653,"name":"garyk","email":"gkotton@vmware.com","username":"garyk"},"change_message_id":"85950becdd34646a919bf5fb35009bba6e781542","unresolved":false,"context_lines":[{"line_number":2708,"context_line":"                          db.instance_type_extra_specs_update_or_create,"},{"line_number":2709,"context_line":"                          self.ctxt, \u0027nonexists\u0027, {})"},{"line_number":2710,"context_line":""},{"line_number":2711,"context_line":"    def test_instance_type_extra_specs_update_or_create_retry(self):"},{"line_number":2712,"context_line":""},{"line_number":2713,"context_line":"        def counted():"},{"line_number":2714,"context_line":"            def get_id(context, flavorid, session):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAP3%2F%2Fq7U%3D","line":2711,"updated":"2013-07-14 11:55:16.000000000","message":"is there a bug open with this issue?","commit_id":"0d9cf62bfc929c7bd844b7722c380e029ffc50d4"},{"author":{"_account_id":7711,"name":"Yuriy Zveryanskyy","email":"yzveryanskyy@mirantis.com","username":"yuriyz"},"change_message_id":"3a20e1621069d8c782929b452ad075533bdc625a","unresolved":false,"context_lines":[{"line_number":2708,"context_line":"                          db.instance_type_extra_specs_update_or_create,"},{"line_number":2709,"context_line":"                          self.ctxt, \u0027nonexists\u0027, {})"},{"line_number":2710,"context_line":""},{"line_number":2711,"context_line":"    def test_instance_type_extra_specs_update_or_create_retry(self):"},{"line_number":2712,"context_line":""},{"line_number":2713,"context_line":"        def counted():"},{"line_number":2714,"context_line":"            def get_id(context, flavorid, session):"}],"source_content_type":"text/x-python","patch_set":12,"id":"AAAAP3%2F%2Fp0A%3D","line":2711,"in_reply_to":"AAAAP3%2F%2Fq7U%3D","updated":"2013-07-15 08:37:41.000000000","message":"An existent code is used, only max_retries added. This is test for new functionality.","commit_id":"0d9cf62bfc929c7bd844b7722c380e029ffc50d4"}]}