Skip to content
This repository was archived by the owner on Nov 1, 2024. It is now read-only.

robust parsing #12

Open
kevingerman wants to merge 2 commits into
base: main
Choose a base branch
from
Open

robust parsing #12

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9dc3a6
robust parsing
kevingerman Oct 5, 2021
8cdfbb1
Merge branch 'rapidsai:main' into robusto
kevingerman May 18, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions asvdb/asvdb.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,10 @@ def __readResults(self, infoOnly=False, filterByInfoObjs=None):
if infoOnly:
machineResults.append(bi)
else:
# FIXME: if results not in rDict, throw better error
resultsDict = rDict["results"]
resultsDict = rDict.get("results",{})
if not resultsDict:
print("ERROR: No results found!")
Copy link
Collaborator

@rlratzel rlratzel Oct 12, 2021

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?

Copy link
Author

@kevingerman kevingerman Oct 21, 2021 •
edited
Loading

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.

#TODO: replace print statements with logging
# Populate the list of BenchmarkResult objs associated
# with the BenchmarkInfo obj
resultObjs = []
Expand All @@ -420,15 +422,17 @@ def __readResults(self, infoOnly=False, filterByInfoObjs=None):
f"file: {resultsFile.as_posix()} "
f"invalid name\"{benchmarkName}\", skipping.")
continue

benchmarkSpec = bDict[benchmarkName]
benchmarkSpec = bDict.get(benchmarkName)
Copy link
Collaborator

@rlratzel rlratzel Oct 12, 2021

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.

# benchmarkResults is the entry in this particular
# result file for this benchmark
benchmarkResults = resultsDict[benchmarkName]
if not benchmarkResults or not(hasattr(benchmarkResults,'__iter__')):
benchmarkResults={}
print( f"WARNING: benchmarkResults not found for {benchmarkName}")

paramNames = benchmarkSpec["param_names"]
paramValues = benchmarkResults["params"]
results = benchmarkResults["result"]
paramNames = benchmarkSpec.get("param_names",[])
paramValues = benchmarkResults.get("params",[])
results = benchmarkResults.get("result",[])
# Inverse of the write operation described in
# self.__updateResultJson()
paramsCartProd = list(itertools.product(*paramValues))
Expand Down Expand Up @@ -773,7 +777,7 @@ def __loadJsonDictFromFile(self, jsonFile):
either reading in the existing file or returning {}
"""
if path.exists(jsonFile):
with open(jsonFile) as fobj:
with open(jsonFile, 'rb') as fobj:
# FIXME: ideally this could use flock(), but some situations do
# not allow grabbing a file lock (NFS?)
# fcntl.flock(fobj, fcntl.LOCK_EX)
Expand Down