Skip to content

Commit 0aca28d

Browse files
authored
Fix allowed Argo parameter list and logging scope data not being rendered correctly (#870)
* Fix argo parameter validation keys. Fix to correctly serialize logger scope values. Signed-off-by: Victor Chang <vicchang@nvidia.com> * Fix unit tests Change argo pod date time to use the universal full ("U") format specifier Signed-off-by: Victor Chang <vicchang@nvidia.com>
1 parent 3cababe commit 0aca28d

File tree

22 files changed

+195
-190
lines changed

22 files changed

+195
-190
lines changed

src/TaskManager/Plug-ins/Argo/StaticValues/Keys.cs renamed to src/Common/Configuration/ArgoParameters.cs

Lines changed: 58 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,25 @@
1414
* limitations under the License.
1515
*/
1616

17-
namespace Monai.Deploy.WorkflowManager.TaskManager.Argo.StaticValues
17+
using System.Collections.Generic;
18+
19+
namespace Monai.Deploy.WorkflowManager.Common.Configuration
1820
{
19-
internal static class Keys
21+
public static class ArgoParameters
2022
{
23+
public struct ResourcesKey
24+
{
25+
public string TaskKey { get; set; }
26+
public string ArgoKey { get; set; }
27+
}
28+
public static class ResourcesKeys
29+
{
30+
public static readonly ResourcesKey GpuLimit = new() { TaskKey = GpuRequired, ArgoKey = "nvidia.com/gpu" };
31+
32+
public static readonly ResourcesKey MemoryLimit = new() { TaskKey = Memory, ArgoKey = "memory" };
33+
34+
public static readonly ResourcesKey CpuLimit = new() { TaskKey = Cpu, ArgoKey = "cpu" };
35+
}
2136
/// <summary>
2237
/// Key for the namespace where the Argo workflows are stored and executed.
2338
/// </summary>
@@ -76,18 +91,33 @@ internal static class Keys
7691
/// <summary>
7792
/// Key for resource limitations
7893
/// </summary>
79-
public static readonly string ArgoResource = "resources";
94+
public static readonly string Resources = "resources";
8095

8196
/// <summary>
8297
/// Key for resource limitations
8398
/// </summary>
84-
public static readonly string ArgoParameters = "parameters";
99+
public static readonly string Parameters = "parameters";
85100

86101
/// <summary>
87102
/// Key for priority classnames on task plugin arguments side
88103
/// </summary>
89104
public static readonly string TaskPriorityClassName = "priority";
90105

106+
/// <summary>
107+
/// Key for the CPU.
108+
/// </summary>
109+
public static readonly string Cpu = "cpu";
110+
111+
/// <summary>
112+
/// Key for the memory.
113+
/// </summary>
114+
public static readonly string Memory = "memory";
115+
116+
/// <summary>
117+
/// Key for the GPU.
118+
/// </summary>
119+
public static readonly string GpuRequired = "gpu_required";
120+
91121
/// <summary>
92122
/// Required arguments to run the Argo workflow.
93123
/// </summary>
@@ -96,6 +126,30 @@ internal static class Keys
96126
WorkflowTemplateName
97127
};
98128

129+
/// <summary>
130+
/// Required arguments to run the Argo workflow.
131+
/// </summary>
132+
public static readonly IReadOnlyList<string> VaildParameters =
133+
new List<string> {
134+
Namespace,
135+
BaseUrl,
136+
AllowInsecureseUrl,
137+
WorkflowTemplateName,
138+
TimeoutSeconds,
139+
ArgoApiToken,
140+
MessagingEndpoint,
141+
MessagingUsername,
142+
MessagingPassword,
143+
MessagingExchange,
144+
MessagingVhost,
145+
Resources,
146+
Parameters,
147+
TaskPriorityClassName,
148+
Cpu,
149+
Memory,
150+
GpuRequired,
151+
};
152+
99153
/// <summary>
100154
/// Required settings to run the Argo workflow.
101155
/// </summary>
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
/*
2+
* Copyright 2021-2023 MONAI Consortium
3+
* Copyright 2021 NVIDIA Corporation
4+
*
5+
* Licensed under the Apache License, Version 2.0 (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
18+
using System.Globalization;
19+
using System.Runtime.Serialization;
20+
21+
namespace Monai.Deploy.WorkflowManager.Common.Miscellaneous
22+
{
23+
[Serializable]
24+
public class LoggingDataDictionary<TKey, TValue> : Dictionary<TKey, TValue> where TKey : notnull
25+
{
26+
public LoggingDataDictionary()
27+
{
28+
}
29+
30+
protected LoggingDataDictionary(SerializationInfo info, StreamingContext context) : base(info, context)
31+
{
32+
}
33+
34+
public override string ToString()
35+
{
36+
var pairs = this.Select(x => string.Format(CultureInfo.InvariantCulture, "{0}={1}", x.Key, x.Value));
37+
return string.Join(", ", pairs);
38+
}
39+
}
40+
}

src/Common/Miscellaneous/ValidationConstants.cs

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -53,21 +53,6 @@ public static class ValidationConstants
5353
/// </summary>
5454
public static readonly string Notifications = "notifications";
5555

56-
/// <summary>
57-
/// Key for the CPU.
58-
/// </summary>
59-
public static readonly string Cpu = "cpu";
60-
61-
/// <summary>
62-
/// Key for the memory.
63-
/// </summary>
64-
public static readonly string Memory = "memory";
65-
66-
/// <summary>
67-
/// Key for the GPU.
68-
/// </summary>
69-
public static readonly string GpuRequired = "gpu_required";
70-
7156
/// <summary>
7257
/// Key for recipient emails.
7358
/// </summary>
@@ -107,24 +92,6 @@ public enum NotificationValues
10792
Mode
10893
};
10994

110-
/// <summary>
111-
/// Key for the endpoint where the Argo server is running.
112-
/// </summary>
113-
public static readonly string BaseUrl = "server_url";
114-
115-
/// <summary>
116-
/// Key for the name of the main 'WorkflowTemplate' stored on the targeted Argo server.
117-
/// </summary>
118-
public static readonly string WorkflowTemplateName = "workflow_template_name";
119-
120-
/// <summary>
121-
/// Required arguments to run the Argo task args.
122-
/// </summary>
123-
public static readonly IReadOnlyList<string> ArgoRequiredParameters =
124-
new List<string> {
125-
BaseUrl,
126-
WorkflowTemplateName
127-
};
12895

12996

13097
/// <summary>

src/TaskManager/Plug-ins/AideClinicalReview/AideClinicalReviewPlugin.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ private void ValidateEventAndInit()
189189

190190
public override async Task<ExecutionStatus> ExecuteTask(CancellationToken cancellationToken = default)
191191
{
192-
using var loggingScope = _logger.BeginScope(new Dictionary<string, object>
192+
using var loggingScope = _logger.BeginScope(new LoggingDataDictionary<string, object>
193193
{
194194
["correlationId"] = Event.CorrelationId,
195195
["workflowInstanceId"] = Event.WorkflowInstanceId,

0 commit comments

Comments
 (0)