Skip to content

Conversation

@renner
Copy link
Member

@renner renner commented Dec 3, 2025

This patch is enabling some system group related functionalities, so far:

  • List all system groups visible to the user
  • Create new system groups
  • List systems assigned to a group
  • Add and remove systems from groups

@cbbayburt
Copy link
Contributor

Thanks for the PR, @renner.

I think the next tool to add would be list_sysyems_in_group. For the other end of the relation, I'll add a system_groups attribute to get_system_details tool so the LLM can figure out the connection.

I think for now, these 2 tools would be sufficient. For later, we can consider adding/removing systems to groups, or managing groups, but since they're not daily/repeat operations, I wouldn't clutter the toolset with them for now.

Just one suggestion for docstring tool descriptions moving forward:

Between the current Returns: paragraphs and the following, I found out that the following works better:

async def get_system_details(system_identifier: Union[str, int], ctx: Context):
    """Gets details of the specified system.

    Args:
        system_identifier: The system name (e.g., "buildhost.example.com") or system ID (e.g., 1000010000).
        Prefer using numerical system IDs instead of system names when possible.
    Returns:
        An object that contains the following attributes of the system:
            - system_id: The numerical ID of the system within Uyuni server
            - system_name: The registered system name, usually its main FQDN
            - last_boot: The last boot time of the system known to Uyuni server
            - uuid: UUID of the system if it is a virtual instance, null otherwise.
            - cpu: An object with the following CPU attributes of the system:
                - family: The CPU family
                - mhz: The CPU clock speed
                - model: The CPU model
                - vendor: The CPU vendor
                - arch: The CPU architecture
            - installed_product: List of installed products on the system.
              You can use this field to identify what OS the system is running.

        An empty object is returned if the system cannot be found.

        Example:
            {
              "system_id": "100010001",
              "system_name": "opensuse.example.local",
              "last_boot": "2025-04-01T15:21:56Z",
              "uuid": "a8c3f40d-c1ae-406e-9f9b-96e7d5fdf5a3",
              "cpu": {
                "family": "15",
                "mhz": "1896.436",
                "model": "QEMU Virtual CPU",
                "vendor": "AuthenticAMD",
                "arch": "x86_64"
              }
            }
        """

Python type definitions don't make any sense to the LLM, because what it'll see is just JSON. So, for the "Returns:" clause, one clear natural language description including its attributes, followed by a clear JSON example works best.

@renner renner force-pushed the enable-system-groups branch from 94681c3 to f8f9640 Compare December 4, 2025 22:15
@renner
Copy link
Member Author

renner commented Dec 5, 2025

Thanks @cbbayburt, I added a commit to improve the Return: part of the docstring tool descriptions. Hope it looks good now. I might add some additional test cases, so far only group creation is covered.

@cbbayburt cbbayburt force-pushed the enable-system-groups branch 2 times, most recently from c263ec7 to 25b28f8 Compare December 9, 2025 17:05
@cbbayburt cbbayburt changed the base branch from main to minor-fixes-serverpy December 9, 2025 17:05
@cbbayburt cbbayburt marked this pull request as ready for review December 9, 2025 17:06
Copy link
Contributor

@ycedres ycedres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Also tested the changed test cases file and runs ok.

@cbbayburt cbbayburt force-pushed the minor-fixes-serverpy branch from b20cd83 to a8041b3 Compare December 11, 2025 13:07
Base automatically changed from minor-fixes-serverpy to main December 11, 2025 13:08
@cbbayburt cbbayburt force-pushed the enable-system-groups branch from 25b28f8 to 9635a2e Compare December 11, 2025 13:09
@cbbayburt cbbayburt merged commit 9635a2e into main Dec 11, 2025
@cbbayburt cbbayburt deleted the enable-system-groups branch December 11, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants