Bundle validation: verify with charms. #70
Conversation
|
Test PASSed. |
| } | ||
| return nil, err | ||
| } | ||
| r, size, err := h.openBlob(url) |
There was a problem hiding this comment.
I don't think it's necessary to open the blob. All the information is available in the
Entity document. I'd define a type, say entityCharm, which implements the
methods required by the charm.Charm interface.
e.g.
type entityCharm mongodoc.Entity
func (e *entityCharm) Meta() *charm.Meta {
return e.CharmMeta
}
... etc
Then you can just retrieve all the entities from the entities collection with the
appropriate ids (entities.Find(bson.D{{"_id", bson.D{{"$in", ids}}}}).All(&found)
might work ok) put them into a map and return them as is.
|
Excellent direction, thanks, but with some suggested simplifications above. |
|
Test PASSed. |
|
Test PASSed. |
| func (h *handler) bundleCharms(ids []string) (map[string]charm.Charm, error) { | ||
| urls := make([]*charm.Reference, len(ids)) | ||
| urlIdmap := make(map[charm.Reference]string, len(ids)) | ||
| for i, id := range ids { |
There was a problem hiding this comment.
// TODO resolve ids concurrently
?
|
Test PASSed. |
|
Test PASSed. |
| } | ||
|
|
||
| func (h *handler) bundleCharms(ids []string) (map[string]charm.Charm, error) { | ||
| urls := make([]*charm.Reference, len(ids)) |
There was a problem hiding this comment.
This doesn't seem quite right. If URL resolving fails, we can be left
with empty slots here.
I'd suggest:
urls := make([]*charm.Reference, 0, len(ids))
and then append to urls after parsing each one.
It would be good to have some better test coverage in this area. Perhaps
a unit test of bundleCharms itself?
|
Nicer, thanks. A few further suggestions above. |
|
👍 |
|
Test PASSed. |
|
Test PASSed. |
| @@ -208,3 +214,82 @@ func (h *handler) openBlob(id *charm.Reference) (blobstore.ReadSeekCloser, int64 | |||
| } | |||
| return r, size, nil | |||
| } | |||
|
|
|||
| // entityCharm implements charm.Charm. | |||
There was a problem hiding this comment.
perhaps move this (and the methods) after bundleCharms?
|
LGTM with a few minor suggestions, thanks! |
|
Test PASSed. |
|
|
thank you for the great reviews!
|
Status: merge request accepted. Url: http://ci.jujugui.org:8080/job/charmstore-merge |
Bundle validation: verify with charms. Validate the bundle with respect to charms in the store.
Validate the bundle with respect to charms in the store.