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

Issue 6878052: rpc: new package

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 3 months ago by rog
Modified:
11 years, 1 month ago
Reviewers:
dimitern, mp+138089, fwereade, TheMue
Visibility:
Public.

Description

rpc: new package The rpc package is styled after the standard Go rpc package, but is a little more flexible and closer to our requirements. We also change state/api to use this package, so there's an idea of how it will look in practice. https://code.launchpad.net/~rogpeppe/juju-core/176-rpc-spike/+merge/138089 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : state/api, rpc: proof-of-concept spike #

Patch Set 3 : state/api, rpc: proof-of-concept spike #

Patch Set 4 : state/api, rpc: proof-of-concept spike #

Patch Set 5 : rpc: new package #

Patch Set 6 : rpc: new package #

Total comments: 18

Patch Set 7 : rpc: new package #

Patch Set 8 : rpc: new package #

Total comments: 1

Patch Set 9 : rpc: new package #

Patch Set 10 : rpc: new package #

Patch Set 11 : rpc: new package #

Patch Set 12 : rpc: new package #

Patch Set 13 : rpc: new package #

Total comments: 30

Patch Set 14 : rpc: new package #

Patch Set 15 : rpc: new package #

Unified diffs Side-by-side diffs Delta from patch set Stats (+989 lines, -103 lines) Patch
A [revision details] View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +2 lines, -0 lines 0 comments Download
M cmd/jujud/machine_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +8 lines, -4 lines 0 comments Download
M environs/jujutest/livetests.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +3 lines, -1 line 0 comments Download
A rpc/client.go View 1 2 3 1 chunk +62 lines, -0 lines 0 comments Download
A rpc/rpc.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +222 lines, -0 lines 0 comments Download
A rpc/rpc_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +249 lines, -0 lines 0 comments Download
A rpc/server.go View 1 2 3 4 5 6 7 1 chunk +193 lines, -0 lines 0 comments Download
M state/api/api_test.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +22 lines, -13 lines 0 comments Download
A state/api/apiclient.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +56 lines, -0 lines 0 comments Download
A state/api/apiserver.go View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +41 lines, -0 lines 0 comments Download
M state/api/client.go View 1 2 3 4 3 chunks +55 lines, -23 lines 0 comments Download
M state/api/server.go View 1 2 3 6 chunks +76 lines, -62 lines 0 comments Download

Messages

Total messages: 13
rog
Please take a look.
11 years, 2 months ago (2013-01-30 08:14:15 UTC) #1
rog
Please take a look.
11 years, 2 months ago (2013-01-30 08:17:47 UTC) #2
dimitern
LGTM, with a few suggestions. https://codereview.appspot.com/6878052/diff/6002/rpc/rpc.go File rpc/rpc.go (right): https://codereview.appspot.com/6878052/diff/6002/rpc/rpc.go#newcode23 rpc/rpc.go:23: type Server struct { ...
11 years, 2 months ago (2013-01-30 08:29:48 UTC) #3
TheMue
So far looking good, some remarks and questions. But I like the direction. https://codereview.appspot.com/6878052/diff/6002/rpc/rpc.go File ...
11 years, 2 months ago (2013-01-30 08:46:39 UTC) #4
rog
Please take a look. https://codereview.appspot.com/6878052/diff/6002/rpc/rpc.go File rpc/rpc.go (right): https://codereview.appspot.com/6878052/diff/6002/rpc/rpc.go#newcode23 rpc/rpc.go:23: type Server struct { On ...
11 years, 2 months ago (2013-01-30 10:07:02 UTC) #5
TheMue
LGTM
11 years, 2 months ago (2013-01-30 10:18:39 UTC) #6
fwereade
NOT LGTM until we've talked about this. https://codereview.appspot.com/6878052/diff/8012/state/api/apiclient.go File state/api/apiclient.go (right): https://codereview.appspot.com/6878052/diff/8012/state/api/apiclient.go#newcode25 state/api/apiclient.go:25: func (m ...
11 years, 2 months ago (2013-01-30 16:58:07 UTC) #7
rog
Please take a look.
11 years, 2 months ago (2013-01-30 18:25:22 UTC) #8
rog
Please take a look.
11 years, 1 month ago (2013-02-08 08:37:17 UTC) #9
fwereade
Despite reservations, LGTM assuming consideration of the following. https://codereview.appspot.com/6878052/diff/28001/cmd/jujud/machine_test.go File cmd/jujud/machine_test.go (right): https://codereview.appspot.com/6878052/diff/28001/cmd/jujud/machine_test.go#newcode231 cmd/jujud/machine_test.go:231: instId, ...
11 years, 1 month ago (2013-02-12 12:42:01 UTC) #10
rog
Please take a look. https://codereview.appspot.com/6878052/diff/28001/cmd/jujud/machine_test.go File cmd/jujud/machine_test.go (right): https://codereview.appspot.com/6878052/diff/28001/cmd/jujud/machine_test.go#newcode231 cmd/jujud/machine_test.go:231: instId, err := st.Machine(m.Id()).InstanceId() On ...
11 years, 1 month ago (2013-02-12 13:53:03 UTC) #11
fwereade
LGTM, thanks for the responses and for the patience.
11 years, 1 month ago (2013-02-12 13:58:40 UTC) #12
rog
11 years, 1 month ago (2013-02-12 14:16:13 UTC) #13
*** Submitted:

rpc: new package

The rpc package is styled after the standard
Go rpc package, but is a little more flexible
and closer to our requirements.

We also change state/api to use this
package, so there's an idea of how it
will look in practice.

R=dimitern, TheMue, fwereade
CC=
https://codereview.appspot.com/6878052
Sign in to reply to this message.

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