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

Issue 93500044: state/apiserver/common: Resources.RegisterNamed

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 11 months ago by jameinel
Modified:
9 years, 11 months ago
Reviewers:
dimitern, mp+220208
Visibility:
Public.

Description

state/apiserver/common: Resources.RegisterNamed Two small changes here. 1) Change common.Resources to allow you to RegisterNamed rather than only allowing you to register and get back the Id. This was necessary for the second step, but generally allows us to declare Resources that Facades can know about that aren't reported to clients. 2) Change the pingTimeout object from being an attribute of srvRoot into being just another resource like the Watchers at a specific resource name. While working on the changes for pingTimeout, I noticed that no tests actually failed if I started returning nullTimeout. So I added a test that ensures calling Ping() actually does delay the connection from being closed. (We only had a test that if you never called Ping in time, it would die, but not that Ping can delay that death.) This is a step towards making all of the Facades follow a common pattern, as this allows Pinger to also just take Resources rather than needing to know about details of the srvRoot object. https://code.launchpad.net/~jameinel/juju-core/api-named-resources/+merge/220208 (do not edit description out of merge proposal)

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+118 lines, -13 lines) Patch
A [revision details] View 1 chunk +2 lines, -0 lines 0 comments Download
M state/apiserver/admin.go View 1 chunk +2 lines, -1 line 0 comments Download
M state/apiserver/common/resource.go View 2 chunks +21 lines, -0 lines 0 comments Download
M state/apiserver/common/resource_test.go View 3 chunks +50 lines, -0 lines 0 comments Download
M state/apiserver/pinger_test.go View 1 chunk +19 lines, -0 lines 0 comments Download
M state/apiserver/root.go View 4 chunks +8 lines, -11 lines 0 comments Download
M state/apiserver/root_test.go View 2 chunks +16 lines, -1 line 0 comments Download

Messages

Total messages: 2
jameinel
Please take a look.
9 years, 11 months ago (2014-05-20 08:26:06 UTC) #1
dimitern
9 years, 11 months ago (2014-05-20 09:51:35 UTC) #2
LGTM
Sign in to reply to this message.

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