-
Notifications
You must be signed in to change notification settings - Fork 11
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
HeadersCollection usability #208
Comments
Thanks for reporting this, is this something you'd be willing to take on? |
It's easy enough, yes, I can pick it up, what would be the preferred solution? |
We'd need something that feels "natural" in python. new RequestHeaders {
{"foo": "bar"}
}; which feels natural to a dotnet developer In Go, that's not supported, so we have two constructors (one empty, one that accepts a map) It needs to feel like a map/dictionary API wise. |
I have been taking a closer look at this, and ... I'm not sure 😢 . Having Other than that, Looking for guidance on how to proceed here, or a Python expert that can show me the way 🙂 . |
From you initial post it sounded like all the changes you wanted to make were additional. |
I thought that the changes were only additional ... but when attempting to implement it I found out that either we duplicate the In either case, since the |
Could we imagine having:
Thoughts? |
Your proposal makes a lot of sense coming from Java and C#, although I notice that some different tradeoffs are applied in this codebase. Also, your proposal is an API breaking change(we cannot remove a public field), how would we roll it out? |
I don't think it's breaking if the getter and the setters are using the same name as the public field we have today. which seems possible through descriptors but I'm no python expert. @samwelkanda what's your opinion on all that? |
Hey folks, Similar example with query params: query_params = TestRequestBuilder.TestRequestBuilderGetQueryParameters(limit=1, page=0)
config = RequestConfiguration(query_parameters=query_params, headers=HeadersCollection()) could be simplified by directly providing the query params as dictionary config = RequestConfiguration(query_parameters={limit:1, page:0}, headers=HeadersCollection()) Just my2c here as user, hope they are valuable inputs :) |
As demonstrated in microsoft/kiota#3962 I think we are facing a usability regression.
I don't have a strong opinion on where to go from here but either:
get_all
that accepts also a plainDict
, or similar.cc. @baywet
The text was updated successfully, but these errors were encountered: