Post Action Plugin Infrastructure by amd-benprice · Pull Request #277 · amd/node-scraper

amd-benprice · 2026-08-31T20:33:44Z

Summary

This PR adds post-actions, which is a method of running more plugins based on the granular results of prior plugins. Post actions can be specified only through the plugin configuration JSON file. The README is updated with the specific semantics of how post actions can be configured. Any plugin can be used as a post-action plugin. This PR also adds unit tests for post actions, and updates one existing unit test.

This PR also contains a bugfix that prevented OS detection during remote execution. The fix was to not make a pydantic model copy of system_info in pluginexecutor.py, and instead use the pre-existing instance of the model. The copy would be update with the correct OS family, but the original, which was then passed to the plugins themselves, was not updated, which caused all plugins to not run.

Test plan

pytest test/unit
pytest test/functional (if applicable)
pre-commit run --all-files

Checklist

Added/updated tests (or explained why not)
Updated docs/README if behavior changed
No secrets or credentials committed

alexandraBara

Couple things to consider:

currently nothing prevents the same plugin from appearing twice in post actions with diff conditions
PluginConfig.merge() drops post-actions in pluginrecipe.py. What do we do with this? do we allow postconditions in recipes?
This merges post_actions_plugins: PluginExecutor.merge_configs() but this one does not: PluginConfig.merge() (recipes)

alexandraBara · 2026-09-01T13:19:45Z

+#
+# MIT License
+#
+# Copyright (c) 2025 Advanced Micro Devices, Inc.


new files must have 2026

Fixed in 3cc2d56 and b845a9d

alexandraBara · 2026-09-01T13:52:25Z

+    """If set, only inspect the PluginResult whose ``source`` matches this name.
+    If None, all results are candidates."""
+
+    status: Optional[str] = None


this type should be ExecutionStatus

Fixed in 3cc2d56

alexandraBara · 2026-09-01T13:52:39Z

+    Accepts any :class:`~nodescraper.enums.ExecutionStatus` name
+    (e.g. ``"WARNING"``, ``"ERROR"``, ``"EXECUTION_FAILURE"``)."""
+
+    event_category: Optional[str] = None


this should also be EventPriority not str

Assuming you meant event_priority with this comment, which is fixed in 3cc2d56

alexandraBara · 2026-09-01T13:53:43Z

+    """If set, only inspect the PluginResult whose ``source`` matches this name.
+    If None, all results are candidates."""
+
+    status: Optional[str] = None


Suggested change

status: Optional[str] = None

status: Optional[ExecutionStatus] = None

Fixed in 3cc2d56

alexandraBara · 2026-09-01T13:55:25Z

+    def _matches_result(self, result: PluginResult) -> bool:
+        """Return True if *result* satisfies all specified fields (AND logic).
+
+        Each field that is not None must be satisfied; unset fields are skipped.
+        """
+        # --- status check ---
+        if self.status is not None:
+            try:
+                status_threshold = ExecutionStatus[self.status.upper()]
+            except KeyError:
+                return False
+            if result.status < status_threshold:
+                return False


once the correct type is set for status this function can probably just be:

Suggested change

def _matches_result(self, result: PluginResult) -> bool:

"""Return True if *result* satisfies all specified fields (AND logic).

Each field that is not None must be satisfied; unset fields are skipped.

"""

# --- status check ---

if self.status is not None:

try:

status_threshold = ExecutionStatus[self.status.upper()]

except KeyError:

return False

if result.status < status_threshold:

return False

def _matches_result(self, result: PluginResult) -> bool:

"""Return True if *result* satisfies all specified fields (AND logic).

if self.status is not None and result.status < self.status:

return False

Fixed in 3cc2d56

alexandraBara · 2026-09-01T13:57:47Z

+    event_description_contains: Optional[str] = None
+    """If set, at least one event's description must contain this substring
+    (case-sensitive)."""
+


Once the types are fixed, i recommend you add these too:

@field_validator("status", mode="before") @classmethod def _coerce_status(cls, v): # mirror TaskResult.validate_status ... @field_validator("event_priority", mode="before") @classmethod def _coerce_priority(cls, v): # mirror Event.validate_priority (or import shared helper) ...

Fixed in 3cc2d56

alexandraBara · 2026-09-01T14:02:04Z

+        }
+    """
+
+    plugin: str


i think this needs a @field_validator to sanitize input like "" or " ". Maybe something like:

@field_validator("plugin") @classmethod def _strip_and_require_plugin(cls, v: str) -> str: v = v.strip() if not v: raise ValueError("plugin name must not be empty") return v

Fixed in 3cc2d56

amd-benprice · 2026-09-01T16:41:46Z

Couple things to consider:

currently nothing prevents the same plugin from appearing twice in post actions with diff conditions

PluginConfig.merge() drops post-actions in pluginrecipe.py. What do we do with this? do we allow postconditions in recipes?

This merges post_actions_plugins: PluginExecutor.merge_configs() but this one does not: PluginConfig.merge() (recipes)

For 1, I think this is fine, since users could also just specify the same plugin multiple times in their config with different args, so I think this fits that same usage model.

For 2+3, post actions should be allowed in recipes in my opinion, and I have made the changes you suggested to allow for this, and added some tests in 3cc2d56

amd-benprice added 2 commits August 26, 2026 18:07

Initial commit for post actions

ff59341

bugfix for remote execution

b9b8e92

amd-benprice requested review from alexandraBara, graepaul and landrews-amd August 31, 2026 20:33

amd-benprice self-assigned this Aug 31, 2026

github-actions Bot added documentation Improvements or additions to documentation tests framework labels Aug 31, 2026

amd-benprice and others added 2 commits August 31, 2026 16:37

Merge branch 'development' into ben_post_actions

e71a08b

Address pre-commit issues

5a93be8

alexandraBara requested changes Sep 1, 2026

View reviewed changes

amd-benprice and others added 3 commits September 1, 2026 12:29

Address PR comments

3cc2d56

Merge branch 'development' into ben_post_actions

98d77e2

Missed one license year

b845a9d

amd-benprice requested a review from alexandraBara September 1, 2026 17:50

	status: Optional[str] = None
	status: Optional[ExecutionStatus] = None

Uh oh!

Conversation

amd-benprice commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Checklist

Uh oh!

alexandraBara left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

amd-benprice commented Sep 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

amd-benprice commented Aug 31, 2026 •

edited

Loading