Skip to content

Commit ca37412

Browse files
Merge pull request #186 from amd/development
dev -> main
2 parents ce26437 + 00b62f7 commit ca37412

File tree

82 files changed

+4335
-660
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

82 files changed

+4335
-660
lines changed
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
name: Bug report
2+
description: Report a reproducible problem with Node Scraper
3+
title: "[Bug]: "
4+
labels: ["bug"]
5+
body:
6+
- type: markdown
7+
attributes:
8+
value: |
9+
Thanks for taking the time to report a bug. Please include enough detail to reproduce the issue.
10+
- type: textarea
11+
id: summary
12+
attributes:
13+
label: Summary
14+
description: What happened and what did you expect to happen?
15+
placeholder: Clear and concise description of the bug.
16+
validations:
17+
required: true
18+
- type: textarea
19+
id: steps
20+
attributes:
21+
label: Steps to reproduce
22+
description: Minimal, concrete steps to reproduce the issue.
23+
placeholder: |
24+
1. ...
25+
2. ...
26+
3. ...
27+
validations:
28+
required: true
29+
- type: textarea
30+
id: logs
31+
attributes:
32+
label: CLI output / logs
33+
description: Paste relevant output (redact secrets/tokens/hosts as needed).
34+
render: shell
35+
validations:
36+
required: false
37+
- type: input
38+
id: version
39+
attributes:
40+
label: Version
41+
description: Output of `node-scraper --version` (or the git commit hash).
42+
placeholder: "e.g. 0.6.1"
43+
validations:
44+
required: false
45+
- type: dropdown
46+
id: install_method
47+
attributes:
48+
label: Install method
49+
options:
50+
- PyPI (pip install amd-node-scraper)
51+
- Editable install from source (pip install -e .)
52+
- Other
53+
validations:
54+
required: false
55+
- type: input
56+
id: python
57+
attributes:
58+
label: Python version
59+
placeholder: "e.g. 3.11.8"
60+
validations:
61+
required: false
62+
- type: textarea
63+
id: environment
64+
attributes:
65+
label: Environment
66+
description: OS, target system type (LOCAL/REMOTE), and anything else relevant.
67+
placeholder: |
68+
OS:
69+
Target:
70+
Notes:
71+
validations:
72+
required: false

.github/ISSUE_TEMPLATE/config.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
blank_issues_enabled: false
2+
contact_links:
3+
- name: Questions
4+
url: https://github.com/amd/node-scraper/issues
5+
about: If you're not sure it's a bug/feature request yet, start with an issue and add details.
6+
- name: Security reports
7+
url: https://github.com/amd/node-scraper/security/policy
8+
about: Please report security issues privately (see SECURITY.md).
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
name: Feature request
2+
description: Suggest an enhancement or new capability
3+
title: "[Feature]: "
4+
labels: ["enhancement"]
5+
body:
6+
- type: markdown
7+
attributes:
8+
value: |
9+
Thanks for the suggestion. Please describe the problem you're trying to solve and what success looks like.
10+
- type: textarea
11+
id: problem
12+
attributes:
13+
label: Problem / motivation
14+
description: What problem are you trying to solve?
15+
placeholder: "I want to..."
16+
validations:
17+
required: true
18+
- type: textarea
19+
id: proposal
20+
attributes:
21+
label: Proposed solution
22+
description: What would you like Node Scraper to do?
23+
validations:
24+
required: true
25+
- type: textarea
26+
id: alternatives
27+
attributes:
28+
label: Alternatives considered
29+
description: What else have you tried or considered?
30+
validations:
31+
required: false
32+
- type: textarea
33+
id: scope
34+
attributes:
35+
label: Scope / impact
36+
description: What plugins/OSes/environments would this affect?
37+
validations:
38+
required: false

.github/pull_request_template.md

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
## Summary
2+
-
3+
4+
## Test plan
5+
- [ ] `pytest test/unit`
6+
- [ ] `pytest test/functional` (if applicable)
7+
- [ ] `pre-commit run --all-files`
8+
9+
## Checklist
10+
- [ ] Added/updated tests (or explained why not)
11+
- [ ] Updated docs/README if behavior changed
12+
- [ ] No secrets or credentials committed
Lines changed: 261 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,261 @@
1+
#!/usr/bin/env python3
2+
"""Checks conventions under ``nodescraper/plugins`` (stderr warnings only; non-blocking).
3+
4+
1. **Command strings in collectors/analyzers** , for ``Collector``
5+
or ``Analyzer`` classes: a *class-level* assignment to a string (or f-string) that
6+
looks like a shell/CLI invocation must use the name ``CMD`` or
7+
``CMD_<suffix>`` (e.g. ``CMD_LIST``). Names starting with ``_`` and names
8+
listed in ``_CMD_CHECK_SKIP_NAMES`` are ignored; see
9+
``_looks_like_shell_command_literal`` for what counts as command-like.
10+
11+
2. **Args models** — In ``collector_args.py`` and ``analyzer_args.py``,
12+
for classes named ``*Args`` that subclass ``BaseModel``, ``CollectorArgs``,
13+
``AnalyzerArgs``, or another ``*Args``: each public field should assign
14+
``pydantic.Field(...)`` with a non-empty ``description=`` (for help/CLI
15+
text). ``ClassVar`` fields, ``_``-prefixed names, and ``model_config`` are
16+
skipped.
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import ast
22+
import re
23+
import sys
24+
from pathlib import Path
25+
26+
_REPO_ROOT = Path(__file__).resolve().parent.parent.parent
27+
PLUGIN_ROOT = _REPO_ROOT / "nodescraper" / "plugins"
28+
29+
# Class-level names in collectors/analyzers that are not shell-command strings.
30+
_CMD_CHECK_SKIP_NAMES = frozenset(
31+
{
32+
"AMD_SMI_EXE",
33+
"DATA_MODEL",
34+
"SUPPORTED_OS_FAMILY",
35+
"COLLECTOR",
36+
"ANALYZER",
37+
"COLLECTOR_ARGS",
38+
"ANALYZER_ARGS",
39+
"TYPE_CHECKING",
40+
}
41+
)
42+
43+
44+
def _is_stringish(expr: ast.expr) -> bool:
45+
if isinstance(expr, ast.Constant) and isinstance(expr.value, str):
46+
return True
47+
if isinstance(expr, ast.JoinedStr):
48+
return True
49+
return False
50+
51+
52+
def _stringish_preview(expr: ast.expr) -> str | None:
53+
"""Best-effort static string for command-like heuristics (f-strings may be partial)."""
54+
if isinstance(expr, ast.Constant) and isinstance(expr.value, str):
55+
return expr.value
56+
if isinstance(expr, ast.JoinedStr):
57+
parts: list[str] = []
58+
for elt in expr.values:
59+
if isinstance(elt, ast.Constant) and isinstance(elt.value, str):
60+
parts.append(elt.value)
61+
else:
62+
parts.append("\x00") # dynamic segment
63+
return "".join(parts) if parts else ""
64+
return None
65+
66+
67+
def _looks_like_shell_command_literal(s: str) -> bool:
68+
"""True if this class-level string is plausibly a shell/CLI invocation (not IDs, tokens, paths)."""
69+
s = s.strip()
70+
if not s:
71+
return False
72+
if re.fullmatch(r"0x[0-9a-fA-F]+", s):
73+
return False
74+
# OS / config tokens such as PRETTY_NAME, VERSION_ID
75+
if re.fullmatch(r"[A-Z][A-Z0-9_]+", s):
76+
return False
77+
# Filenames / simple paths (no shell metacharacters)
78+
if "." in s and not re.search(r"[\s|;&$`]", s):
79+
return False
80+
if re.search(r"[\s|;&$`<>]", s):
81+
return True
82+
# Typical one-word inband commands: uptime, sysctl, dmesg, amd-smi, etc.
83+
if re.fullmatch(r"[a-z][a-z0-9_.-]*", s, flags=re.IGNORECASE):
84+
return True
85+
return False
86+
87+
88+
def _base_name(node: ast.expr) -> str | None:
89+
if isinstance(node, ast.Name):
90+
return node.id
91+
if isinstance(node, ast.Subscript):
92+
return _base_name(node.value)
93+
if isinstance(node, ast.Attribute):
94+
return node.attr
95+
return None
96+
97+
98+
def _is_collector_or_analyzer_class(cls: ast.ClassDef) -> bool:
99+
return cls.name.endswith("Collector") or cls.name.endswith("Analyzer")
100+
101+
102+
def _field_call_name(func: ast.expr) -> bool:
103+
if isinstance(func, ast.Name) and func.id == "Field":
104+
return True
105+
if isinstance(func, ast.Attribute) and func.attr == "Field":
106+
return True
107+
return False
108+
109+
110+
def _field_has_nonempty_description(call: ast.Call) -> bool:
111+
for kw in call.keywords:
112+
if kw.arg != "description" or kw.value is None:
113+
continue
114+
v = kw.value
115+
if isinstance(v, ast.Constant) and isinstance(v.value, str) and v.value.strip():
116+
return True
117+
return False
118+
119+
120+
def _check_cmd_prefixes(path: Path, tree: ast.Module) -> list[str]:
121+
"""Rule #1: warn when a command-like class attr is not ``CMD`` / ``CMD_*``."""
122+
msgs: list[str] = []
123+
for node in tree.body:
124+
# Keeps only classes whose names end with Collector or Analyzer (e.g. ProcessCollector, PcieAnalyzer).
125+
if not isinstance(node, ast.ClassDef) or not _is_collector_or_analyzer_class(node):
126+
continue
127+
for stmt in node.body:
128+
if not isinstance(stmt, ast.Assign) or len(stmt.targets) != 1:
129+
continue
130+
t = stmt.targets[0]
131+
if not isinstance(t, ast.Name):
132+
continue
133+
name = t.id
134+
if name.startswith("_") or name in _CMD_CHECK_SKIP_NAMES:
135+
continue
136+
if not _is_stringish(stmt.value):
137+
continue
138+
preview = _stringish_preview(stmt.value)
139+
if preview is None or not _looks_like_shell_command_literal(preview):
140+
continue
141+
if name == "CMD" or name.startswith("CMD_"):
142+
continue
143+
msgs.append(
144+
f"{path}:{stmt.lineno}: [{node.name}] command-like class attribute {name!r} "
145+
"should be renamed to CMD or to start with CMD_."
146+
)
147+
return msgs
148+
149+
150+
def _is_args_class(cls: ast.ClassDef) -> bool:
151+
if not cls.name.endswith("Args"):
152+
return False
153+
if not cls.bases:
154+
return False
155+
for b in cls.bases:
156+
bn = _base_name(b)
157+
if bn in ("BaseModel", "CollectorArgs", "AnalyzerArgs"):
158+
return True
159+
if bn and bn.endswith("Args"):
160+
return True
161+
return False
162+
163+
164+
def _annotation_mentions_classvar(ann: ast.expr | None) -> bool:
165+
if ann is None:
166+
return False
167+
if isinstance(ann, ast.Name) and ann.id == "ClassVar":
168+
return True
169+
if isinstance(ann, ast.Subscript):
170+
return _annotation_mentions_classvar(ann.value)
171+
if isinstance(ann, ast.Attribute) and ann.attr == "ClassVar":
172+
return True
173+
if isinstance(ann, ast.BinOp) and isinstance(ann.op, ast.BitOr):
174+
return _annotation_mentions_classvar(ann.left) or _annotation_mentions_classvar(ann.right)
175+
return False
176+
177+
178+
def _check_args_fields(path: Path, tree: ast.Module) -> list[str]:
179+
"""Rule #2: warn when Args fields lack ``Field`` with a non-empty ``description``."""
180+
msgs: list[str] = []
181+
for node in tree.body:
182+
if not isinstance(node, ast.ClassDef) or not _is_args_class(node):
183+
continue
184+
for stmt in node.body:
185+
if isinstance(stmt, ast.AnnAssign):
186+
if _annotation_mentions_classvar(stmt.annotation):
187+
continue
188+
if not isinstance(stmt.target, ast.Name):
189+
continue
190+
field_name = stmt.target.id
191+
if field_name.startswith("_") or field_name in ("model_config",):
192+
continue
193+
if stmt.value is None:
194+
msgs.append(
195+
f"{path}:{stmt.lineno}: [{node.name}] {field_name}: "
196+
"use Field(..., description='...') for every Args field."
197+
)
198+
continue
199+
if isinstance(stmt.value, ast.Call) and _field_call_name(stmt.value.func):
200+
if not _field_has_nonempty_description(stmt.value):
201+
msgs.append(
202+
f"{path}:{stmt.lineno}: [{node.name}] {field_name}: "
203+
"Field(...) must include a non-empty description= for help text."
204+
)
205+
else:
206+
msgs.append(
207+
f"{path}:{stmt.lineno}: [{node.name}] {field_name}: "
208+
"must assign pydantic Field(...) with description=."
209+
)
210+
elif isinstance(stmt, ast.Assign) and len(stmt.targets) == 1:
211+
t = stmt.targets[0]
212+
if not isinstance(t, ast.Name):
213+
continue
214+
field_name = t.id
215+
if field_name.startswith("_") or field_name in ("model_config",):
216+
continue
217+
val = stmt.value
218+
if isinstance(val, ast.Call) and _field_call_name(val.func):
219+
if not _field_has_nonempty_description(val):
220+
msgs.append(
221+
f"{path}:{stmt.lineno}: [{node.name}] {field_name}: "
222+
"Field(...) must include a non-empty description= for help text."
223+
)
224+
return msgs
225+
226+
227+
def main() -> None:
228+
if not PLUGIN_ROOT.is_dir():
229+
sys.stderr.write(f"warning: plugins directory not found: {PLUGIN_ROOT}\n")
230+
return
231+
232+
all_msgs: list[str] = []
233+
for path in sorted(PLUGIN_ROOT.rglob("*.py")):
234+
rel = path.relative_to(_REPO_ROOT)
235+
name = path.name
236+
try:
237+
src = path.read_text(encoding="utf-8")
238+
tree = ast.parse(src, filename=str(path))
239+
except (OSError, SyntaxError) as e:
240+
all_msgs.append(f"{rel}: could not parse: {e}")
241+
continue
242+
243+
if "collector" in name and name.endswith(".py"):
244+
all_msgs.extend(_check_cmd_prefixes(rel, tree))
245+
if "analyzer" in name and name.endswith(".py"):
246+
all_msgs.extend(_check_cmd_prefixes(rel, tree))
247+
248+
if name == "collector_args.py" or name == "analyzer_args.py":
249+
all_msgs.extend(_check_args_fields(rel, tree))
250+
251+
if all_msgs:
252+
sys.stderr.write("plugin convention warnings (commit not blocked):\n")
253+
for m in all_msgs:
254+
sys.stderr.write(f" WARNING: {m}\n")
255+
else:
256+
sys.stdout.write("Success: no plugin convention warnings.\n")
257+
sys.exit(0)
258+
259+
260+
if __name__ == "__main__":
261+
main()

0 commit comments

Comments
 (0)