Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(172)

Issue 1557223002: Make UseRemote work for dev_appserver too. (Closed)

Created:
4 years, 11 months ago by iannucci
Modified:
4 years, 11 months ago
Reviewers:
dnj, Vadim Sh., estaab, martiniss
CC:
chromium-reviews, infra-reviews+luci-gae_chromium.org
Base URL:
https://github.com/luci/gae.git@master
Target Ref:
refs/heads/master
Project:
luci-gae
Visibility:
Public.

Description

Make UseRemote work for dev_appserver too. Now interacting with remote_api is as simple as: ```go ctx := context.Background() if err := prod.UseRemote(&ctx, "localhost:8080", nil); err != nil { panic(err) } // ctx is usable with luci/gae ``` Also changes the context-derivation function signature to be safer, since the client's context will always be valid, and can be used for things, such as logging even in the error handling case. This also makes UseRemote work when used FROM appengine to another appengine app. Fixes #36. R=dnj@chromium.org, estaab@chromium.org, martiniss@chromium.org, vadimsh@chromium.org BUG= Committed: https://github.com/luci/gae/commit/ed44bee34120a74ccb1a6b026752fd905b2517e1

Patch Set 1 #

Patch Set 2 : Add support for creating remote_api connections FROM appengine #

Unified diffs Side-by-side diffs Delta from patch set Stats (+68 lines, -16 lines) Patch
M impl/prod/context.go View 1 4 chunks +68 lines, -16 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
iannucci
4 years, 11 months ago (2016-01-05 01:42:04 UTC) #1
martiniss
lgtm so far... but you said you would be adding tests, right? Not sure where ...
4 years, 11 months ago (2016-01-05 18:13:57 UTC) #2
iannucci
On 2016/01/05 at 18:13:57, martiniss wrote: > lgtm so far... but you said you would ...
4 years, 11 months ago (2016-01-05 19:22:26 UTC) #3
iannucci
On 2016/01/05 at 19:22:26, iannucci wrote: > On 2016/01/05 at 18:13:57, martiniss wrote: > > ...
4 years, 11 months ago (2016-01-05 19:24:13 UTC) #5
dnj
lgtm
4 years, 11 months ago (2016-01-05 21:16:45 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1557223002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1557223002/20001
4 years, 11 months ago (2016-01-05 22:49:51 UTC) #9
commit-bot: I haz the power
4 years, 11 months ago (2016-01-05 22:52:15 UTC) #11
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://github.com/luci/gae/commit/ed44bee34120a74ccb1a6b026752fd905b2517e1

Powered by Google App Engine
This is Rietveld 408576698