Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1050)

Issue 7241062: Fix relation-ids cache coherency for hooks.

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 12 months ago by hazmat
Modified:
8 years, 12 months ago
Reviewers:
bcsaller, mp+146033
Visibility:
Public.

Description

Fix relation-ids cache coherency for hooks. Relation-ids was answering questions about relations directly from state, whilst other pieces of relation-* cli was using a hook execution cache. This led to odd races where a hook would query relation-ids and then attempt to use relation-list/etc against that relation resulting in a relation not found error. This branch also does a drive by to improve the error reporting for relation not found to include the relation id where applicable. https://code.launchpad.net/~hazmat/juju/sane-relation-ids/+merge/146033 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Fix relation-ids cache coherency for hooks. #

Total comments: 2

Patch Set 3 : Fix relation-ids cache coherency for hooks. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -22 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M juju/hooks/invoker.py View 1 4 chunks +11 lines, -6 lines 0 comments Download
M juju/hooks/protocol.py View 2 chunks +4 lines, -2 lines 0 comments Download
M juju/hooks/tests/test_invoker.py View 1 9 chunks +17 lines, -13 lines 0 comments Download
M juju/state/errors.py View 1 chunk +12 lines, -0 lines 0 comments Download
M juju/unit/tests/test_lifecycle.py View 1 2 1 chunk +3 lines, -1 line 0 comments Download

Messages

Total messages: 5
hazmat
Please take a look.
8 years, 12 months ago (2013-02-01 03:41:27 UTC) #1
hazmat
Please take a look.
8 years, 12 months ago (2013-02-01 14:17:06 UTC) #2
bcsaller
LGTM, subtle issue you're fixing there. https://codereview.appspot.com/7241062/diff/3001/juju/hooks/invoker.py File juju/hooks/invoker.py (right): https://codereview.appspot.com/7241062/diff/3001/juju/hooks/invoker.py#newcode255 juju/hooks/invoker.py:255: list(set(idents).intersection(set(self._relation_contexts)))) It doesn't ...
8 years, 12 months ago (2013-02-01 16:11:10 UTC) #3
hazmat
thanks for the review https://codereview.appspot.com/7241062/diff/3001/juju/hooks/invoker.py File juju/hooks/invoker.py (right): https://codereview.appspot.com/7241062/diff/3001/juju/hooks/invoker.py#newcode255 juju/hooks/invoker.py:255: list(set(idents).intersection(set(self._relation_contexts)))) On 2013/02/01 16:11:10, bcsaller ...
8 years, 12 months ago (2013-02-01 16:13:43 UTC) #4
hazmat
8 years, 12 months ago (2013-02-01 16:27:47 UTC) #5
*** Submitted:

Fix relation-ids cache coherency for hooks.

Relation-ids was answering questions about relations
directly from state, whilst other pieces of relation-*
cli was using a hook execution cache. This led to
odd races where a hook would query relation-ids 
and then attempt to use relation-list/etc against
that relation resulting in a relation not found error.

This branch also does a drive by to improve the error
reporting for relation not found to include the relation
id where applicable.

R=bcsaller
CC=
https://codereview.appspot.com/7241062
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b