Skip to content

Conversation

@elazar
Copy link

@elazar elazar commented Nov 2, 2025

Description

Adds comprehensive guidance to the connector documentation on proper command wrapping and connector control parameter filtering. This addresses a documentation gap that can lead to TypeError exceptions in custom connectors.

What's Added

A new section "Implementing run_shell_command" that covers:

  • Using make_unix_command_for_host() for proper shell wrapping (essential for shell operators like &&, ||, pipes)
  • Filtering connector control parameters with extract_control_arguments() utility
  • Why control parameters (_success_exit_codes, _timeout, _get_pty, _stdin) must be filtered before calling make_unix_command_for_host()
  • Complete working example following PyInfra best practices
  • References to built-in connectors (docker, chroot, ssh, local) as reference implementations

Why This Matters

Without this guidance, custom connector developers may:

  1. Pass control parameters directly to make_unix_command_for_host(), causing TypeError: unexpected keyword argument
  2. Manually reimplement parameter filtering instead of using the existing extract_control_arguments() utility
  3. Skip command wrapping entirely, causing shell operators to fail

This documentation evolved from debugging and fixing parameter handling bugs in the pyinfra-orbstack third-party connector.

Real-World Impact

This pattern is already used by PyInfra's built-in connectors (docker, dockerssh, chroot), but isn't documented for third-party connector authors. Making this explicit will help prevent bugs and ensure consistency across the connector ecosystem.


Checklist

  • Pull request is based on the default branch (3.x at this time)
  • Pull request includes tests for any new/updated operations/facts (N/A - documentation only)
  • Pull request includes documentation for any new/updated operations/facts (This IS the documentation)
  • Tests pass (see scripts/dev-test.sh) (Will run after PR submission)
  • Type checking & code style passes (see scripts/dev-lint.sh) (Will run after PR submission)

Additional Context

The extract_control_arguments() utility function already exists in pyinfra.connectors.util and is used by several built-in connectors, but wasn't documented in the connector development guide. This PR makes that pattern explicit and recommended for all custom connectors.

Example of the Problem

Without proper parameter filtering:

# ❌ This will fail with TypeError
wrapped_command = make_unix_command_for_host(
    self.state,
    self.host,
    command,
    _success_exit_codes=[0, 1],  # ERROR: unexpected keyword argument
    **other_args,
)

With the documented pattern:

# ✅ This works correctly
control_args = extract_control_arguments(arguments)
wrapped_command = make_unix_command_for_host(
    self.state,
    self.host,
    command,
    **arguments,  # control params already extracted
)

Add comprehensive guidance for custom connector implementations on:
- Using make_unix_command_for_host() for proper shell wrapping
- Filtering connector control parameters with extract_control_arguments()
- Why command wrapping matters for shell operators
- Example implementation following PyInfra best practices
- References to built-in connector implementations

This addresses a documentation gap where connector developers may not
be aware of the need to filter control parameters (_success_exit_codes,
_timeout, _get_pty, _stdin) before calling make_unix_command_for_host().

Without proper filtering, connectors will encounter TypeError when these
parameters are passed to make_unix_command(), which does not accept them.

The new section recommends using the existing extract_control_arguments()
utility from pyinfra.connectors.util, which is already used by built-in
connectors like docker, dockerssh, and chroot.
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.

1 participant