OpenStack Compute (nova)

Merge lp://qastaging/~anso/nova/ec2-security-groups into lp://qastaging/~soren/nova/ec2-security-groups

  1. ec2-security-groups
  2. Merge into ec2-security-groups
Proposed by Vish Ishaya
Status: Merged
Merged at revision: 312
Proposed branch: lp://qastaging/~anso/nova/ec2-security-groups
Merge into: lp://qastaging/~soren/nova/ec2-security-groups
Prerequisite: lp://qastaging/~hudson-openstack/nova/trunk
Diff against target: 346 lines (+69/-58)
6 files modified
nova/api/ec2/cloud.py (+14/-16)
nova/db/api.py (+1/-1)
nova/db/sqlalchemy/api.py (+31/-23)
nova/db/sqlalchemy/models.py (+18/-13)
nova/tests/virt_unittest.py (+1/-1)
nova/virt/libvirt_conn.py (+4/-4)
To merge this branch: bzr merge lp://qastaging/~anso/nova/ec2-security-groups
Reviewer Review Type Date Requested Status
Devin Carlen (community) Approve
Review via email: mp+37030@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2010-09-29.

Description of the change

Fixes deleted objects showing in relations and a few issues with too many rules getting revoked.

To post a comment you must log in.
Revision history for this message
Devin Carlen (devcamcar) wrote : Posted in a previous version of this proposal

Looks good except the syntax of the primaryjoins and secondaryjoins is strange. What's with the double quotes? It looks unnecessary based on the sqlalchemy docs, which gives this example of using primary join:

primaryjoin=and_(users_table.c.user_id==addresses_table.c.user_id,
                addresses_table.c.city=='Boston'))

Examples from patch:

219 + secondaryjoin="and_(SecurityGroup.id == SecurityGroupInstanceAssociation.security_group_id,"
220 + "Instance.id == SecurityGroupInstanceAssociation.instance_id,"
221 + "SecurityGroup.deleted == False,"
222 + "Instance.deleted == False)",

and

231 + primaryjoin="and_(SecurityGroupIngressRule.parent_group_id == SecurityGroup.id,"
232 + "SecurityGroupIngressRule.deleted == False)")

review: Needs Information
Revision history for this message
Vish Ishaya (vishvananda) wrote : Posted in a previous version of this proposal

the primary and secondary were wrong, the quotes is fine though. If you surround it in quotes, you can refer to tables that haven't been defined yet or are in the process of being defined. Reproposing.

Revision history for this message
Vish Ishaya (vishvananda) wrote :

This version acually works! :(

Revision history for this message
Devin Carlen (devcamcar) wrote :

+ instance_filter = 'nova-instance-%s' % instance_ref['name']

should be

+ instance_filter = 'nova-instance-%s' % instance_ref['ec2_id']

review: Needs Fixing
Revision history for this message
Devin Carlen (devcamcar) wrote :

Talked with Vish, name is correct, lgtm

review: Approve
lp://qastaging/~anso/nova/ec2-security-groups updated
319. By Vish Ishaya

show project ids for groups instead of user ids

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches