Code Monkey home page Code Monkey logo

Comments (13)

baywet avatar baywet commented on May 18, 2024 1

@nikithauc @MIchaelMainer to keep the conversation going I put together a draft pull request with #330 so you can provide feedback.
The only thing that's missing from there are some code generation changes for the get/put/patch/post/delete async method to accept another callback allowing consumers to override default middleware options for a single request. (planning to add that tomorrow)
Let me know what you think! And I'll replicate on Java/TypeScript.

from kiota.

baywet avatar baywet commented on May 18, 2024

@darrelmiller to add additional details to why he thinks this approach is a bad idea.

from kiota.

baywet avatar baywet commented on May 18, 2024

Solution:

  • no abstractions for responses, middlewares etc, that's tied to the http client
  • move the generic middlewares from graph core to kiota http
  • take a dependency on kiota abstractions, kiota serialization, kiota http and kiota authentication in graph core
  • (for dotnet) remove the graph core http provider model (deprecated)

from kiota.

nikithauc avatar nikithauc commented on May 18, 2024

Here are my preliminary thoughts when I think of two separate libraries - a Kiota core and Graph core.

  • With Kiota core, what is the plan to maintain and support issues which are reported for non Graph API's?
  • How to decide is a new feature request or a middleware goes into Kiota Core or Graph Core? I think we should have a thoroughly discussed and documented approach to be followed while planning and implementing new features. Example - Design to inherit some feature from Kiota Core and how to implement Graph specifications in the Graph core.

from kiota.

baywet avatar baywet commented on May 18, 2024

Thanks for joining the conversation @nikithauc
For the first point, I think the details are still being ironed out but I can see a situation where either the community, the kiota team or the sdk team for a given language (or a mix of all those audiences) address those issues.
For the second point, anything that's not Graph specific or OData specific should probably live in kiota core (e.g. retry handler, chaos handler, etc...) but the rest should stay in graph core (e.g. oData batching, large file uploads, odata type down casting, ...)
What do you think?

from kiota.

nikithauc avatar nikithauc commented on May 18, 2024

For the second point, anything that's not Graph specific or OData specific should probably live in kiota core (e.g. retry handler, chaos handler, etc...) but the rest should stay in graph core (e.g. oData batching, large file uploads, odata type down casting, ...)

I agree. Adding to this, I suggest opening a discussion of possible use cases concerning development in kiota core and graph core and we should model a good design/documentation. Example, consider a functionality which could be in kiota core and has a specific implementation in the graph core. Another case would be maintain synchronous development of a functionality in kiota core and graph core and across multiple languages. The use cases and possible approach should documented.

@baywet can you please elaborate on?
no abstractions for responses, middlewares etc, that's tied to the http client.

from kiota.

nikithauc avatar nikithauc commented on May 18, 2024

@baywet How will the core library containing the (kiota/graph) be configured with the ApiClient?

Some more points to discuss and consider during the core development are:

  1. What about custom middlewares?
  2. How do we return a raw response?
  3. Consider a user using just the core library without the request builders. How would the request and the response look like?
  4. More on point 3, how to allow user to pass a path in the request such as "/me/events" in the core?

from kiota.

baywet avatar baywet commented on May 18, 2024

Transition

Here is what I see happening to move the generic components from JS core to Kiota cores:

  1. Start a feature/v4 branch on JS Core
  2. Move all the generic components here
  3. Publish kiota abstraction & cores to public feeds
  4. Take a dependency from JS core on kiota packages
  5. (service libraries work)
  6. release
  7. open the champagne

I think this should allow us to quickly move components that need to be moved, get the fluent library out there while avoiding too much maintenance burden.
Should a bug be raised and fixed in the v3 of JS core, we'll need to duplicate the patch here as long as the v4 is not released, a bit more painful than merging branches but it should not be the end of the world.
And again, this is "tentative", I'm happy to review better proposals.

Abstractions for responses

Right now kiota has an abstraction layer over HTTP requests, (request info) that's not tied to any native library. My initial thoughts were to create abstractions for HTTP responses so our middlewares could be generic (not tied to a specific http client).
But after discussing with @darrelmiller, it appears that's a nasty business we don't want to get in, which would add a lot of work, including rebuilding all the middlewares we currently have instead of moving those.
Bottom line, our middlewares will be tied to a specific http client library, consumers will get everything (the http core service + middlewares), or they can build their own (including the middlewares).

Custom middlewares

This is still ongoing thinking process but I was envisioning a static method HttpClient.createDefault(authProvider) in the kiota http core library (kind of what we have today in Java/dotnet) which would return a native client with all the default middlewares. With that, customers can further customize it, potentially adding custom middlewares if they want to. Graph JS core would leverage it for its own HttpClient.createDefault method, adding its additional middlewares on top (telemetry...).
The HttpCore service already accepts the native http client as part of its constructor.

Raw response

If you're referring to getting the raw http response in lieu of the deserialized model, this is already possible today thanks to the native response handler that consumers can use to get called back with the native http response, this can be passed as an optional argument of any action method (get, post, put, patch...)

Using the core library without the request builders

I think this defeats the purpose. The core library's sole responsibility is to translate http request abstractions (request info) produced by the fluent API of the request builders into response models also produced by the generation process. And it does so wrapping the native http client.
If people want to craft requests manually, just use the native client with the middlewares configuration (see the custom middlewares section)

I hope that lengthy reply answers a lot of your questions. Thanks for keeping the discussion going!

from kiota.

baywet avatar baywet commented on May 18, 2024

To clarify the configuration experience which is WIP on #100:
The middlewares will be preconfigured by a public static method in the kiota http core library.
Then client can chose to do

var authProvider = new AzureIdentityAuthenticationProvider(tokenCredential);
            var client = new ApiClient(new HttpCore(authProvider));

And get the default configuration

Or

var authProvider = new AzureIdentityAuthenticationProvider(tokenCredential);
var nativeClient = HttpClientConfiguration.GetDefaultConfiguration(authProvider);
//some configuration on the native client, including adding/removing middlewares
var client = new ApiClient(new HttpCore(authProvider, httpClient: nativeClient));

from kiota.

MIchaelMainer avatar MIchaelMainer commented on May 18, 2024

Using the core library without the request builders. I think this defeats the purpose.

People will want to use ApiClient and will hold a reference to the native client through ApiClient. At some point, they may need to fallback to the native client because the generated request builders don't generate the API call as is expected by the service API. At that point, we need HttpClient. Either HttpClient is exposed by ApiClient as a property, or ApiClient has a a method for making arbitrary requests like we have in Java.

from kiota.

baywet avatar baywet commented on May 18, 2024

Fair point, at this moment the http client is a private field, and making it a public readonly property should not represent a lot of work since we're only talking about a modification in the HTTP core service implementation, no code-gen work.

private readonly System.Net.Http.HttpClient client;

from kiota.

baywet avatar baywet commented on May 18, 2024

added the generation result for dotnet, let me know what you think! microsoft/kiota-samples#146

from kiota.

baywet avatar baywet commented on May 18, 2024

Update: I'm done, please review the linked PR.
For TypeScript, fetch doesn't offer a middleware infrastructure like dotnet/okhttp do. So I mimicked the bare minimum needed for the feature to move forward. We can always improve it once we start the js core migration.

from kiota.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.