-
Notifications
You must be signed in to change notification settings - Fork 42
add Context counterparts methods for Client #161
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
why do you move the creation of ctx.background from net.http package into our client? what is the benefit? |
| } | ||
|
|
||
| func (s *RepositoryService[R]) Create(repo R) error { | ||
| return s.CreateContext(context.Background(), repo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@anmoel Because it's shorter than duplicate code with CreateContext like this:
func (s *RepositoryService[R]) Create(ctx context.Context, repo R) error {
data, err := tools.JsonMarshalInterfaceToIOReader(repo)
if err != nil {
return err
}
body, resp, err := s.client.Post(s.endpoint, data)
if err != nil {
return err
}
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
return fmt.Errorf("could not create repository %v: HTTP: %d, %s", repo, resp.StatusCode, string(body))
}
return nil
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean, why you want to manage the context? in our old code, we don't manage the context. we used the http client functions without context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To support request cancellation, timeout and so on.
An example use-case is terraform-provider-nexus: All Resource.Create|Delete|Exists|Read|Update methods are deprecated in favor of XContext counterparts
// Deprecated: Use CreateContext or CreateWithoutTimeout instead. This
// implementation does not support request cancellation initiated by
// Terraform, such as a system or practitioner sending SIGINT (Ctrl-c).
// This implementation also does not support warning diagnostics.
Create [CreateFunc](https://pkg.go.dev/github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema@v2.36.1#CreateFunc)
Merge after #163