untested or excessive dict comprehension

[hougangliu]

in keystone/contrib/federation/utils.py
        def extract_groups(groups_by_domain):
            for groups in groups_by_domain.values():
                for group in {g['name']: g for g in groups}.values(): >>>>>>>>>>>>>> this is invalid syntax in python 2.6
                    yield group

        # initialize the group_ids as a set to eliminate duplicates
        user_name = None

[Boris Bobrov (bbobrov)]

Keystone doesn't support py26 any more, so it is acceptable

[hougangliu (liuhoug)]

I think "for group in {g['name']: g for g in groups}.values():" is too fancy.

why not "for group in groups:"

[Boris Bobrov (bbobrov)]

Yep, that looks weird.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/150079

[Boris Bobrov (bbobrov)]

I'm not sure if the patch above is required. There is a chance that the comprehension is there for a reason, but it seems to be untested.

[Marek Denis (marek-denis)]

The reason for adding this 'fancy' comprehension is that for input [{'name': 'a'}, {'name':'b'}, {'name': 'a'}] we want to remove duplicates so the output would be: ['{'name': 'a'}, {'name': 'b'}].

What tests are lacking?

[Boris Bobrov (bbobrov)]

> What tests are lacking?

If I remove the comprehension and leave it as 'for group in groups:' it doesn't fail tests. Thus the comprehension is not really required. Thus for nice code's sake it can be removed. Right?

If not, the comprehension removal should fail tests.

[Dolph Mathews (dolph)]

I've abandoned the referenced changed due to inactivity and failing tests. If this is still an issue, please reset the status.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Dolph Mathews (<email address hidden>) on branch: master
Review: https://review.openstack.org/150079
Reason: Abandoning due to inactivity and failing tests.

[Morgan Fainberg (mdrnstm)]

Marking as invalid since this should have expired as incomplete long ago.