Skip to content

Conversation

@dungdm93
Copy link
Contributor

@dungdm93 dungdm93 commented May 5, 2025

Merge after #163

@dungdm93
Copy link
Contributor Author

dungdm93 commented May 5, 2025

cc @chris-ruecker, @anmoel

@anmoel
Copy link
Member

anmoel commented May 8, 2025

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)
Copy link
Contributor Author

@dungdm93 dungdm93 May 12, 2025

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
}

Copy link
Member

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.

Copy link
Contributor Author

@dungdm93 dungdm93 Jun 13, 2025

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants