)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c84c5a9d4ea6ec3fa9c6af5c885451155b3c4f43","unresolved":false,"context_lines":[{"line_number":17,"context_line":""},{"line_number":18,"context_line":"* _set_traits is made public"},{"line_number":19,"context_line":"* classes are put before methods, public methods before private"},{"line_number":20,"context_line":"* a direct import of sqlalchemy\u0027s null() is replace by using"},{"line_number":21,"context_line":"  that already imported sa.null(). It\u0027s the same thing."},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"Change-Id: Ibe11ebd7ccc97a46586edd6c897b173ed67ca134"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"5fc1f717_e05b882b","line":21,"range":{"start_line":20,"start_character":44,"end_line":21,"end_character":6},"updated":"2019-03-06 23:17:51.000000000","message":"replaced by using the ?","commit_id":"ac14d1bb1ba385e4d2f44892e8edf18a8dcd4583"}],"placement/deploy.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"d8a794b64ab994f757793d523fa11989c4f97922","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from placement import handler"},{"line_number":23,"context_line":"from placement import microversion"},{"line_number":24,"context_line":"from placement.objects import resource_class"},{"line_number":25,"context_line":"from placement.objects import trait"},{"line_number":26,"context_line":"from placement import requestlog"},{"line_number":27,"context_line":"from placement import resource_class_cache as rc_cache"},{"line_number":28,"context_line":"from placement import util"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_4274bb68","line":25,"updated":"2019-03-08 14:08:41.000000000","message":"nit: in many places we use the name \u0027trait\u0027 to refer to a single trait, and \u0027trait_obj\u0027 to refer to the trait object module. I know it isn\u0027t critical here, but it would seem that keep a standard naming convention throughout the code might make things clearer.","commit_id":"830552dc86f152520050fe5be5be5fee5cf34319"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"424eec9ec56760494886ba1977018d88d4115009","unresolved":false,"context_lines":[{"line_number":22,"context_line":"from placement import handler"},{"line_number":23,"context_line":"from placement import microversion"},{"line_number":24,"context_line":"from placement.objects import resource_class"},{"line_number":25,"context_line":"from placement.objects import trait"},{"line_number":26,"context_line":"from placement import requestlog"},{"line_number":27,"context_line":"from placement import resource_class_cache as rc_cache"},{"line_number":28,"context_line":"from placement import util"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_9d120609","line":25,"in_reply_to":"5fc1f717_4274bb68","updated":"2019-03-08 14:47:49.000000000","message":"I tried to stick with the existing naming pattern in whatever module I was making changes in. I agree that being consistent globally would be nice, but I didn\u0027t want to create local inconsistency to achieve that, now. Should be easy to do in a later patch.\n\nHowever, I really don\u0027t like the *_obj name, much in the same way that I think the objects package name is no longer a good fit. So until we figure out that whole thing, staying the course seems right?","commit_id":"830552dc86f152520050fe5be5be5fee5cf34319"}],"placement/handlers/trait.py":[{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"d8a794b64ab994f757793d523fa11989c4f97922","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    except exception.TraitExists:"},{"line_number":89,"context_line":"        # Get the trait that already exists to get last-modified time."},{"line_number":90,"context_line":"        if want_version.matches((1, 15)):"},{"line_number":91,"context_line":"            trait \u003d trait_obj.Trait.get_by_name(context, name)"},{"line_number":92,"context_line":"        req.response.status \u003d 204"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    req.response.content_type \u003d None"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_e20847da","line":91,"range":{"start_line":91,"start_character":20,"end_line":91,"end_character":47},"updated":"2019-03-08 14:08:41.000000000","message":"Nit: now that the Trait class is in its own module, this could be a module-level method rather than a classmethod. Not critical, just less verbose: \n\n  trait \u003d trait_obj.get_by_name(context, name)","commit_id":"830552dc86f152520050fe5be5be5fee5cf34319"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"424eec9ec56760494886ba1977018d88d4115009","unresolved":false,"context_lines":[{"line_number":88,"context_line":"    except exception.TraitExists:"},{"line_number":89,"context_line":"        # Get the trait that already exists to get last-modified time."},{"line_number":90,"context_line":"        if want_version.matches((1, 15)):"},{"line_number":91,"context_line":"            trait \u003d trait_obj.Trait.get_by_name(context, name)"},{"line_number":92,"context_line":"        req.response.status \u003d 204"},{"line_number":93,"context_line":""},{"line_number":94,"context_line":"    req.response.content_type \u003d None"}],"source_content_type":"text/x-python","patch_set":5,"id":"5fc1f717_ddd64e28","line":91,"range":{"start_line":91,"start_character":20,"end_line":91,"end_character":47},"in_reply_to":"5fc1f717_e20847da","updated":"2019-03-08 14:47:49.000000000","message":"Yes, I agree, but was on a different goal path in this set of changes. There will always be (at least I hope so) more opportunities for changes as we continue.\n\nWe will eventually reach zero code.","commit_id":"830552dc86f152520050fe5be5be5fee5cf34319"}],"placement/objects/trait.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c84c5a9d4ea6ec3fa9c6af5c885451155b3c4f43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5fc1f717_40e61c47","updated":"2019-03-06 23:17:51.000000000","message":"✔","commit_id":"ac14d1bb1ba385e4d2f44892e8edf18a8dcd4583"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c84c5a9d4ea6ec3fa9c6af5c885451155b3c4f43","unresolved":false,"context_lines":[{"line_number":23,"context_line":"from placement import db_api"},{"line_number":24,"context_line":"from placement import exception"},{"line_number":25,"context_line":"from placement.objects import common as common_obj"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"_RP_TRAIT_TBL \u003d models.ResourceProviderTrait.__table__"},{"line_number":28,"context_line":"_TRAIT_TBL \u003d models.Trait.__table__"},{"line_number":29,"context_line":"_TRAIT_LOCK \u003d \u0027trait_sync\u0027"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_20d7504e","line":26,"updated":"2019-03-06 23:17:51.000000000","message":"nit: add a newline","commit_id":"ac14d1bb1ba385e4d2f44892e8edf18a8dcd4583"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c84c5a9d4ea6ec3fa9c6af5c885451155b3c4f43","unresolved":false,"context_lines":[{"line_number":172,"context_line":"        return cls._set_objects(context, db_traits)"},{"line_number":173,"context_line":""},{"line_number":174,"context_line":""},{"line_number":175,"context_line":"def ensure_trait_sync(ctx):"},{"line_number":176,"context_line":"    \"\"\"Ensures that the os_traits library is synchronized to the traits db."},{"line_number":177,"context_line":""},{"line_number":178,"context_line":"    If _TRAITS_SYNCED is False then this process has not tried to update the"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_c0cbeca2","line":175,"range":{"start_line":175,"start_character":4,"end_line":175,"end_character":21},"updated":"2019-03-06 23:17:51.000000000","message":"nit: could rename to ensure_sync since we\u0027re in trait module.","commit_id":"ac14d1bb1ba385e4d2f44892e8edf18a8dcd4583"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"e3c5a74c017e1afa1752c068c0e282506b89a35c","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            _TRAITS_SYNCED \u003d True"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"@db_api.placement_context_manager.writer"},{"line_number":201,"context_line":"def set_traits(context, rp, traits):"},{"line_number":202,"context_line":"    \"\"\"Given a ResourceProvider object and a TraitList object, replaces the set"},{"line_number":203,"context_line":"    of traits associated with the resource provider."},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    :raises: ConcurrentUpdateDetected if the resource provider\u0027s traits or"},{"line_number":206,"context_line":"             inventory was changed in between the time when we first started to"},{"line_number":207,"context_line":"             set traits and the end of this routine."},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    :param rp: The ResourceProvider object to set traits against"},{"line_number":210,"context_line":"    :param traits: A TraitList object or list of Trait objects"},{"line_number":211,"context_line":"    \"\"\""},{"line_number":212,"context_line":"    # Get the internal IDs of our existing traits"},{"line_number":213,"context_line":"    existing_traits \u003d _get_traits_by_provider_id(context, rp.id)"},{"line_number":214,"context_line":"    existing_traits \u003d set(rec[\u0027id\u0027] for rec in existing_traits)"},{"line_number":215,"context_line":"    want_traits \u003d set(trait.id for trait in traits)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    to_add \u003d want_traits - existing_traits"},{"line_number":218,"context_line":"    to_delete \u003d existing_traits - want_traits"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    if not to_add and not to_delete:"},{"line_number":221,"context_line":"        return"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    if to_delete:"},{"line_number":224,"context_line":"        _delete_traits_from_provider(context, rp.id, to_delete)"},{"line_number":225,"context_line":"    if to_add:"},{"line_number":226,"context_line":"        _add_traits_to_provider(context, rp.id, to_add)"},{"line_number":227,"context_line":"    rp.increment_generation()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"def _add_traits_to_provider(ctx, rp_id, to_add):"},{"line_number":231,"context_line":"    \"\"\"Adds trait associations to the provider with the supplied ID."},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    :param ctx: `placement.context.RequestContext` that has an oslo_db Session"},{"line_number":234,"context_line":"    :param rp_id: Internal ID of the resource provider on which to add"},{"line_number":235,"context_line":"                  trait associations"},{"line_number":236,"context_line":"    :param to_add: set() containing internal trait IDs for traits to add"},{"line_number":237,"context_line":"    \"\"\""},{"line_number":238,"context_line":"    for trait_id in to_add:"},{"line_number":239,"context_line":"        try:"},{"line_number":240,"context_line":"            ins_stmt \u003d _RP_TRAIT_TBL.insert().values("},{"line_number":241,"context_line":"                resource_provider_id\u003drp_id,"},{"line_number":242,"context_line":"                trait_id\u003dtrait_id)"},{"line_number":243,"context_line":"            ctx.session.execute(ins_stmt)"},{"line_number":244,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":245,"context_line":"            # Another thread already set this trait for this provider. Ignore"},{"line_number":246,"context_line":"            # this for now (but ConcurrentUpdateDetected will end up being"},{"line_number":247,"context_line":"            # raised almost assuredly when we go to increment the resource"},{"line_number":248,"context_line":"            # provider\u0027s generation later, but that\u0027s also fine)"},{"line_number":249,"context_line":"            pass"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"def _delete_traits_from_provider(ctx, rp_id, to_delete):"},{"line_number":253,"context_line":"    \"\"\"Deletes trait associations from the provider with the supplied ID and"},{"line_number":254,"context_line":"    set() of internal trait IDs."},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    :param ctx: `placement.context.RequestContext` that has an oslo_db Session"},{"line_number":257,"context_line":"    :param rp_id: Internal ID of the resource provider from which to delete"},{"line_number":258,"context_line":"                  trait associations"},{"line_number":259,"context_line":"    :param to_delete: set() containing internal trait IDs for traits to"},{"line_number":260,"context_line":"                      delete"},{"line_number":261,"context_line":"    \"\"\""},{"line_number":262,"context_line":"    del_stmt \u003d _RP_TRAIT_TBL.delete().where("},{"line_number":263,"context_line":"        sa.and_("},{"line_number":264,"context_line":"            _RP_TRAIT_TBL.c.resource_provider_id \u003d\u003d rp_id,"},{"line_number":265,"context_line":"            _RP_TRAIT_TBL.c.trait_id.in_(to_delete)))"},{"line_number":266,"context_line":"    ctx.session.execute(del_stmt)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"@oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_de273a64","line":266,"range":{"start_line":200,"start_character":0,"end_line":266,"end_character":33},"updated":"2019-03-07 18:38:25.000000000","message":"This is borderline... I\u0027m kinda on the fence about it.\n\nI actually feel that these methods should stay in the resource_provider.py file. These methods are essentially just the implementation of the ResourceProvider.set_traits() method, which is more about the resource provider object than the trait object.\n\nConversely, I think that _get_traits_by_provider_id() below SHOULD stay in this file and be a \"public\" method -- i.e. get_traits_by_provider_id() because it\u0027s more clearly returning Trait objects...\n\nI suppose I could live with it this way, though... Ed, what are you thoughts on this?","commit_id":"198da74b9c1730d1dfd33ee7dc0e85cea2f9db3a"},{"author":{"_account_id":1063,"name":"Ed Leafe","email":"ed@leafe.com","username":"ed-leafe"},"change_message_id":"5d0ded9dad4e593bd3d0f2abe566ddf8d930ec4b","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            _TRAITS_SYNCED \u003d True"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"@db_api.placement_context_manager.writer"},{"line_number":201,"context_line":"def set_traits(context, rp, traits):"},{"line_number":202,"context_line":"    \"\"\"Given a ResourceProvider object and a TraitList object, replaces the set"},{"line_number":203,"context_line":"    of traits associated with the resource provider."},{"line_number":204,"context_line":""},{"line_number":205,"context_line":"    :raises: ConcurrentUpdateDetected if the resource provider\u0027s traits or"},{"line_number":206,"context_line":"             inventory was changed in between the time when we first started to"},{"line_number":207,"context_line":"             set traits and the end of this routine."},{"line_number":208,"context_line":""},{"line_number":209,"context_line":"    :param rp: The ResourceProvider object to set traits against"},{"line_number":210,"context_line":"    :param traits: A TraitList object or list of Trait objects"},{"line_number":211,"context_line":"    \"\"\""},{"line_number":212,"context_line":"    # Get the internal IDs of our existing traits"},{"line_number":213,"context_line":"    existing_traits \u003d _get_traits_by_provider_id(context, rp.id)"},{"line_number":214,"context_line":"    existing_traits \u003d set(rec[\u0027id\u0027] for rec in existing_traits)"},{"line_number":215,"context_line":"    want_traits \u003d set(trait.id for trait in traits)"},{"line_number":216,"context_line":""},{"line_number":217,"context_line":"    to_add \u003d want_traits - existing_traits"},{"line_number":218,"context_line":"    to_delete \u003d existing_traits - want_traits"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"    if not to_add and not to_delete:"},{"line_number":221,"context_line":"        return"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    if to_delete:"},{"line_number":224,"context_line":"        _delete_traits_from_provider(context, rp.id, to_delete)"},{"line_number":225,"context_line":"    if to_add:"},{"line_number":226,"context_line":"        _add_traits_to_provider(context, rp.id, to_add)"},{"line_number":227,"context_line":"    rp.increment_generation()"},{"line_number":228,"context_line":""},{"line_number":229,"context_line":""},{"line_number":230,"context_line":"def _add_traits_to_provider(ctx, rp_id, to_add):"},{"line_number":231,"context_line":"    \"\"\"Adds trait associations to the provider with the supplied ID."},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    :param ctx: `placement.context.RequestContext` that has an oslo_db Session"},{"line_number":234,"context_line":"    :param rp_id: Internal ID of the resource provider on which to add"},{"line_number":235,"context_line":"                  trait associations"},{"line_number":236,"context_line":"    :param to_add: set() containing internal trait IDs for traits to add"},{"line_number":237,"context_line":"    \"\"\""},{"line_number":238,"context_line":"    for trait_id in to_add:"},{"line_number":239,"context_line":"        try:"},{"line_number":240,"context_line":"            ins_stmt \u003d _RP_TRAIT_TBL.insert().values("},{"line_number":241,"context_line":"                resource_provider_id\u003drp_id,"},{"line_number":242,"context_line":"                trait_id\u003dtrait_id)"},{"line_number":243,"context_line":"            ctx.session.execute(ins_stmt)"},{"line_number":244,"context_line":"        except db_exc.DBDuplicateEntry:"},{"line_number":245,"context_line":"            # Another thread already set this trait for this provider. Ignore"},{"line_number":246,"context_line":"            # this for now (but ConcurrentUpdateDetected will end up being"},{"line_number":247,"context_line":"            # raised almost assuredly when we go to increment the resource"},{"line_number":248,"context_line":"            # provider\u0027s generation later, but that\u0027s also fine)"},{"line_number":249,"context_line":"            pass"},{"line_number":250,"context_line":""},{"line_number":251,"context_line":""},{"line_number":252,"context_line":"def _delete_traits_from_provider(ctx, rp_id, to_delete):"},{"line_number":253,"context_line":"    \"\"\"Deletes trait associations from the provider with the supplied ID and"},{"line_number":254,"context_line":"    set() of internal trait IDs."},{"line_number":255,"context_line":""},{"line_number":256,"context_line":"    :param ctx: `placement.context.RequestContext` that has an oslo_db Session"},{"line_number":257,"context_line":"    :param rp_id: Internal ID of the resource provider from which to delete"},{"line_number":258,"context_line":"                  trait associations"},{"line_number":259,"context_line":"    :param to_delete: set() containing internal trait IDs for traits to"},{"line_number":260,"context_line":"                      delete"},{"line_number":261,"context_line":"    \"\"\""},{"line_number":262,"context_line":"    del_stmt \u003d _RP_TRAIT_TBL.delete().where("},{"line_number":263,"context_line":"        sa.and_("},{"line_number":264,"context_line":"            _RP_TRAIT_TBL.c.resource_provider_id \u003d\u003d rp_id,"},{"line_number":265,"context_line":"            _RP_TRAIT_TBL.c.trait_id.in_(to_delete)))"},{"line_number":266,"context_line":"    ctx.session.execute(del_stmt)"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":""},{"line_number":269,"context_line":"@oslo_db_api.wrap_db_retry(max_retries\u003d5, retry_on_deadlock\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":2,"id":"5fc1f717_dee65ae9","line":266,"range":{"start_line":200,"start_character":0,"end_line":266,"end_character":33},"in_reply_to":"5fc1f717_de273a64","updated":"2019-03-07 18:46:31.000000000","message":"I hate to say it, but I think I agree with Jay. :)\n\nThis isn\u0027t about doing anything to traits; it\u0027s about associating/dissociating traits from the RP. So it does seem to be a better fit in the resource_provider class.","commit_id":"198da74b9c1730d1dfd33ee7dc0e85cea2f9db3a"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"7b35090b21bfdc838d4a16cf3655567a36a5778c","unresolved":false,"context_lines":[{"line_number":11,"context_line":"#    under the License."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"import collections"},{"line_number":14,"context_line":"import six"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import os_traits"},{"line_number":17,"context_line":"from oslo_concurrency import lockutils"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_8e31770e","line":14,"range":{"start_line":14,"start_character":0,"end_line":14,"end_character":10},"updated":"2019-03-08 02:52:37.000000000","message":"nit: This line should be between line 20 and 21.","commit_id":"327df9093b3d3d44cf97a010ce82c25c22e126a7"}],"placement/tests/functional/db/test_resource_provider.py":[{"author":{"_account_id":25625,"name":"Tetsuro Nakamura","email":"tetsuro.nakamura.bc@hco.ntt.co.jp","username":"tetsuro0907"},"change_message_id":"4303768b2aa339e4432a52b163f07b13ab6a94aa","unresolved":false,"context_lines":[{"line_number":1427,"context_line":"            self.ctx, filters\u003d{\"prefix\": \"NOT_EXISTED\"})"},{"line_number":1428,"context_line":"        self.assertEqual(0, len(traits))"},{"line_number":1429,"context_line":""},{"line_number":1430,"context_line":"    def test_set_traits_for_resource_provider(self):"},{"line_number":1431,"context_line":"        rp \u003d self._create_provider(\u0027fake_resource_provider\u0027)"},{"line_number":1432,"context_line":"        generation \u003d rp.generation"},{"line_number":1433,"context_line":"        self.assertIsInstance(rp.id, int)"},{"line_number":1434,"context_line":""},{"line_number":1435,"context_line":"        trait_names \u003d [\u0027CUSTOM_TRAIT_A\u0027, \u0027CUSTOM_TRAIT_B\u0027, \u0027CUSTOM_TRAIT_C\u0027]"},{"line_number":1436,"context_line":"        tb.set_traits(rp, *trait_names)"},{"line_number":1437,"context_line":""},{"line_number":1438,"context_line":"        rp_traits \u003d rp_obj.TraitList.get_all_by_resource_provider(self.ctx, rp)"},{"line_number":1439,"context_line":"        self._assert_traits(trait_names, rp_traits)"},{"line_number":1440,"context_line":"        self.assertEqual(rp.generation, generation + 1)"},{"line_number":1441,"context_line":"        generation \u003d rp.generation"},{"line_number":1442,"context_line":""},{"line_number":1443,"context_line":"        trait_names.remove(\u0027CUSTOM_TRAIT_A\u0027)"},{"line_number":1444,"context_line":"        updated_traits \u003d rp_obj.TraitList.get_all("},{"line_number":1445,"context_line":"            self.ctx, filters\u003d{\u0027name_in\u0027: trait_names})"},{"line_number":1446,"context_line":"        self._assert_traits(trait_names, updated_traits)"},{"line_number":1447,"context_line":"        tb.set_traits(rp, *trait_names)"},{"line_number":1448,"context_line":"        rp_traits \u003d rp_obj.TraitList.get_all_by_resource_provider(self.ctx, rp)"},{"line_number":1449,"context_line":"        self._assert_traits(trait_names, rp_traits)"},{"line_number":1450,"context_line":"        self.assertEqual(rp.generation, generation + 1)"},{"line_number":1451,"context_line":""},{"line_number":1452,"context_line":"    def test_set_traits_for_correct_resource_provider(self):"},{"line_number":1453,"context_line":"        \"\"\"This test creates two ResourceProviders, and attaches same trait to"},{"line_number":1454,"context_line":"        both of them. Then detaching the trait from one of them, and ensure"},{"line_number":1455,"context_line":"        the trait still associated with another one."},{"line_number":1456,"context_line":"        \"\"\""},{"line_number":1457,"context_line":"        # Create two ResourceProviders"},{"line_number":1458,"context_line":"        rp1 \u003d self._create_provider(\u0027fake_resource_provider1\u0027)"},{"line_number":1459,"context_line":"        rp2 \u003d self._create_provider(\u0027fake_resource_provider2\u0027)"},{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"        tname \u003d \u0027CUSTOM_TRAIT_A\u0027"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"        # Associate the trait with two ResourceProviders"},{"line_number":1464,"context_line":"        tb.set_traits(rp1, tname)"},{"line_number":1465,"context_line":"        tb.set_traits(rp2, tname)"},{"line_number":1466,"context_line":""},{"line_number":1467,"context_line":"        # Ensure the association"},{"line_number":1468,"context_line":"        rp1_traits \u003d rp_obj.TraitList.get_all_by_resource_provider("},{"line_number":1469,"context_line":"            self.ctx, rp1)"},{"line_number":1470,"context_line":"        rp2_traits \u003d rp_obj.TraitList.get_all_by_resource_provider("},{"line_number":1471,"context_line":"            self.ctx, rp2)"},{"line_number":1472,"context_line":"        self._assert_traits([tname], rp1_traits)"},{"line_number":1473,"context_line":"        self._assert_traits([tname], rp2_traits)"},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"        # Detach the trait from one of ResourceProvider, and ensure the"},{"line_number":1476,"context_line":"        # trait association with another ResourceProvider still exists."},{"line_number":1477,"context_line":"        tb.set_traits(rp1)"},{"line_number":1478,"context_line":"        rp1_traits \u003d rp_obj.TraitList.get_all_by_resource_provider("},{"line_number":1479,"context_line":"            self.ctx, rp1)"},{"line_number":1480,"context_line":"        rp2_traits \u003d rp_obj.TraitList.get_all_by_resource_provider("},{"line_number":1481,"context_line":"            self.ctx, rp2)"},{"line_number":1482,"context_line":"        self._assert_traits([], rp1_traits)"},{"line_number":1483,"context_line":"        self._assert_traits([tname], rp2_traits)"},{"line_number":1484,"context_line":""},{"line_number":1485,"context_line":"    def test_trait_delete_in_use(self):"},{"line_number":1486,"context_line":"        rp \u003d self._create_provider(\u0027fake_resource_provider\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5fc1f717_58dc47e3","side":"PARENT","line":1483,"range":{"start_line":1430,"start_character":0,"end_line":1483,"end_character":48},"updated":"2019-03-14 10:30:11.000000000","message":"Hmmmmm, I would keep these two tests in test_resource_provider.py. Yeah, look into what you did in [1]. You didn\u0027t move test_provider_modify_inventory which exercises rp.set_inventory. Similarly tests for rp.set_traits can be kept in test_resource_provider.py. \n\n[1] https://review.openstack.org/#/c/642111/5/placement/tests/functional/db/test_resource_provider.py","commit_id":"62a5e79a1bfe444d94ff6c9804e4aee23597e3b5"},{"author":{"_account_id":11564,"name":"Chris Dent","email":"cdent@anticdent.org","username":"chdent"},"change_message_id":"27a9d79b24155d106b16a3065340eeb848f45b07","unresolved":false,"context_lines":[{"line_number":1427,"context_line":"            self.ctx, filters\u003d{\"prefix\": \"NOT_EXISTED\"})"},{"line_number":1428,"context_line":"        self.assertEqual(0, len(traits))"},{"line_number":1429,"context_line":""},{"line_number":1430,"context_line":"    def test_set_traits_for_resource_provider(self):"},{"line_number":1431,"context_line":"        rp \u003d self._create_provider(\u0027fake_resource_provider\u0027)"},{"line_number":1432,"context_line":"        generation \u003d rp.generation"},{"line_number":1433,"context_line":"        self.assertIsInstance(rp.id, int)"},{"line_number":1434,"context_line":""},{"line_number":1435,"context_line":"        trait_names \u003d [\u0027CUSTOM_TRAIT_A\u0027, \u0027CUSTOM_TRAIT_B\u0027, \u0027CUSTOM_TRAIT_C\u0027]"},{"line_number":1436,"context_line":"        tb.set_traits(rp, *trait_names)"},{"line_number":1437,"context_line":""},{"line_number":1438,"context_line":"        rp_traits \u003d rp_obj.TraitList.get_all_by_resource_provider(self.ctx, rp)"},{"line_number":1439,"context_line":"        self._assert_traits(trait_names, rp_traits)"},{"line_number":1440,"context_line":"        self.assertEqual(rp.generation, generation + 1)"},{"line_number":1441,"context_line":"        generation \u003d rp.generation"},{"line_number":1442,"context_line":""},{"line_number":1443,"context_line":"        trait_names.remove(\u0027CUSTOM_TRAIT_A\u0027)"},{"line_number":1444,"context_line":"        updated_traits \u003d rp_obj.TraitList.get_all("},{"line_number":1445,"context_line":"            self.ctx, filters\u003d{\u0027name_in\u0027: trait_names})"},{"line_number":1446,"context_line":"        self._assert_traits(trait_names, updated_traits)"},{"line_number":1447,"context_line":"        tb.set_traits(rp, *trait_names)"},{"line_number":1448,"context_line":"        rp_traits \u003d rp_obj.TraitList.get_all_by_resource_provider(self.ctx, rp)"},{"line_number":1449,"context_line":"        self._assert_traits(trait_names, rp_traits)"},{"line_number":1450,"context_line":"        self.assertEqual(rp.generation, generation + 1)"},{"line_number":1451,"context_line":""},{"line_number":1452,"context_line":"    def test_set_traits_for_correct_resource_provider(self):"},{"line_number":1453,"context_line":"        \"\"\"This test creates two ResourceProviders, and attaches same trait to"},{"line_number":1454,"context_line":"        both of them. Then detaching the trait from one of them, and ensure"},{"line_number":1455,"context_line":"        the trait still associated with another one."},{"line_number":1456,"context_line":"        \"\"\""},{"line_number":1457,"context_line":"        # Create two ResourceProviders"},{"line_number":1458,"context_line":"        rp1 \u003d self._create_provider(\u0027fake_resource_provider1\u0027)"},{"line_number":1459,"context_line":"        rp2 \u003d self._create_provider(\u0027fake_resource_provider2\u0027)"},{"line_number":1460,"context_line":""},{"line_number":1461,"context_line":"        tname \u003d \u0027CUSTOM_TRAIT_A\u0027"},{"line_number":1462,"context_line":""},{"line_number":1463,"context_line":"        # Associate the trait with two ResourceProviders"},{"line_number":1464,"context_line":"        tb.set_traits(rp1, tname)"},{"line_number":1465,"context_line":"        tb.set_traits(rp2, tname)"},{"line_number":1466,"context_line":""},{"line_number":1467,"context_line":"        # Ensure the association"},{"line_number":1468,"context_line":"        rp1_traits \u003d rp_obj.TraitList.get_all_by_resource_provider("},{"line_number":1469,"context_line":"            self.ctx, rp1)"},{"line_number":1470,"context_line":"        rp2_traits \u003d rp_obj.TraitList.get_all_by_resource_provider("},{"line_number":1471,"context_line":"            self.ctx, rp2)"},{"line_number":1472,"context_line":"        self._assert_traits([tname], rp1_traits)"},{"line_number":1473,"context_line":"        self._assert_traits([tname], rp2_traits)"},{"line_number":1474,"context_line":""},{"line_number":1475,"context_line":"        # Detach the trait from one of ResourceProvider, and ensure the"},{"line_number":1476,"context_line":"        # trait association with another ResourceProvider still exists."},{"line_number":1477,"context_line":"        tb.set_traits(rp1)"},{"line_number":1478,"context_line":"        rp1_traits \u003d rp_obj.TraitList.get_all_by_resource_provider("},{"line_number":1479,"context_line":"            self.ctx, rp1)"},{"line_number":1480,"context_line":"        rp2_traits \u003d rp_obj.TraitList.get_all_by_resource_provider("},{"line_number":1481,"context_line":"            self.ctx, rp2)"},{"line_number":1482,"context_line":"        self._assert_traits([], rp1_traits)"},{"line_number":1483,"context_line":"        self._assert_traits([tname], rp2_traits)"},{"line_number":1484,"context_line":""},{"line_number":1485,"context_line":"    def test_trait_delete_in_use(self):"},{"line_number":1486,"context_line":"        rp \u003d self._create_provider(\u0027fake_resource_provider\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"5fc1f717_febcfb06","side":"PARENT","line":1483,"range":{"start_line":1430,"start_character":0,"end_line":1483,"end_character":48},"in_reply_to":"5fc1f717_58dc47e3","updated":"2019-03-14 10:37:56.000000000","message":"That makes sense. The set_traits method has moved back and forth between the two modules during the development of these changes and I guess the tests didn\u0027t keep up.","commit_id":"62a5e79a1bfe444d94ff6c9804e4aee23597e3b5"}],"placement/tests/functional/db/test_trait.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c84c5a9d4ea6ec3fa9c6af5c885451155b3c4f43","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"5fc1f717_006014a5","updated":"2019-03-06 23:17:51.000000000","message":"✔","commit_id":"ac14d1bb1ba385e4d2f44892e8edf18a8dcd4583"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"a13287d490d14b78bb0d3aceee85aad605b6b22f","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from placement.tests.functional.db import test_base as tb"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"class ResourceProviderTraitTestCase(tb.PlacementDbBaseTestCase):"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def _assert_traits(self, expected_traits, traits_objs):"},{"line_number":23,"context_line":"        expected_traits.sort()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_1c95cae4","line":20,"range":{"start_line":20,"start_character":6,"end_line":20,"end_character":35},"updated":"2019-03-07 20:34:22.000000000","message":"Could just be called TraitTestCase. We don\u0027t put traits on anything else. And the suite has lots of tests that don\u0027t touch providers.","commit_id":"327df9093b3d3d44cf97a010ce82c25c22e126a7"},{"author":{"_account_id":7634,"name":"Takashi Natsume","email":"takanattie@gmail.com","username":"natsumet"},"change_message_id":"7b35090b21bfdc838d4a16cf3655567a36a5778c","unresolved":false,"context_lines":[{"line_number":17,"context_line":"from placement.tests.functional.db import test_base as tb"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"class ResourceProviderTraitTestCase(tb.PlacementDbBaseTestCase):"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"    def _assert_traits(self, expected_traits, traits_objs):"},{"line_number":23,"context_line":"        expected_traits.sort()"}],"source_content_type":"text/x-python","patch_set":3,"id":"5fc1f717_0e2587c2","line":20,"range":{"start_line":20,"start_character":6,"end_line":20,"end_character":35},"in_reply_to":"5fc1f717_1c95cae4","updated":"2019-03-08 02:52:37.000000000","message":"+1","commit_id":"327df9093b3d3d44cf97a010ce82c25c22e126a7"}],"placement/tests/unit/objects/test_trait.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"c84c5a9d4ea6ec3fa9c6af5c885451155b3c4f43","unresolved":false,"context_lines":[{"line_number":18,"context_line":""},{"line_number":19,"context_line":"class TestTraits(base.TestCase):"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    @mock.patch(\"placement.objects.trait.\""},{"line_number":22,"context_line":"                \"_trait_sync\")"},{"line_number":23,"context_line":"    def test_sync_flag(self, mock_sync):"},{"line_number":24,"context_line":"        synced \u003d trait._TRAITS_SYNCED"}],"source_content_type":"text/x-python","patch_set":1,"id":"5fc1f717_c059acfe","line":21,"updated":"2019-03-06 23:17:51.000000000","message":"join lines","commit_id":"ac14d1bb1ba385e4d2f44892e8edf18a8dcd4583"}]}