From 1ad5106b08d122d877c8c800c6cab9c30a598c16 Mon Sep 17 00:00:00 2001 From: vaikas-google Date: Fri, 20 Nov 2015 14:31:56 -0500 Subject: [PATCH] address code review comments --- manager/manager/deployer.go | 8 +++----- manager/manager/manager.go | 11 +++++------ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/manager/manager/deployer.go b/manager/manager/deployer.go index 3acfcedf3..0b86310b6 100644 --- a/manager/manager/deployer.go +++ b/manager/manager/deployer.go @@ -38,7 +38,7 @@ type Deployer interface { // NewDeployer returns a new initialized Deployer. // TODO(vaikas): Add a flag for setting the timeout. func NewDeployer(url string) Deployer { - return &deployer{url, 5} + return &deployer{url, 15} } type deployer struct { @@ -126,7 +126,7 @@ func (d *deployer) callServiceWithConfiguration(method, operation string, config return nil, fmt.Errorf("cannot unmarshal response: (%v)", err) } } - return result, err + return result, nil } func (d *deployer) callService(method, url string, reader io.Reader, callback formatter) ([]byte, error) { @@ -140,9 +140,7 @@ func (d *deployer) callService(method, url string, reader io.Reader, callback fo } timeout := time.Duration(time.Duration(d.timeout) * time.Second) - client := http.Client{ - Timeout: timeout, - } + client := http.Client{Timeout: timeout} response, err := client.Do(request) if err != nil { return nil, callback(err) diff --git a/manager/manager/manager.go b/manager/manager/manager.go index 8c534086c..c115d182d 100644 --- a/manager/manager/manager.go +++ b/manager/manager/manager.go @@ -104,9 +104,8 @@ func (m *manager) CreateDeployment(t *Template) (*Deployment, error) { return nil, err } - actualConfig, err := m.deployer.CreateConfiguration(manifest.ExpandedConfig) - log.Printf("Got Back %s", actualConfig) - if err != nil { + actualConfig, createErr := m.deployer.CreateConfiguration(manifest.ExpandedConfig) + if createErr != nil { // Deployment failed, mark as failed log.Printf("CreateConfiguration failed: %v", err) m.repository.SetDeploymentStatus(t.Name, FailedStatus) @@ -114,7 +113,7 @@ func (m *manager) CreateDeployment(t *Template) (*Deployment, error) { // return the failure as such. Otherwise, we're going to add the manifest // and hence resource specific errors down below. if actualConfig == nil { - return nil, err + return nil, createErr } } else { m.repository.SetDeploymentStatus(t.Name, DeployedStatus) @@ -131,8 +130,8 @@ func (m *manager) CreateDeployment(t *Template) (*Deployment, error) { // to a check fail (either deployment doesn't exist, or a manifest with the same // name already exists). // TODO(vaikas): Should we combine both errors and return a nicely formatted error for both? - if err != nil { - return nil, err + if createErr != nil { + return nil, createErr } else { return nil, aErr }