-
Notifications
You must be signed in to change notification settings - Fork 21
[Revalidation] Add configurable overrides for revalidation rates #588
Conversation
src/NuGet.Services.Revalidate/Job.cs
Outdated
@@ -112,6 +119,12 @@ await scope.ServiceProvider | |||
protected override void ConfigureJobServices(IServiceCollection services, IConfigurationRoot configurationRoot) | |||
{ | |||
services.Configure<RevalidationConfiguration>(configurationRoot.GetSection(JobConfigurationSectionName)); | |||
services.Configure<RevalidationConfiguration>(config => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm configuring the same configuration twice to override the event rates. See this documentation:
You can add multiple configuration providers... They're applied in the order that they're registered... When more than one configuration service is enabled, the last configuration source specified wins and sets the configuration value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't you then have
services.Configure<RevalidationConfiguration>(config =>
{
config.MinPackageEventRate = _overrideMinPackageEventRate;
config.MaxPackageEventRate = _overrideMaxPackageEventRate;
});
as the first Configure
call, followed by
services.Configure<RevalidationConfiguration>(configurationRoot.GetSection(JobConfigurationSectionName));
Which will override defaults if specified?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CLI arguments should override the settings file as the CLI arguments come from Octopus, which is what I'll be updating.
The Configure
methods are called in the order that they're registered:
- The setting's files configs are applied
- The CLI overrides are applied on top of the setting's files
I've deployed this and verified the behavior is as expected.
@@ -9,6 +9,8 @@ title #{Jobs.nuget.services.revalidate.Title} | |||
|
|||
start /w NuGet.Services.Revalidate.exe ^ | |||
-Configuration #{Jobs.nuget.services.revalidate.Configuration} ^ | |||
-OverrideMinPackageEventRate #{Jobs.nuget.services.revalidation.MinPackageEventRate} ^ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move this to the JSON. As talked about offline, we have other Octopus replacement tokens in the JSON so we can't can just mimic that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⌚️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⌚️
I've split tests into two categories: 1. "First results" - These tests that the expected results are first 2. "Top results" - These tests that the expected results are found I ensured that our tests align with the most frequently selected results, as per our search telemetry. Addresses NuGet/NuGetGallery#6980
This will let us speed up/slow down the revalidation job by changing Octopus variables and creating a new deployment. Addresses https://github.com/NuGet/Engineering/issues/1789
I've split tests into two categories: 1. "First results" - These tests that the expected results are first 2. "Top results" - These tests that the expected results are found I ensured that our tests align with the most frequently selected results, as per our search telemetry. Addresses NuGet/NuGetGallery#6980
This will let us speed up/slow down the revalidation job by changing Octopus variables and creating a new deployment.
Addresses https://github.com/NuGet/Engineering/issues/1789