From bd99ae82d1640a9885e8fe54c181a0167e98b35a Mon Sep 17 00:00:00 2001 From: Ville Aikas Date: Wed, 3 Feb 2016 09:57:48 -0800 Subject: [PATCH] address CR comments --- dm/dm.go | 9 +++++---- manager/manager/typeresolver.go | 22 ++++++++++------------ registry/registryprovider.go | 4 ++-- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/dm/dm.go b/dm/dm.go index 099e108a2..14edb977e 100644 --- a/dm/dm.go +++ b/dm/dm.go @@ -39,9 +39,10 @@ import ( ) var ( - deployment_name = flag.String("name", "", "Name of deployment, used for deploy and update commands (defaults to template name)") - stdin = flag.Bool("stdin", false, "Reads a configuration from the standard input") - properties = flag.String("properties", "", "Properties to use when deploying a template (e.g., --properties k1=v1,k2=v2)") + deployment_name = flag.String("name", "", "Name of deployment, used for deploy and update commands (defaults to template name)") + stdin = flag.Bool("stdin", false, "Reads a configuration from the standard input") + properties = flag.String("properties", "", "Properties to use when deploying a template (e.g., --properties k1=v1,k2=v2)") + // TODO(vaikas): CHange the default name once we figure out where the charts live. template_registry = flag.String("registry", "application-dm-templates", "Registry name") service = flag.String("service", "http://localhost:8001/api/v1/proxy/namespaces/dm/services/manager-service:manager", "URL for deployment manager") binary = flag.String("binary", "../expandybird/expansion/expansion.py", "Path to template expansion binary") @@ -202,7 +203,7 @@ func execute() { panic(fmt.Errorf("Failed to create a registry from arguments: %#v", err)) } path := fmt.Sprintf("registries/%s", args[1]) - callService(path, "POST", "set registry", ioutil.NopCloser(bytes.NewReader(reg))) + callService(path, "POST", "create registry", ioutil.NopCloser(bytes.NewReader(reg))) case "get": if len(args) < 2 { fmt.Fprintln(os.Stderr, "No deployment name supplied") diff --git a/manager/manager/typeresolver.go b/manager/manager/typeresolver.go index d7a2a8155..afc2eb9fe 100644 --- a/manager/manager/typeresolver.go +++ b/manager/manager/typeresolver.go @@ -45,8 +45,8 @@ type typeResolver struct { } type fetchableURL struct { - registry registry.Registry - url string + reg registry.Registry + url string } type fetchUnit struct { @@ -64,10 +64,8 @@ func resolverError(c *common.Configuration, err error) error { } func (tr *typeResolver) performHTTPGet(d util.HTTPDoer, u string, allowMissing bool) (content string, err error) { - var g util.HTTPClient - if d == nil { - g = tr.c - } else { + g := tr.c + if d != nil { g = util.NewHTTPClient(3, d, util.NewSleeper()) } r, code, err := g.Get(u) @@ -102,7 +100,7 @@ func (tr *typeResolver) ResolveTypes(config *common.Configuration, imports []*co toFetch := make([]*fetchUnit, 0, tr.maxUrls) for _, r := range config.Resources { // Map the type to a fetchable URL (if applicable) or skip it if it's a non-fetchable type (primitive for example). - urls, registry, err := registry.GetDownloadURLs(tr.rp, r.Type) + urls, urlRegistry, err := registry.GetDownloadURLs(tr.rp, r.Type) if err != nil { return nil, resolverError(config, fmt.Errorf("Failed to understand download url for %s: %v", r.Type, err)) } @@ -110,7 +108,7 @@ func (tr *typeResolver) ResolveTypes(config *common.Configuration, imports []*co f := &fetchUnit{} for _, u := range urls { if len(u) > 0 { - f.urls = append(f.urls, fetchableURL{registry, u}) + f.urls = append(f.urls, fetchableURL{urlRegistry, u}) // Add to existing map so it is not fetched multiple times. existing[r.Type] = true } @@ -140,7 +138,7 @@ func (tr *typeResolver) ResolveTypes(config *common.Configuration, imports []*co templates := []string{} url := toFetch[0].urls[0] for _, u := range toFetch[0].urls { - template, err := tr.performHTTPGet(u.registry, u.url, false) + template, err := tr.performHTTPGet(u.reg, u.url, false) if err != nil { return nil, resolverError(config, err) } @@ -156,7 +154,7 @@ func (tr *typeResolver) ResolveTypes(config *common.Configuration, imports []*co } schemaURL := url.url + schemaSuffix - sch, err := tr.performHTTPGet(url.registry, schemaURL, true) + sch, err := tr.performHTTPGet(url.reg, schemaURL, true) if err != nil { return nil, resolverError(config, err) } @@ -170,7 +168,7 @@ func (tr *typeResolver) ResolveTypes(config *common.Configuration, imports []*co for _, v := range s.Imports { i := &common.ImportFile{Name: v.Name} var existingSchema string - urls, registry, conversionErr := registry.GetDownloadURLs(tr.rp, v.Path) + urls, urlRegistry, conversionErr := registry.GetDownloadURLs(tr.rp, v.Path) if conversionErr != nil { return nil, resolverError(config, fmt.Errorf("Failed to understand download url for %s: %v", v.Path, conversionErr)) } @@ -182,7 +180,7 @@ func (tr *typeResolver) ResolveTypes(config *common.Configuration, imports []*co for _, u := range urls { if len(fetched[u]) == 0 { // If this import URL is new to us, add it to the URLs to fetch. - toFetch = append(toFetch, &fetchUnit{[]fetchableURL{fetchableURL{registry, u}}}) + toFetch = append(toFetch, &fetchUnit{[]fetchableURL{fetchableURL{urlRegistry, u}}}) } else { // If this is not a new import URL and we've already fetched its contents, // reuse them. Also, check if we also found a schema for that import URL and diff --git a/registry/registryprovider.go b/registry/registryprovider.go index 53cb0165d..c4120834f 100644 --- a/registry/registryprovider.go +++ b/registry/registryprovider.go @@ -163,7 +163,7 @@ type githubRegistryProvider struct { // NewGithubRegistryProvider creates a GithubRegistryProvider. func NewGithubRegistryProvider(cp common.CredentialProvider) GithubRegistryProvider { if cp == nil { - panic(fmt.Errorf("cp is nil: %v", cp)) + panic(fmt.Errorf("no credential provider")) } return &githubRegistryProvider{cp: cp} } @@ -234,7 +234,7 @@ type gcsRegistryProvider struct { // NewGCSRegistryProvider creates a GCSRegistryProvider. func NewGCSRegistryProvider(cp common.CredentialProvider) GCSRegistryProvider { if cp == nil { - panic(fmt.Errorf("cp is nil: %v", cp)) + panic(fmt.Errorf("no credential provider")) } return &gcsRegistryProvider{cp: cp} }