Chromium Code Reviews
Help | Chromium Project | Sign in
(651)

Issue 5542002: Switch from gsdview.appspot.com to commondatastorage.googleapis.com. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 4 months ago by davidjames
Modified:
2 years, 8 months ago
CC:
chromium-os-reviews_chromium.org, Mandeep Singh Baines, anush, sosa
Visibility:
Public.

Description

Switch from gsdview.appspot.com to commondatastorage.googleapis.com.

This server requires that unsafe characters are quoted in URLs, so we do that
here.

Change-Id: I6b7c430671b02a8da11777029b0fdb59a87dcae1

BUG=chromium-os:9902
TEST=Ran unit tests. Ran example buildbot run and diff'd new version and old
version of Packages file.

Patch Set 1 #

Patch Set 2 : 80 chars #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+13 lines, -10 lines) Lint Patch
M chromite/lib/binpkg.py View 3 chunks +4 lines, -2 lines 0 comments ? errors Download
M prebuilt.py View 1 chunk +1 line, -2 lines 1 comment ? errors Download
M prebuilt_unittest.py View 1 4 chunks +8 lines, -6 lines 0 comments ? errors Download
Trybot results:
Commit:

Messages

Total messages: 4
davidjames
3 years, 4 months ago #1
diandersAtChromium
I assume that the point of this change is to somehow make the prebuilts available ...
3 years, 4 months ago #2
sosa
LGTM
3 years, 4 months ago #3
davidjames
3 years, 4 months ago #4
On 2010/12/02 23:03:21, diandersAtChromium wrote:
> I assume that the point of this change is to somehow make the prebuilts
> available to those outside of Google?  Is that right?

No -- the prebuilts are already available publicly.

When we retrieve prebuilts, we currently go to gsdview.appspot.com, which then
redirects to commondatastorage.googleapis.com. In logs, we've seen that
gsdview.appspot.com sometimes fails, so we think that switching to going to
commondatastorage.googleapis.com directly will get more reliable performance. In
order to do this, though, we need to do the same escaping that gsdview does for
us.

> * Did you test the code path that prevents the "deduplication" stuff from
> running when the host changes?  This will be the first time that uses it,
right?

Yup, I did test this locally. Might be good to add a unit test. I'll think about
this.

> * Did you test to make sure that private packages won't be uploaded to the
> public servers?

This change doesn't change the uploading, so this part should be good.

Cheers,

David
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1280:2d3e6564b7b6