-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
validate shape of packages.config
and consolidate error handling
#11303
Conversation
var invalidPackageElements = Packages.Where(p => p.GetAttribute("id") is null || p.GetAttribute("version") is null).ToList(); | ||
if (invalidPackageElements.Any()) | ||
{ | ||
throw new UnparseableFileException("`package` element missing `id` or `version` attributes", path); |
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.
This was the fix for the linked issue.
@@ -14,4 +19,23 @@ public JobErrorBase(string type) | |||
|
|||
[JsonPropertyName("error-details")] | |||
public Dictionary<string, object> Details { get; init; } = new(); | |||
|
|||
public static JobErrorBase ErrorFromException(Exception ex, string jobId, string currentDirectory) |
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.
This is the consolidated error conversion.
1950ee9
to
8667468
Compare
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.
Looks good, though I can't give approval ATM due to a permissions issue.
{ | ||
var (experimentsManager, _errorResult) = await ExperimentsManager.FromJobFileAsync(jobPath.FullName); | ||
// since we have more than 8 arguments, we have to pull them out manually |
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.
That's annoying.
8667468
to
692d989
Compare
Validate the shape of
packages.config
when first opening the fileFixes #11250.
During this I realized that we had way too many ways to represent errors and to convert them, so everything was consolidated into a single function
JobErrorBase.ErrorFromException()
. Everything else is just calling this helper and updating the tests.