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.

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

providers: Change the signature of NewRequest to accept body as interface{} #1053

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

jhrozek
Copy link
Contributor

@jhrozek jhrozek commented Sep 29, 2023

The providers.REST interface defined a NewRequest() method that was
implemented by the generic HTTP REST provider as well as the github REST
provider. However, the two implementations treat the body of the request
a bit differently - the HTTP REST provider treats the body as a generic
io.Reader which was also exposed in the high level interface, but the
github NewRequest treats the body as an interface that can be
JSON-encoded.

And because io.Reader doesn't really encode to JSON easily, but expects to be
able to infer the types
from the
interface it's encoding, I think it's better to just expose interface{} in
the high-level provider interface and let the lower-level implementation deal
with the conversion. This way the caller would know to pass e.g.
map[string]any instead of plain io.Reader.

If we want to keep the reader in the high level interface, then another
option might be to encode the io.Reader into map[string]any in the
RestClient.NewRequest method.

…face{}

The providers.REST interface defined a NewRequest() method that was
implemented by the generic HTTP REST provider as well as the github REST
provider. However, the two implementations treat the body of the request
a bit differently - the HTTP REST provider treats the body as a generic
`io.Reader` which was also exposed in the high level interface, but the
github NewRequest treats the body as an interface that can be
JSON-encoded.

And because io.Reader doesn't really encode to JSON easily, but [expects to be
able to infer the types](https://pkg.go.dev/encoding/json#Marshal) from the
interface it's encoding, I think it's better to just expose `interface{}` in
the high-level provider interface and let the lower-level implementation deal
with the conversion. This way the caller would know to pass e.g.
`map[string]any` instead of plain io.Reader.

If we want to keep the reader in the high level interface, then another
option might be to encode the io.Reader into map[string]any in the
`RestClient.NewRequest` method.
@jhrozek jhrozek merged commit 098edd3 into stacklok:main Sep 29, 2023
12 checks passed
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.

2 participants