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

request configuration lack a default implementation #178

Open
baywet opened this issue Jul 22, 2024 · 4 comments
Open

request configuration lack a default implementation #178

baywet opened this issue Jul 22, 2024 · 4 comments
Assignees
Labels
bug A broken experience help wanted Standard GitHub label

Comments

@baywet
Copy link
Member

baywet commented Jul 22, 2024

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 microsoft/kiota#4995

@baywet baywet added help wanted Standard GitHub label bug A broken experience labels Jul 22, 2024
@rkodev rkodev self-assigned this Aug 15, 2024
@rkodev
Copy link
Contributor

rkodev commented Aug 29, 2024

@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
Copy link
Member Author

baywet commented Aug 29, 2024

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

It was either:

  • I didn't see the default query parameters.
  • I thought that asking people to use the generic type was too complicated. (but since it's what we do for the whole API surface in the interest of size, that'd be an odd feedback)
  • Something else entirely that I forgot about.

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

rkodev commented Aug 29, 2024

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
Copy link
Member Author

baywet commented Aug 29, 2024

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.

@rkodev rkodev assigned rkodev and unassigned rkodev Aug 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A broken experience help wanted Standard GitHub label
Projects
Status: Todo 📃
Development

No branches or pull requests

2 participants