Task/kiota core dependency #141
Conversation
msgraph/core/graph_client.py
Outdated
| return wrapper | ||
|
|
||
|
|
||
| class GraphClient: |
There was a problem hiding this comment.
is inheritance from multiple parents supported in Python? If not, I'm not sure how the GraphServiceClient will inherit from this one.
There was a problem hiding this comment.
GraphServiceClient will be passed a GraphRequestAdapter instance. GraphRequestAdapter in turn accepts a GraphClient as one of the arguments
There was a problem hiding this comment.
I think the naming here is leading to confusion, if this is not a client in the sense of http client or a client in the sense of a service client, what is its function? so we can find a better name for it.
There was a problem hiding this comment.
GraphClient is indeed a http client, and will be used to make raw string requests to the graph API by users who do not want the service library. Alse added a separate GraphRequestAdapter class that will be used for the full sdk/self-serve sdks.
There was a problem hiding this comment.
We may not need the separation of the two , I believe it should be possible to build an instance of RequestInformation using the raw string and then pass it to the RequestAdapter to send the request. This would mean one GraphClient which holds a RequestAdapter inside it.
This way we can possibly avoid implementation/functional drifts due to slight variations in configuration of the client e.g Auth which is handled by the RequestAdapter.
There was a problem hiding this comment.
Could work. Lemme test it out. I'll revert with findings.
msgraph/core/middleware/redirect.py
Outdated
| from .middleware import GraphRequest | ||
|
|
||
|
|
||
| class GraphRedirectHandler(RedirectHandler): |
There was a problem hiding this comment.
do we still need this at all since we should already have it in kiota http?
There was a problem hiding this comment.
Extends the one in kiota_http to add graph specific stuff such as request context and setting FeatureUsageFlags
There was a problem hiding this comment.
Just for my benefit, is there any other extra customization being done apart from FeatureUsageFlags and request context.
I believe that FeatureUsageFlags can all be inferred at one place like the telemetry handler by simply checking the items in the middleware collection or computed once beforehand when the client is initialized.
Similarly, request context can be copied over to new requests if its held within a RequestOption instance.
This way we can have only one redirect handler from the core abstractions.
There was a problem hiding this comment.
No other customizations other than setting, RequestContext and updating FeatureUsage. I can definitely refactor the implementation to avoid extending the middleware.
There was a problem hiding this comment.
thanks for the additional context, please do so Python implementation is not diverging from other languages.
| async def send(self, request: GraphRequest, transport: AsyncKiotaTransport): | ||
| """Adds telemetry headers and sends the http request. | ||
| """ | ||
| self.set_request_context_and_feature_usage(request, transport) |
Overview
Refactors the python core library to take a dependency on Kiota core