microsoft / kiota-abstractions-go

Abstractions library for the Kiota generated SDKs in go
https://aka.ms/kiota/docs
MIT License
11 stars 10 forks source link

request configuration lack a default implementation #178

Open baywet opened 2 months ago

baywet commented 2 months ago

if you try to create a request configuration for an endpoint that does not document any query parameters, we don't provide an implementing type with the default query parameters, making it really hard to use it. Found out while working on https://github.com/microsoft/kiota/issues/4995

rkodev commented 1 month ago

@baywet for clarity on this issue we do have an implementation of the request configuration. To instantiate a request configuration with no query parameters

requestConfig := &RequestConfiguration[DefaultQueryParameters]{}

could you kindly add some clarity

baywet commented 1 month ago

Sorry about the confusing issue. I'm having difficulties remembering myself.

It was either:

I guess what I was trying to do is to disable the compression for a single request, which didn't have any query parameters, and that should be doable via.

requestConfig := &RequestConfiguration[DefaultQueryParameters]{
 Options: [NewCompressionOption()]
}
client.Foo().Bar().Get(context, requestConfig)

(or something like that) Which isn't terrible.

I guess if we can confirm that endpoint with no query parameters get generated with RequestConfiguration[DefaultQueryParameters] for the request configuration parameter, and the ones that do have query parameters RequestConfiguration[CustomQueryParameters] we should be good to close this issue.

rkodev commented 1 month ago

I can confirm that we do not use the generic struct RequestConfiguration on any of the classes, might be because of the performance hit in compilation we took when we tried introducing generics a while back? not sure.

On second try, the only bit I can find that might need improving experience for a dev , who is trying to add a none documented parameter to the request, is the need to create a custom struct and remember to add the uriparametername tag , for parameters that need encoding.

To simplify this, we can add a method AddQueryParametersFromMap on RequestInformation that takes map[string]interface{} as a parameter users can pass the map of query options directly.

Users should be able to use this constructor below

requestConfig := &RequestConfiguration[map[string]interface{}]{}
baywet commented 1 month ago

Thank you for the additional information.

I can confirm that we do not use the generic struct RequestConfiguration on any of the classes, might be because of the performance hit in compilation we took when we tried introducing generics a while back? not sure.

Even with the --ebc flag?

To simplify this, we can add a method AddQueryParametersFromMap on RequestInformation that takes map[string]interface{} as a parameter users can pass the map of query options directly.

Having them revert to the request information means they now need to figure out how to execute the request information. Unless we get strong indications this is what people want, I'd avoid it for now.