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

Issue 14218044: environs/simplestreams: fix GetMetadata

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 6 months ago by axw
Modified:
10 years, 4 months ago
Reviewers:
thumper, axw1, mp+188758, fwereade
Visibility:
Public.

Description

environs/simplestreams: fix GetMetadata GetMetadata was dropping out if a datasource returned no matching product IDs, without an error. I've changed it to check that a non- empty list of products is returned before bailing out. Fixes #1233924 https://code.launchpad.net/~axwalk/juju-core/lp1233924-simplestreams-getmetadata-fallback/+merge/188758 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 4

Patch Set 2 : environs/simplestreams: fix GetMetadata #

Total comments: 3

Patch Set 3 : environs/simplestreams: fix GetMetadata #

Patch Set 4 : environs/simplestreams: fix GetMetadata #

Unified diffs Side-by-side diffs Delta from patch set Stats (+109 lines, -42 lines) Patch
A [revision details] View 1 2 3 1 chunk +2 lines, -0 lines 0 comments Download
M environs/simplestreams/simplestreams.go View 1 2 3 2 chunks +4 lines, -3 lines 0 comments Download
M environs/simplestreams/simplestreams_test.go View 1 2 3 1 chunk +43 lines, -0 lines 0 comments Download
M environs/simplestreams/testing/testing.go View 1 2 3 1 chunk +24 lines, -0 lines 0 comments Download
M environs/tools/tools_test.go View 1 2 3 6 chunks +12 lines, -22 lines 0 comments Download
M provider/openstack/export_test.go View 1 2 3 2 chunks +2 lines, -4 lines 0 comments Download
M provider/openstack/local_test.go View 1 2 3 6 chunks +22 lines, -13 lines 0 comments Download

Messages

Total messages: 10
axw
Please take a look.
10 years, 6 months ago (2013-10-02 03:12:13 UTC) #1
thumper
https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go File environs/simplestreams/simplestreams_test.go (right): https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode300 environs/simplestreams/simplestreams_test.go:300: Series: []string{"precise"}, // never match What is the never ...
10 years, 6 months ago (2013-10-02 04:02:00 UTC) #2
axw
Please take a look.
10 years, 6 months ago (2013-10-02 04:48:50 UTC) #3
axw1
https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go File environs/simplestreams/simplestreams_test.go (right): https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode300 environs/simplestreams/simplestreams_test.go:300: Series: []string{"precise"}, // never match On 2013/10/02 04:02:00, thumper ...
10 years, 6 months ago (2013-10-02 05:07:52 UTC) #4
axw1
On 2013/10/02 05:07:52, axw1 wrote: > https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go > File environs/simplestreams/simplestreams_test.go (right): > > https://codereview.appspot.com/14218044/diff/1/environs/simplestreams/simplestreams_test.go#newcode300 > ...
10 years, 6 months ago (2013-10-02 07:21:38 UTC) #5
thumper
> So, after discussing with jam, GetMetadata *might* be how it is on purpose. You ...
10 years, 6 months ago (2013-10-02 21:23:31 UTC) #6
thumper
LGTM https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams_test.go File environs/simplestreams/simplestreams_test.go (right): https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams_test.go#newcode327 environs/simplestreams/simplestreams_test.go:327: c.Assert(source.count, gc.Equals, 2*len(sources)) Much nicer.
10 years, 6 months ago (2013-10-02 21:24:54 UTC) #7
fwereade
Fine with the idea, I think the implementation's in the wrong place. https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams.go File environs/simplestreams/simplestreams.go ...
10 years, 5 months ago (2013-10-04 10:26:41 UTC) #8
axw
Please take a look. https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams.go File environs/simplestreams/simplestreams.go (right): https://codereview.appspot.com/14218044/diff/6001/environs/simplestreams/simplestreams.go#newcode406 environs/simplestreams/simplestreams.go:406: if err == nil && ...
10 years, 5 months ago (2013-10-10 07:05:40 UTC) #9
axw
10 years, 4 months ago (2013-11-05 04:40:29 UTC) #10
Please take a look.
Sign in to reply to this message.

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