You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "antonio-mello-ai (via GitHub)" <gi...@apache.org> on 2026/03/12 20:10:29 UTC
[PR] fix(providers/amazon): handle multi-node job log streams in BatchClientHook [airflow]
antonio-mello-ai opened a new pull request, #63476:
URL: https://github.com/apache/airflow/pull/63476
## Summary
- Fix `get_job_all_awslogs_info()` to extract log stream names from both single-node (`attempts[].container.logStreamName`) and multi-node (`attempts[].taskProperties[].containers[].logStreamName`) AWS Batch job schemas
- Filter out `None` values that previously bypassed the `if not stream_names:` guard, causing `TypeError` in `urllib.parse.quote_plus()` downstream
## Root Cause
Multi-node AWS Batch jobs return log stream names under `attempts[].taskProperties[].containers[].logStreamName`, not the single-node path `attempts[].container.logStreamName`. The existing code only checked the single-node path, producing `stream_names = [None]` for multi-node jobs. Since `[None]` is truthy, it passed the emptiness guard and propagated `None` downstream.
## Test Plan
- [x] New test: multi-node job with `taskProperties` schema extracts stream names correctly
- [x] New test: multi-node job with no `logStreamName` in `container` falls through to warning path
- [x] All 41 tests pass (39 existing + 2 new)
- [x] All pre-commit hooks pass (`prek`)
Closes #54254
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/amazon): handle multi-node job log streams in BatchClientHook [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #63476:
URL: https://github.com/apache/airflow/pull/63476#issuecomment-4057469723
@o-nikolas — Could you please check whether your review feedback on this PR has been addressed? @antonio-mello-ai appears to have responded to your comments. @antonio-mello-ai, do you believe the reviewer's concerns have been resolved?
If the concerns are resolved, please resolve the conversation threads. Thank you!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/amazon): handle multi-node job log streams in BatchClientHook [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #63476:
URL: https://github.com/apache/airflow/pull/63476
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/amazon): handle multi-node job log streams in BatchClientHook [airflow]
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on PR #63476:
URL: https://github.com/apache/airflow/pull/63476#issuecomment-4070233073
Just getting back to this monday, looks all good to me
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/amazon): handle multi-node job log streams in BatchClientHook [airflow]
Posted by "antonio-mello-ai (via GitHub)" <gi...@apache.org>.
antonio-mello-ai commented on code in PR #63476:
URL: https://github.com/apache/airflow/pull/63476#discussion_r2928038560
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/batch_client.py:
##########
@@ -476,8 +476,19 @@ def get_job_all_awslogs_info(self, job_id: str) -> list[dict[str, str]]:
p.get("container", {}).get("logConfiguration", {})
for p in job_node_properties.get("nodeRangeProperties", {})
]
- # one stream name per attempt
- stream_names = [a.get("container", {}).get("logStreamName") for a in job_desc.get("attempts", [])]
+ # one stream name per attempt — handles both single-node and multi-node schemas
+ stream_names = []
+ for attempt in job_desc.get("attempts", []):
+ # Single-node: attempts[].container.logStreamName
+ container_stream = attempt.get("container", {}).get("logStreamName")
+ if container_stream:
+ stream_names.append(container_stream)
Review Comment:
Good call — added `continue` in 85432c7. Thanks for the review\!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/amazon): handle multi-node job log streams in BatchClientHook [airflow]
Posted by "o-nikolas (via GitHub)" <gi...@apache.org>.
o-nikolas commented on code in PR #63476:
URL: https://github.com/apache/airflow/pull/63476#discussion_r2928001989
##########
providers/amazon/src/airflow/providers/amazon/aws/hooks/batch_client.py:
##########
@@ -476,8 +476,19 @@ def get_job_all_awslogs_info(self, job_id: str) -> list[dict[str, str]]:
p.get("container", {}).get("logConfiguration", {})
for p in job_node_properties.get("nodeRangeProperties", {})
]
- # one stream name per attempt
- stream_names = [a.get("container", {}).get("logStreamName") for a in job_desc.get("attempts", [])]
+ # one stream name per attempt — handles both single-node and multi-node schemas
+ stream_names = []
+ for attempt in job_desc.get("attempts", []):
+ # Single-node: attempts[].container.logStreamName
+ container_stream = attempt.get("container", {}).get("logStreamName")
+ if container_stream:
+ stream_names.append(container_stream)
Review Comment:
Feels more natural to just continue here, no? The below for loop will not get anything to iterate over but feels odd to let it attempt even thought we already found a match.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(providers/amazon): handle multi-node job log streams in BatchClientHook [airflow]
Posted by "antonio-mello-ai (via GitHub)" <gi...@apache.org>.
antonio-mello-ai commented on PR #63476:
URL: https://github.com/apache/airflow/pull/63476#issuecomment-4057688543
@potiuk Yes, the reviewer's concern has been addressed — added `continue` in 85432c7 as @o-nikolas suggested. I've resolved the conversation thread. Thanks for following up\!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org