-
Notifications
You must be signed in to change notification settings - Fork 53
Description
Describe the bug
The current implementation of the RestClient has a coupled HttpClient instantiation in line 186. To be fair in the ServiceCollectionExtension the whole notionclient is a singleton which mitigates issues that you could run into.
My main issue that I was having was that I wanted to have more control of the httpclient and be able to add resilient policies to it via the .AddPolicyHandler
extensions.
Additional context
In my project I basically copied the RestClient code, but made the logger and HttpClient a dependency that will get injected. ClientOptions is no longer a dependency and instead it comes in through the IOptions pattern and uses the configuration to setup the httpclient.
public static IHttpClientBuilder AddNotionClient(
this IServiceCollection services, IConfiguration configuration)
{
// Register HttpClient for NotionService
services.Configure<ClientOptions>(configuration.GetSection("Notion"));
var httpClientBuilder = services.AddHttpClient<IRestClient, NotionBaeRestClient>((srv, httpClient) =>
{
var options = srv.GetService<IOptions<ClientOptions>>()?.Value ?? throw new Exception("Notion client options not configured");
httpClient.BaseAddress = new Uri(options.BaseUrl);
httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", options.AuthToken);
httpClient.DefaultRequestHeaders.Add("Notion-Version", options.NotionVersion);
})
.ConfigurePrimaryHttpMessageHandler(() => new LoggingHandler { InnerHandler = new HttpClientHandler() });
services.AddSingleton<IUsersClient, UsersClient>();
services.AddSingleton<IDatabasesClient, DatabasesClient>();
services.AddSingleton<IPagesClient, PagesClient>();
services.AddSingleton<ISearchClient, SearchClient>();
services.AddSingleton<ICommentsClient, CommentsClient>();
services.AddSingleton<IBlocksClient, BlocksClient>();
services.AddSingleton<IAuthenticationClient, AuthenticationClient>();
services.AddSingleton<INotionClient, NotionClient>();
return httpClientBuilder;
}
my appsettings.local.json file looks like this
{
"Notion" : {
"AuthToken" : "<APITOKEN>",
"BaseUrl" : "https://api.notion.com/",
"NotionVersion" : "2022-06-28"
}
}
in my Program.cs
file i am using the extension like this
builder.Services.AddNotionClient(builder.Configuration)
.AddPolicyHandler(Policy.BulkheadAsync<HttpResponseMessage>(10, Int32.MaxValue))
.AddPolicyHandler((provider, _) => GetRetryOnRateLimitingPolicy(provider));
I can fork and make a PR if you are interested in this. If not, no hard feelings here, my needs are pretty specific but I wonder if anyone else who wants to do retries to get around api rate limits or simply have a bit of retry / resilience could benefit from this..
If you would like to review my code and how its implemented / being used in my context my project can be found here NotionBae MCP Server