-
Notifications
You must be signed in to change notification settings - Fork 184
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
Http[Request/Response]Data contract classes design discussion #122
Comments
I find it amazing that no-one from the worker team has replied to this. I'd like to know the rationale for the introduction of those classes as well. Just seeking to understand as it does feel like a strange design decision from my outside perspective. |
Short answer is now that we are running out of process, we’re working with a representation of the http request that came into the function app, not the actual http request itself. We want to be explicit that that’s the case. It also means we don’t have the full capabilities of an http request, so a separate type helps us expose only the functionality that is available. @drub0y’s feedback is valid and we’ll be looking at expanding HttpRequestData’s surface area to address some of those concerns. /cc @fabiocav |
@anthonychu I have some methods that map results into the desired IActionResult and use that for both Azure functions and App Services that share the same libraries. Currently IActionResult is just being treated as a POCO and the response is returned like this:
This wouldn't be much of a problem if I could just toss the code and value into the HttpResponseData, but as mentioned above it only accepts a string value. Because of this I must now create another mapper that is also going to handle the serialization, content-type, etc. I would much rather have the serialization managed/configured globally for the function in this scenario and suppose I can probably now do that with my own middleware. Is this something that will be supported in the final v1 without me needing to build custom middleware or mappers? I was hoping Functions/AspNet would move closer together, and with direct access to the host builder this is a huge step forward, but it looks like this new direction is a big reset and will take quite a while to reach feature parity with the experiences and bindings many of us are used to. I am very excited for the possibilities this approach opens up, but disappointed it is going to take quite a bit of time before it gets to where it needs to be and is holding back .Net 5 adoption for many of us. Hopefully this can be open sourced soon so others are able to pitch in and contribute :) |
Adding a quick comment here so this doesn't go unanswered, but will be sharing more, in depth, details on this very soon A note about the current To add some context, the out-of-proc model is not new in Functions, it is the model that has been used by all of the other stacks, except for .NET. It unfortunately does not expose the rich set of features we need in order to provide the HTTP experience we want to have with the worker from day one, and imposes severe limitations on a lot of the scenarios supported by the HTTP functionality built into .NET, which is something customers would expect. So instead of hacking things around to make it work with the existing HTTP types and having to provide caveats on a number of things wouldn't work as expected (which would lead to a lot of bad surprises and frustration), we are following the same approach used by the other language workers and providing a representation of the HTTP request and response in a way that is very clear that this is not compatible with what you're used to in .NET/ASP.NET. We of course want to make sure those types are as flexible and performant as possible for what they're intended to do, and will be enhancing them prior to GA, as mentioned above. The good thing about bringing this model to .NET is that we're investing in the out-of-proc integration layer to bring the enhancements required to support the proper HTTP types with all of the capabilities users would expect, which not only will bring back all of the behavior we currently have in .NET, but improve other language experiences as well. As you've pointed out, a lot of work and time have gone into the existing HTTP stack design and implementation in .NET, and we have no intention to reinvent the wheel. Our approach at the moment is to ensure that we can still handle straight forward HTTP workload requirements without having to wait for the significant amount of work that needs to be done before we light up the rich experience we want, and do so in a way that is clear to customers and won't be impacted by the enhancements we have planned. With the current and future models, we won't be pinning ourselves against any specific versions of HTTP types (built in .NET types or our data representations). Updates and new versions will be released as Nuget packages (different extension versions) putting customers in control of when they want to upgrade and giving us the ability to adopt new versions as soon as they become available. We'll be sharing more as we go along, but I hope this helps better understand the current approach and what's on the roadmap for HTTP. |
As I wasn't able to see the actual implementation/code of
Because those metadata information is frequently used, I think it should be implemented to |
Transferred the issue to the worker repo so we can continue updating here. The first wave of changes towards the design shared above is in PR right now. Still in draft, as it is being iterated on. |
Preface: As the title notes, this is probably more of a discussion than an issue, but it doesn't look like discussions are enabled on this repo yet... which is understandable since it's only just recently (i.e. 3 days ago) gone into public beta. 😊
In looking at the samples and what immediately jumped out at me as someone who utilizes HTTP triggered functions the majority of the time is the introduction of two new data contract classes to represent the request and response:
HttpRequestData
andHttpResponseData
.First, I would like to ask that we could have some kind of short explanation of why these classes were introduced. I believe it's because the SDK does not want to pin itself to a specific version of
System.Net.Http
, is that correct? If so, would you care to expand on why you think it's a bad idea to tie a specific extension to another dependency? Frankly, I think it would be a big mistake to force new types on .NET developers in the context of an Azure Function when in so many other contextsSystem.Net.Http
is what we all use and which itself has an extension ecosystem built around it.Second, if it comes down to decoupling, well then I have concerns about the design of these contracts. For example, the
Body
being astring
: this is dangerous for performance on multiple levels. First and most basic, even if I actually do want to return astring
... I shouldn't have to build a whole contiguousstring
like that. Second, if I don't want to return astring
and want to return binary data... what option do I have now? I would have to return base-64 encoded bodies and that's just, well, not a good idea from an HTTP perspective and could completely box out some standard HTTP scenarios like file downloads or serving images. I would also suggest that theHeaders
property being defined as the concrete typeDictionary<string, string>
and always allocated even if not used is a terrible idea from a performance perspective. Anyway, we can split off and go into deeper on into this in separate issues once we have the core discussion established, but the point is that so much of this is already well thought out, designed and implemented into the existingSystem.Net.Http
type system and I think it's a mistake to eschew that for an extension where I'm very much opting in to using HTTP in the first place.The text was updated successfully, but these errors were encountered: