Skip to content

Commit da7546d

Browse files
shreyas-gopalakrishnashreyas-gopalakrishna
committed
Addressing review comments
1 parent 5a144d8 commit da7546d

12 files changed

+78
-49
lines changed

‎src/WebJobs.Script/Workers/Profiles/EnvironmentCondition.cs‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
namespace Microsoft.Azure.WebJobs.Script.Workers
1111
{
12+
// Environment condition checks if environment variables match the expected output
1213
public class EnvironmentCondition : IWorkerProfileCondition
1314
{
1415
private readonly ILogger _logger;
@@ -37,20 +38,23 @@ internal EnvironmentCondition(ILogger logger, IEnvironment environment, WorkerPr
3738

3839
public string Expression => _expression;
3940

41+
/// <inheritdoc />
4042
public bool Evaluate()
4143
{
4244
string value = _environment.GetEnvironmentVariable(Name);
45+
4346
if (string.IsNullOrEmpty(value))
4447
{
4548
return false;
4649
}
4750

48-
_logger.LogDebug($"Evaluating EnvironmentCondition with value: {value} and expression {Expression}");
51+
_logger.LogDebug($"Evaluating EnvironmentCondition with value '{value}' and expression '{Expression}'");
4952

5053
return _regex.IsMatch(value);
5154
}
5255

53-
public void Validate()
56+
// Validates if condition parametrs meet expected values, fail if they don't
57+
internal void Validate()
5458
{
5559
if (string.IsNullOrEmpty(Name))
5660
{

‎src/WebJobs.Script/Workers/Profiles/FalseCondition.cs‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,10 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
33

4-
using System;
5-
using System.ComponentModel.DataAnnotations;
6-
using System.Text.RegularExpressions;
7-
using Microsoft.Azure.WebJobs.Script.Workers.Profiles;
8-
using Microsoft.Extensions.Logging;
9-
104
namespace Microsoft.Azure.WebJobs.Script.Workers
115
{
6+
// The false condition always evalues to false.
7+
// This condition is used to disable a profile when condition providers fail to resolve conditions.
128
public class FalseCondition : IWorkerProfileCondition
139
{
1410
public bool Evaluate()

‎src/WebJobs.Script/Workers/Profiles/HostPropertyCondition.cs‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Microsoft.Azure.WebJobs.Script.Workers
1313
{
14+
// HostPropertycondition checks if host match the expected output for properties such as Sku, Platform, HostVersion
1415
internal class HostPropertyCondition : IWorkerProfileCondition
1516
{
1617
private readonly ILogger _logger;
@@ -35,7 +36,7 @@ public HostPropertyCondition(ILogger logger, ISystemRuntimeInformation systemRun
3536
Validate();
3637
}
3738

38-
public enum ConditionHostPropertyName
39+
public enum HostProperty
3940
{
4041
Sku,
4142
Platform,
@@ -46,24 +47,18 @@ public enum ConditionHostPropertyName
4647

4748
public string Expression => _expression;
4849

50+
/// <inheritdoc />
4951
public bool Evaluate()
5052
{
51-
string value = string.Empty;
52-
Enum.TryParse(Name, out ConditionHostPropertyName hostPropertyName);
53-
switch (hostPropertyName)
53+
Enum.TryParse(Name, out HostProperty hostPropertyName);
54+
55+
string value = hostPropertyName switch
5456
{
55-
case ConditionHostPropertyName.Sku:
56-
value = ScriptSettingsManager.Instance.GetSetting(EnvironmentSettingNames.AzureWebsiteSku);
57-
break;
58-
case ConditionHostPropertyName.Platform:
59-
value = _systemRuntimeInformation.GetOSPlatform().ToString();
60-
break;
61-
case ConditionHostPropertyName.HostVersion:
62-
value = ScriptHost.Version;
63-
break;
64-
default:
65-
break;
66-
}
57+
HostProperty.Sku => ScriptSettingsManager.Instance.GetSetting(EnvironmentSettingNames.AzureWebsiteSku),
58+
HostProperty.Platform => _systemRuntimeInformation.GetOSPlatform().ToString(),
59+
HostProperty.HostVersion => ScriptHost.Version,
60+
_ => null
61+
};
6762

6863
if (string.IsNullOrEmpty(value))
6964
{
@@ -75,14 +70,15 @@ public bool Evaluate()
7570
return _regex.IsMatch(value);
7671
}
7772

78-
public void Validate()
73+
// Validates if condition parametrs meet expected values, fail if they don't
74+
internal void Validate()
7975
{
8076
if (string.IsNullOrEmpty(Name))
8177
{
8278
throw new ValidationException($"HostPropertyCondition {nameof(Name)} cannot be empty.");
8379
}
8480

85-
if (!Enum.GetNames(typeof(ConditionHostPropertyName)).Any(x => x.ToLower().Contains(Name)))
81+
if (!Enum.GetNames(typeof(HostProperty)).Any(x => x.ToLower().Contains(Name)))
8682
{
8783
throw new ValidationException($"HostPropertyCondition {nameof(Name)} is not a valid host property name.");
8884
}

‎src/WebJobs.Script/Workers/Profiles/IWorkerProfileCondition.cs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@
33

44
namespace Microsoft.Azure.WebJobs.Script.Workers
55
{
6+
// Interface for different types of conditions
67
public interface IWorkerProfileCondition
78
{
9+
/// <summary>
10+
/// Check if different condition type meet their criteria
11+
/// </summary>
812
bool Evaluate();
913
}
1014
}

‎src/WebJobs.Script/Workers/Profiles/IWorkerProfileConditionProvider.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the MIT License. See License.txt in the project root for license information.
33

4-
using System;
5-
using System.Collections.Generic;
6-
using System.Text;
7-
84
namespace Microsoft.Azure.WebJobs.Script.Workers.Profiles
95
{
6+
// Manage differnt conditions
107
internal interface IWorkerProfileConditionProvider
118
{
9+
/// <summary>
10+
/// Factory method to create a profile condition
11+
/// </summary>
1212
bool TryCreateCondition(WorkerProfileConditionDescriptor descriptor, out IWorkerProfileCondition condition);
1313
}
1414
}

‎src/WebJobs.Script/Workers/Profiles/IWorkerProfileManager.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,27 @@
77

88
namespace Microsoft.Azure.WebJobs.Script.Workers
99
{
10+
// Regulate profile operations through the profile manager
1011
public interface IWorkerProfileManager
1112
{
13+
/// <summary>
14+
/// Creates profile condition using different condition descriptor properties
15+
/// </summary>
1216
bool TryCreateWorkerProfileCondition(WorkerProfileConditionDescriptor conditionDescriptor, out IWorkerProfileCondition condition);
1317

18+
/// <summary>
19+
/// Save different profiles for a given worker runtime language
20+
/// </summary>
1421
void SaveWorkerDescriptionProfiles(List<WorkerDescriptionProfile> workerDescriptionProfiles, string language);
1522

23+
/// <summary>
24+
/// Load a profile that meets it's conditions
25+
/// </summary>
1626
void LoadWorkerDescriptionFromProfiles(RpcWorkerDescription defaultWorkerDescription, out RpcWorkerDescription workerDescription);
1727

28+
/// <summary>
29+
/// Verify if the current profile's conditions have changed
30+
/// </summary>
1831
bool IsCorrectProfileLoaded(string workerRuntime);
1932
}
2033
}

‎src/WebJobs.Script/Workers/Profiles/SystemConditionProvider.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public SystemConditionProvider(ILogger<SystemConditionProvider> logger, ISystemR
2020
_systemRuntimeInformation = systemRuntimeInfo ?? throw new ArgumentNullException(nameof(systemRuntimeInfo));
2121
}
2222

23+
/// <inheritdoc />
2324
public bool TryCreateCondition(WorkerProfileConditionDescriptor descriptor, out IWorkerProfileCondition condition)
2425
{
2526
condition = descriptor.Type switch

‎src/WebJobs.Script/Workers/Profiles/WorkerDescriptionProfile.cs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,15 @@ public bool EvaluateConditions()
6969
/// <summary>
7070
/// Overrides the worker description parameters to that available in the profile
7171
/// </summary>
72-
public RpcWorkerDescription ApplyProfile(RpcWorkerDescription defaultWorkerDescription)
72+
public RpcWorkerDescription ApplyProfile(RpcWorkerDescription workerDescription)
7373
{
74-
defaultWorkerDescription.Arguments = UseProfileOrDefault(ProfileDescription.Arguments, defaultWorkerDescription.Arguments);
75-
defaultWorkerDescription.DefaultExecutablePath = UseProfileOrDefault(ProfileDescription.DefaultExecutablePath, defaultWorkerDescription.DefaultExecutablePath);
76-
defaultWorkerDescription.DefaultWorkerPath = UseProfileOrDefault(ProfileDescription.DefaultWorkerPath, defaultWorkerDescription.DefaultWorkerPath);
77-
defaultWorkerDescription.Extensions = UseProfileOrDefault(ProfileDescription.Extensions, defaultWorkerDescription.Extensions) as List<string>;
78-
defaultWorkerDescription.Language = UseProfileOrDefault(ProfileDescription.Language, defaultWorkerDescription.Language);
79-
defaultWorkerDescription.WorkerDirectory = UseProfileOrDefault(ProfileDescription.WorkerDirectory, defaultWorkerDescription.WorkerDirectory);
80-
return defaultWorkerDescription;
74+
workerDescription.Arguments = UseProfileOrDefault(ProfileDescription.Arguments, workerDescription.Arguments);
75+
workerDescription.DefaultExecutablePath = UseProfileOrDefault(ProfileDescription.DefaultExecutablePath, workerDescription.DefaultExecutablePath);
76+
workerDescription.DefaultWorkerPath = UseProfileOrDefault(ProfileDescription.DefaultWorkerPath, workerDescription.DefaultWorkerPath);
77+
workerDescription.Extensions = UseProfileOrDefault(ProfileDescription.Extensions, workerDescription.Extensions) as List<string>;
78+
workerDescription.Language = UseProfileOrDefault(ProfileDescription.Language, workerDescription.Language);
79+
workerDescription.WorkerDirectory = UseProfileOrDefault(ProfileDescription.WorkerDirectory, workerDescription.WorkerDirectory);
80+
return workerDescription;
8181
}
8282

8383
private string UseProfileOrDefault(string profileParameter, string defaultParameter)

‎src/WebJobs.Script/Workers/Profiles/WorkerProfileManager.cs‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,26 +9,28 @@
99

1010
namespace Microsoft.Azure.WebJobs.Script.Workers
1111
{
12+
// The default profile manager that manages profiles from language workers
1213
internal class WorkerProfileManager : IWorkerProfileManager
1314
{
1415
private readonly ILogger _logger;
1516
private readonly IEnumerable<IWorkerProfileConditionProvider> _conditionProviders;
1617
private Dictionary<string, List<WorkerDescriptionProfile>> _profiles = new Dictionary<string, List<WorkerDescriptionProfile>>();
1718
private string _activeProfile;
1819

19-
public WorkerProfileManager(ILogger logger,
20-
IEnumerable<IWorkerProfileConditionProvider> conditionProviders)
20+
public WorkerProfileManager(ILogger logger, IEnumerable<IWorkerProfileConditionProvider> conditionProviders)
2121
{
2222
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
2323
_conditionProviders = conditionProviders ?? throw new ArgumentNullException(nameof(conditionProviders));
2424
_activeProfile = string.Empty;
2525
}
2626

27+
/// <inheritdoc />
2728
public void SaveWorkerDescriptionProfiles(List<WorkerDescriptionProfile> workerDescriptionProfiles, string language)
2829
{
2930
_profiles.Add(language, workerDescriptionProfiles);
3031
}
3132

33+
// Evaluate profile conditions for a language
3234
private bool GetEvaluatedProfile(string language, out WorkerDescriptionProfile evaluatedProfile)
3335
{
3436
if (_profiles.TryGetValue(language, out List<WorkerDescriptionProfile> profiles))
@@ -46,17 +48,20 @@ private bool GetEvaluatedProfile(string language, out WorkerDescriptionProfile e
4648
return false;
4749
}
4850

51+
/// <inheritdoc />
4952
public void LoadWorkerDescriptionFromProfiles(RpcWorkerDescription defaultWorkerDescription, out RpcWorkerDescription workerDescription)
5053
{
5154
if (GetEvaluatedProfile(defaultWorkerDescription.Language, out WorkerDescriptionProfile profile))
5255
{
5356
_logger?.LogInformation($"Worker initialized with profile - {profile.Name}, Profile ID {profile.ProfileId} from worker config.");
5457
_activeProfile = profile.ProfileId;
5558
workerDescription = profile.ApplyProfile(defaultWorkerDescription);
59+
return;
5660
}
5761
workerDescription = defaultWorkerDescription;
5862
}
5963

64+
/// <inheritdoc />
6065
public bool TryCreateWorkerProfileCondition(WorkerProfileConditionDescriptor conditionDescriptor, out IWorkerProfileCondition condition)
6166
{
6267
foreach (var provider in _conditionProviders)
@@ -73,6 +78,7 @@ public bool TryCreateWorkerProfileCondition(WorkerProfileConditionDescriptor con
7378
return false;
7479
}
7580

81+
/// <inheritdoc />
7682
public bool IsCorrectProfileLoaded(string workerRuntime)
7783
{
7884
var profileId = string.Empty;

‎src/WebJobs.Script/Workers/Rpc/Configuration/RpcWorkerConfig.cs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the MIT License. See License.txt in the project root for license information.
33

4-
using System.Collections.Generic;
5-
64
namespace Microsoft.Azure.WebJobs.Script.Workers.Rpc
75
{
86
public class RpcWorkerConfig

0 commit comments

Comments
 (0)