Rocm plugin update to accommodate for changes in 7.0 #31
Conversation
|
ROCm has changed the way they provide the version number. |
alexandraBara
changed the title
@cmcknigh I added version-rocm, should i be adding version-utils as well? |
|
|
||
| rocm_data = None | ||
| for path in version_paths: | ||
| if Path(path).exists(): |
There was a problem hiding this comment.
We should not use path.exists() as this would only be supported for local execution. _run_sut_cmd should be used for anything that interacts with the shell of the target node.
| self._log_event( | ||
| category=EventCategory.OS, | ||
| description=f"Could not get ROCm version format from {path}", | ||
| data={"raw_output": res.stdout}, | ||
| priority=EventPriority.ERROR, | ||
| ) |
There was a problem hiding this comment.
May be better to log this at the end using for...else if no break was hit. Otherwise, we will always log an error event if "/opt/rocm/.info/version-rocm" does not exist, even if we are able to read the version from "/opt/rocm/.info/version". When we successfully read the version from either of the paths, we should not be logging error events.
| else: | ||
| self._log_event( | ||
| category=EventCategory.OS, | ||
| description=f"Could not get ROCm version format from {path}", |
There was a problem hiding this comment.
Since this is being logged at the end, we should reference all the paths that were checked rather than just the last one.
f"Unable to read ROCm version from {version_paths }"
Error observed when running node-scraper inside a container with rocm 7.0.0
Previous behavior:
Current behavior: