Skip to content

Conversation

@ksharlandjiev
Copy link
Contributor

Description

This PR adds a new SsmGetCommandInvocationOperator and enhances existing SSM components to provide better command output retrieval capabilities.

Changes

New Features

  • SsmGetCommandInvocationOperator: New operator to retrieve output and execution details from SSM command invocations
    • Supports retrieving output from specific instances or all instances
    • Returns structured data with stdout, stderr, execution times, and status
    • Handles errors gracefully with proper logging

Enhancements

  • SsmHook: Added get_command_invocation() and list_command_invocations() methods
  • SsmRunCommandOperator: Enhanced deferrable mode with proper AWS parameter passing to triggers
  • SsmRunCommandTrigger: Improved async execution and AWS connection parameter handling

Testing

  • Added unit tests for all new functionality
  • Updated the system test demonstrating end-to-end usage

Documentation

  • Updated RST documentation with usage examples

Use Cases

This enhancement enables several important use cases:

  1. Command Output Retrieval: Get stdout/stderr from SSM commands for downstream processing
  2. Debugging: Inspect command execution details for troubleshooting
  3. Data Processing: Use SSM command output as input for subsequent tasks
  4. Monitoring: Track command execution status and performance metrics

Example Usage

# Run a command
run_command = SsmRunCommandOperator(
    task_id="run_command",
    document_name="AWS-RunShellScript",
    run_command_kwargs={
        "InstanceIds": ["i-1234567890abcdef0"],
        "Parameters": {"commands": ["echo 'Hello World'"]}
    }
)

# Get the output
get_output = SsmGetCommandInvocationOperator(
    task_id="get_output",
    command_id="{{ ti.xcom_pull(task_ids='run_command') }}",
    instance_id="i-1234567890abcdef0"
)

Kamen Sharlandjiev added 8 commits October 17, 2025 10:02
This operator allows users to retrieve the output and execution details
from SSM commands, supporting both single instance and all instances scenarios.

Features:
- Retrieve stdout/stderr from SSM command executions
- Support for specific instance or all instances
- Structured output with execution metadata
- Comprehensive error handling

Includes unit tests, documentation updates, and system test integration.
…mode

Fixed deferrable SSM commands failing due to missing region configuration:
  * Fix NoRegionError in SsmRunCommandOperator when using deferrable=True
  * Add region_name, verify, and botocore_config parameters to SsmRunCommandTrigger constructor
  * Update SsmRunCommandOperator to pass AWS connection parameters to trigger
  * Add unit tests for trigger serialization and parameter passing

The SsmRunCommandOperator was failing in deferrable mode with 'NoRegionError: You must specify a region'
because the trigger was not receiving AWS connection parameters from the operator. This change ensures
all AWS configuration (region_name, verify, botocore_config) is properly passed to the trigger and
maintains backward compatibility.
- Ensure consistent return format.
- Update tests to match standardised output structure

The operator now consistently returns the same structure regardless of whether
instance_id is provided, making it easier to work with downstream tasks.
- Change SsmHook.list_command_invocations() to return raw AWS API response
- Update return type from list[dict] to dict for consistency with other AWS hooks
- Modify SsmGetCommandInvocationOperator to handle raw response format
- Update all unit tests to expect standardized response format

This aligns SSM hook with the established pattern used by EC2Hook, LambdaHook,
and other AWS hooks that return raw boto3 responses rather than processed data."
…xception` per latest contribution guidelines
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 21, 2025

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

Copy link
Contributor

@o-nikolas o-nikolas left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! Overall it looks quite good. Some feedback:

I don't love the mixture of changes. It's better to make your PRs as tightly scoped as possible, if you spot other things to fix try to do them in a separate PR.

You have some tests failing that need looking into, static checks can be run on your development machine (automatically as well as pre-commit/pre-push hooks), see the docs here for how to run those.

@ksharlandjiev ksharlandjiev marked this pull request as draft October 22, 2025 08:19
- Break long lines in SsmRunCommandOperator and SsmGetCommandInvocationOperator
- Split long method calls, dictionary configurations, and log messages
- Ensure all lines comply with 79-character limit per PEP 8
@ksharlandjiev
Copy link
Contributor Author

ksharlandjiev commented Oct 22, 2025

Thanks for the contribution! Overall it looks quite good. Some feedback:

I don't love the mixture of changes. It's better to make your PRs as tightly scoped as possible, if you spot other things to fix try to do them in a separate PR.

You have some tests failing that need looking into, static checks can be run on your development machine (automatically as well as pre-commit/pre-push hooks), see the docs here for how to run those.

Thank you for the kind words. Re: Mixture of changes: I understand, and I'll make sure all changes are in their own PR going forward. Please let me know if you want me to revert this commit, and move to a new PR, or I'll be granted exception this time. Thank you!

@ksharlandjiev ksharlandjiev marked this pull request as ready for review October 22, 2025 12:37
@o-nikolas
Copy link
Contributor

Thanks for the contribution! Overall it looks quite good. Some feedback:
I don't love the mixture of changes. It's better to make your PRs as tightly scoped as possible, if you spot other things to fix try to do them in a separate PR.
You have some tests failing that need looking into, static checks can be run on your development machine (automatically as well as pre-commit/pre-push hooks), see the docs here for how to run those.

Thank you for the kind words. Re: Mixture of changes: I understand, and I'll make sure all changes are in their own PR going forward. Please let me know if you want me to revert this commit, and move to a new PR, or I'll be granted exception this time. Thank you!

It's all good this time, no need to move to a new PR.

You still have some static checks failing though. Ruff would be automatically fixed for you on pre-commit/pre-push if you enable those :)

Copy link
Contributor

@ferruzzi ferruzzi left a comment

Choose a reason for hiding this comment

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

Looking good.

Copy link
Contributor

@ferruzzi ferruzzi left a comment

Choose a reason for hiding this comment

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

Nice, thanks for your work!

@o-nikolas o-nikolas merged commit fee683a into apache:main Oct 28, 2025
79 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Oct 28, 2025

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@ksharlandjiev
Copy link
Contributor Author

ksharlandjiev commented Oct 28, 2025

Nice, thanks for your work!

Thank you both for your leadership and guidance!

Lzzz666 pushed a commit to Lzzz666/airflow that referenced this pull request Oct 30, 2025
…e#56936)

Add SsmGetCommandInvocationOperator for retrieving SSM command output

This operator allows users to retrieve the output and execution details
from SSM commands, supporting both single instance and all instances scenarios.

Features:
- Retrieve stdout/stderr from SSM command executions
- Support for specific instance or all instances
- Structured output with execution metadata
- Comprehensive error handling

Includes unit tests, documentation updates, and system test integration.

* Pass AWS connection parameters to SsmRunCommandTrigger in deferrable mode

Fixed deferrable SSM commands failing due to missing region configuration:
  * Fix NoRegionError in SsmRunCommandOperator when using deferrable=True
  * Add region_name, verify, and botocore_config parameters to SsmRunCommandTrigger constructor
  * Update SsmRunCommandOperator to pass AWS connection parameters to trigger
  * Add unit tests for trigger serialization and parameter passing
@ksharlandjiev ksharlandjiev deleted the feature/ssm-get-command-invocation-operator branch October 31, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants