-
Notifications
You must be signed in to change notification settings - Fork 4
robust parsing #12
base: main
Are you sure you want to change the base?
robust parsing #12
Conversation
rlratzel
left a comment
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.
Thanks @kevingerman. I had a couple of questions.
| resultsDict = rDict["results"] | ||
| resultsDict = rDict.get("results",{}) | ||
| if not resultsDict: | ||
| print("ERROR: No results found!") |
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.
should this raise an exception if it's an error?
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 question is, is it a stop error or a fallthrough and notify error. I tend to only stop a program when the error will create incorrect perceptions or break other functionality. If you think this is a stop case, I will change it to be an exception. To me it looked like an unlikely case, but not impossible. I still raised the error message, because there shouldn't be a case where there is a key for the results but no stored results dict. Falling through would allow a database to exist with orphaned results, but that error would not prevent the user from being able to interact with the results that are not broken.
I think that that approach is more consistent with the way that other parts of ASV operate. ASV will exclude results from the published site if it cannot reach a commit for that result using the specified SCM type and repo.
| continue | ||
|
|
||
| benchmarkSpec = bDict[benchmarkName] | ||
| benchmarkSpec = bDict.get(benchmarkName) |
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 wondering if it's better if this crashes with a KeyError here like it did before, since this could now return None and continue on until something tries to use it.
resolves #11