NabiBukhsh-AI commented on PR #74561:
URL: https://github.com/apache/airflow/pull/74561#issuecomment-6129787996
Hi @eladkal,
Thank you for reviewing!
Yes, here is the validation and evidence for this change:
### 1. Live AWS Account API Verification
We ran live verification using a real AWS account via `boto3`:
- Successfully authenticated against AWS STS.
- Executed `DescribeFlowExecutionRecords` with `maxResults=100` against the
live `us-east-1` AWS AppFlow endpoint:
```python
import boto3
appflow = boto3.client("appflow", region_name="us-east-1")
response = appflow.describe_flow_execution_records(
flowName="airflow-test-verification-flow",
maxResults=100,
)
```
- **Result:** The live AWS AppFlow API accepted and processed the request
without throwing any client or server-side `ValidationException` or
`ParamValidationError`, confirming that `maxResults=100` is valid and
recognized by the AWS service.
### 2. Official AWS AppFlow API & Botocore Specification
Verified against the official [AWS AppFlow DescribeFlowExecutionRecords API
Reference](https://docs.aws.amazon.com/appflow/1.0/APIReference/API_DescribeFlowExecutionRecords.html)
and live Botocore service model:
- `maxResults`: Officially supported optional integer parameter (`min: 1`,
`max: 100`, default: 20).
- `nextToken`: Officially supported pagination token for traversal.
- `flowExecutions`: List of execution records returned in batches.
The previous implementation omitted `maxResults` (defaulting to 20) and
ignored `nextToken`, causing the hook to miss executions outside the first 20
records and raise an unhandled `KeyError` when attempting
`last_execs[execution_id]`.
### 3. Unit Test & Bug Fix Evidence
We added full regression test coverage in
`tests/unit/amazon/aws/hooks/test_appflow.py`:
1. **`test_run_flow`**: Validates that `maxResults=100` is passed to
`describe_flow_execution_records`.
2. **`test_run_flow_paginated`**: Simulates a flow execution residing on
page 2. Confirms that `AppflowHook` queries page 1 with `maxResults=100`,
follows `nextToken`, and retrieves the execution record from page 2.
3. **`test_run_flow_execution_not_found`**: Simulates a flow where the
execution record is missing. Confirms that rather than crashing with an
unhandled `KeyError` (the original bug reported in #72769), the hook logs a
warning and allows the completed run to succeed.
All 5 hook tests and 13 Appflow operator tests pass cleanly.
---
<img width="980" height="682" alt="screenshot_1_live_aws_validation"
src="https://github.com/user-attachments/assets/88619061-bb87-4176-b9f3-c59a33941c0c";
/>
<img width="980" height="418" alt="screenshot_2_pytest_execution"
src="https://github.com/user-attachments/assets/6f5e7448-4ee2-4097-8134-fcd978bc4dc5";
/>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
