Merge lp://staging/~tealeg/landscape-client/keystone-scoped-resynch into lp://staging/~landscape/landscape-client/trunk

Proposed by Geoff Teale
Status: Merged
Approved by: Geoff Teale
Approved revision: 714
Merged at revision: 708
Proposed branch: lp://staging/~tealeg/landscape-client/keystone-scoped-resynch
Merge into: lp://staging/~landscape/landscape-client/trunk
Prerequisite: lp://staging/~tealeg/landscape-client/centralise-resynch-handling
Diff against target: 125 lines (+60/-18)
3 files modified
landscape/manager/keystonetoken.py (+3/-3)
landscape/manager/tests/test_keystonetoken.py (+44/-15)
landscape/tests/helpers.py (+13/-0)
To merge this branch: bzr merge lp://staging/~tealeg/landscape-client/keystone-scoped-resynch
Reviewer Review Type Date Requested Status
Jerry Seutter (community) Approve
Chris Glass (community) Approve
Review via email: mp+174289@code.staging.launchpad.net

This proposal supersedes a proposal from 2013-07-05.

Commit message

This branch makes the keystone token plugin only respond
to resynchronize-clients events when they are either
"openstack" or global scope. Note, the server doesn't
currently send "openstack" scope, but I decided that all
plugins should have their scope defined so that we can
easily add scoped resynchs on the server when we need to.

Description of the change

This branch makes the keystone token plugin only respond to resynchronize-clients events when they are either "openstack" or global scope. Note, the server doesn't currently send "openstack" scope, but I decided that all plugins should have their scope defined so that we can easily add scoped resynchs on the server when we need to.

Note - this branch has now been re-based on the centralise-resynch-handling and will need re-review on that basis.

To post a comment you must log in.
Revision history for this message
Jerry Seutter (jseutter) wrote : Posted in a previous version of this proposal

+1, looks good.

review: Approve
Revision history for this message
Jerry Seutter (jseutter) wrote : Posted in a previous version of this proposal

Some lintian issues:

landscape/manager/tests/test_keystonetoken.py:9:1: E303 too many blank lines (3)
landscape/manager/tests/test_keystonetoken.py:125:80: E501 line too long (87 characters)
landscape/broker/tests/test_server.py:368:80: E501 line too long (80 characters)

Revision history for this message
Geoff Teale (tealeg) wrote : Posted in a previous version of this proposal

> Some lintian issues:
>
> landscape/manager/tests/test_keystonetoken.py:9:1: E303 too many blank lines
> (3)
> landscape/manager/tests/test_keystonetoken.py:125:80: E501 line too long (87
> characters)
> landscape/broker/tests/test_server.py:368:80: E501 line too long (80
> characters)

Cheers, all dealt with.

Revision history for this message
Chris Glass (tribaal) wrote : Posted in a previous version of this proposal

[1]
+class FakePersist(object):
+
+ def __init__(self):
+ self.called = False
+
+ def remove(self, key):
+ self.called = True

This should probably go to someplace else, like landscape/lib/persist or landscape/tests/helpers as I assume it can be reused by all other plugins using the persist mechanism?

(ongoing review, saving just because I'm afraid chromium will die on me again)

Revision history for this message
Chris Glass (tribaal) wrote : Posted in a previous version of this proposal

Alright, +1!

review: Approve
Revision history for this message
Geoff Teale (tealeg) wrote :

Chris, I addressed your previous review point.

Revision history for this message
Chris Glass (tribaal) wrote :

Looks good! Thanks!

review: Approve
Revision history for this message
Jerry Seutter (jseutter) wrote :

+1

landscape/tests/helpers.py:655:1: W391 blank line at end of file

review: Approve
Revision history for this message
🤖 Landscape Builder (landscape-builder) wrote :

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

to all changes: