refactor Web Crawler example - use constructor DI instead of Service.… #11
Conversation
| this.httpClient = httpClient; | ||
| // configure the client inside constructor if needed (add custom headers etc.) | ||
| this.httpClient.DefaultRequestHeaders.Add(HeaderNames.UserAgent, "SearchCrawler"); | ||
| // get crawler base url from settings or site configuration, make sure that WebCrawlerBaseUrl is correct | ||
| string baseUrl = ValidationHelper.GetString(Service.Resolve<IAppSettingsService>()["WebCrawlerBaseUrl"], DocumentURLProvider.GetDomainUrl("DancingGoatCore")); | ||
| string baseUrl = ValidationHelper.GetString(appSettingsService["WebCrawlerBaseUrl"], DocumentURLProvider.GetDomainUrl("DancingGoatCore")); |
There was a problem hiding this comment.
Now I'm wondering if we should make a new "section" in appsettings.json for Lucene - we do that with other integrations.
Example:
{
// other settings ...
"xperience.lucene": {
"WebCrawlerBaseUrl": "..."
}
}There was a problem hiding this comment.
It depends, it is just usage example with configuration, not configuration of the integration module itself.
I would keep it as simple as possible.
Should I create the new configuration section?
There was a problem hiding this comment.
Good point! This isn't something we "ship" with the package.
However, this is an example we are providing to developers and from my experience, developers will copy examples assuming they are being given best practice guidance.
So I think we should set developers up for success and do what we would in a real project - at least for this part - in part because we might have appsettings.json configuration that does ship with the package in the future and these settings should be in the same place.
Lahvac
force-pushed
the
refactor/crawler
branch
from
341e255
to
780bc01
Compare
|
The resolution of this request has been moved into the following pull request |
DI refactoring