Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update service_dependency.go to accept a context.Context #293

Merged
merged 1 commit into from
Mar 15, 2021
Merged

Update service_dependency.go to accept a context.Context #293

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
78 changes: 71 additions & 7 deletions service_dependency.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,41 +25,105 @@ type ListServiceDependencies struct {
}

// ListBusinessServiceDependencies lists dependencies of a business service.
// It's recommended to use ListBusinessServiceDependenciesWithContext instead.
func (c *Client) ListBusinessServiceDependencies(businessServiceID string) (*ListServiceDependencies, *http.Response, error) {
resp, err := c.get(context.TODO(), "/service_dependencies/business_services/"+businessServiceID)
return c.listBusinessServiceDependenciesWithContext(context.Background(), businessServiceID)
}

// ListBusinessServiceDependenciesWithContext lists dependencies of a business service.
func (c *Client) ListBusinessServiceDependenciesWithContext(ctx context.Context, businessServiceID string) (*ListServiceDependencies, error) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stmcallister let me know what you think of these methods not including the *http.Response. As far as I can tell there isn't a need for the consumer to have the response.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe I included *http.Response so that folks could get information about the Response, status codes and messages, etc. But, it sounds like the context will provide some of this functionality?

Copy link
Collaborator Author

@theckman theckman Mar 10, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stmcallister Not the context, I think the new APIError type we created will take care of that. Since that should contain all errors in the JSON response, as well as the HTTP status code. If the call returns a nil error value, consumers should assume it's a 2xx response code. If the error value is non-nil, then they can assume it was >299 and inspect the actual error if they really want to know.

The way the *http.Response support is implemented today consumers could only get the status code and HTTP headers, but they cannot read the HTTP response body. By default the http.Response.Body can only be read once, which we do in decodeJSON. Attempts to read it again by callers would result in an io.EOF error, and so the *http.Response we return today isn't very useful, especially because there don't appear to be any documented HTTP response headers that consumers would care about.

We could fix that bug and make the *http.Response useful for consumers, but since there are no documented response headers and we handle the HTTP status code (for failures) in the APIError type, I think we should lean-away from returning the *http.Response and potentially include its removal in our breaking changes in v1.5, since the returned *http.Response never fully worked.

I'm tracking the removal here: #305

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, okay. That makes sense. Thanks for the explanation.

lsd, _, err := c.listBusinessServiceDependenciesWithContext(ctx, businessServiceID)
return lsd, err
}

func (c *Client) listBusinessServiceDependenciesWithContext(ctx context.Context, businessServiceID string) (*ListServiceDependencies, *http.Response, error) {
resp, err := c.get(ctx, "/service_dependencies/business_services/"+businessServiceID)
if err != nil {
return nil, nil, err
}

var result ListServiceDependencies
return &result, resp, c.decodeJSON(resp, &result)
if err = c.decodeJSON(resp, &result); err != nil {
return nil, resp, err
}

return &result, resp, nil
}

// ListTechnicalServiceDependencies lists dependencies of a technical service.
// It's recommended to use ListTechnicalServiceDependenciesWithContext instead.
func (c *Client) ListTechnicalServiceDependencies(serviceID string) (*ListServiceDependencies, *http.Response, error) {
resp, err := c.get(context.TODO(), "/service_dependencies/technical_services/"+serviceID)
return c.listTechnicalServiceDependenciesWithContext(context.Background(), serviceID)
}

// ListTechnicalServiceDependenciesWithContext lists dependencies of a technical service.
func (c *Client) ListTechnicalServiceDependenciesWithContext(ctx context.Context, serviceID string) (*ListServiceDependencies, error) {
lsd, _, err := c.listTechnicalServiceDependenciesWithContext(ctx, serviceID)
return lsd, err
}

func (c *Client) listTechnicalServiceDependenciesWithContext(ctx context.Context, serviceID string) (*ListServiceDependencies, *http.Response, error) {
resp, err := c.get(ctx, "/service_dependencies/technical_services/"+serviceID)
if err != nil {
return nil, nil, err
}

var result ListServiceDependencies
return &result, resp, c.decodeJSON(resp, &result)
if err = c.decodeJSON(resp, &result); err != nil {
return nil, resp, err
}

return &result, resp, nil
}

// AssociateServiceDependencies Create new dependencies between two services.
// It's recommended to use AssociateServiceDependenciesWithContext instead.
func (c *Client) AssociateServiceDependencies(dependencies *ListServiceDependencies) (*ListServiceDependencies, *http.Response, error) {
return c.associateServiceDependenciesWithContext(context.Background(), dependencies)
}

// AssociateServiceDependenciesWithContext Create new dependencies between two services.
func (c *Client) AssociateServiceDependenciesWithContext(ctx context.Context, dependencies *ListServiceDependencies) (*ListServiceDependencies, error) {
lsd, _, err := c.associateServiceDependenciesWithContext(ctx, dependencies)
return lsd, err
}

func (c *Client) associateServiceDependenciesWithContext(ctx context.Context, dependencies *ListServiceDependencies) (*ListServiceDependencies, *http.Response, error) {
resp, err := c.post(context.TODO(), "/service_dependencies/associate", dependencies, nil)
if err != nil {
return nil, nil, err
}

var result ListServiceDependencies
return &result, resp, c.decodeJSON(resp, &result)
if err = c.decodeJSON(resp, &result); err != nil {
return nil, resp, err
}

return &result, resp, nil
}

// DisassociateServiceDependencies Disassociate dependencies between two services.
func (c *Client) DisassociateServiceDependencies(dependencies *ListServiceDependencies) (*ListServiceDependencies, *http.Response, error) {
resp, err := c.post(context.TODO(), "/service_dependencies/disassociate", dependencies, nil)
return c.disassociateServiceDependenciesWithContext(context.Background(), dependencies)
}

// DisassociateServiceDependenciesWithContext Disassociate dependencies between two services.
func (c *Client) DisassociateServiceDependenciesWithContext(ctx context.Context, dependencies *ListServiceDependencies) (*ListServiceDependencies, error) {
lsd, _, err := c.disassociateServiceDependenciesWithContext(ctx, dependencies)
return lsd, err
}

// DisassociateServiceDependencies Disassociate dependencies between two services.
func (c *Client) disassociateServiceDependenciesWithContext(ctx context.Context, dependencies *ListServiceDependencies) (*ListServiceDependencies, *http.Response, error) {
resp, err := c.post(ctx, "/service_dependencies/disassociate", dependencies, nil)
if err != nil {
return nil, nil, err
}

var result ListServiceDependencies
return &result, resp, c.decodeJSON(resp, &result)
if err = c.decodeJSON(resp, &result); err != nil {
return nil, resp, err
}

return &result, resp, nil
}