fix: container_name is null for ecs integration#1592
Conversation
✅ Deploy Preview for sunny-pastelito-5ecb04 canceled.
|
tatiana
left a comment
There was a problem hiding this comment.
Hi @nicor88,
Thank you very much for reporting the issue and fixing it.
This seems to be an info message:
[2025-03-07, 16:40:34 UTC] {ecs.py:515} INFO - EcsOperator overrides: {'containerOverrides': [{'name': None, 'command': ['dbt', '--no-partial-parse', 'run', '--models', 'my_first_dbt_model'], 'environment': [{'name': 'AIRFLOW_CTX_DAG_OWNER', 'value': '***'}, {'name': 'AIRFLOW_CTX_DAG_ID', 'value': 'example_cosmos'}, {'name': 'AIRFLOW_CTX_TASK_ID', 'value': 'dbt_task_group.my_first_dbt_model.run'}, {'name': 'AIRFLOW_CTX_EXECUTION_DATE', 'value': '2025-03-07T16:40:31.716155+00:00'}, {'name': 'AIRFLOW_CTX_TRY_NUMBER', 'value': '1'}, {'name': 'AIRFLOW_CTX_DAG_RUN_ID', 'value': 'manual__2025-03-07T16:40:31.716155+00:00'}, {'name': 'EXTRA_VAR', 'value': 'extra_value'}]}]}
Could you share the actual Boto error you faced, with stacktrace? It may be worth to share the versions being used as well (both of Boto, as well as the AWS Airflow provider.
Based on the ECS documentation, it does seem that the container name is mandatory: https://docs.aws.amazon.com/AmazonECS/latest/developerguide/create-task-definition.html#json-validate-for-create
- For each container to define in your task definition, complete the following steps. For Name, enter a name for the container.
That said, it also seems that the Airflow operator handles this:
:param container_name: The name of the container to fetch logs from. If not set, the first container is used.
@nicor88 would it make sense for us to calculate the container dynamically? If this field is mandatory, I'm worried how it would work with DbtTaskGroup and DbtDag.
@aoelvp94 @CarlosGitto please, could you help reviewing this change? Did you ever face a similar issue, do you have any recommendations?
|
@tatiana I pretty I should have covered your proposal in this commend #1592 (comment) The point is that the EcsRunTaskOperator doesn't fully handle container_name in the way that we want because cosmos pass the "container" overwrides that must contains which command to execute and the container_name. I'm wondering what is wrong with the proposed solution? Also the issue seems due to the fact that in the previous amazon airflow provider version (see this) the issue was not showing up. Based on slack discussion with @aoelvp94 seems that they are not facing any issue.
Also, what we can do it's to setup a default value for I also added a full-logs version in the PR above. aiobotocore 2.19.0 boto3 1.36.3 botocore 1.36.3 |
|
@nicor88 thank you very much for your explanation. We still need to solve the tests, our team will try to get the CI checks to pass before we merge this PR, so it can be released in 1.9.1. |
Co-authored-by: marianore-muttdata <115091420+marianore-muttdata@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1592 +/- ##
=======================================
Coverage 97.43% 97.43%
=======================================
Files 80 80
Lines 4944 4946 +2
=======================================
+ Hits 4817 4819 +2
Misses 127 127 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@pankajastro maybe you missed this: #1592 (comment) but not sure why you put back |
What version of airflow-providers-amazon provider were you using for your testing? |
@pankajastro |
## Description
### TL/DR
* pas `container_name` to kwargs
* use a default value for **aws_conn_id**
### Long version
The current implementation of ECS integration implies passing the
`container_name` as part of the operator_args.
e.g.
```
operator_args = {
"container_name": "main",
...
}
```
Anyhow, this lead to errors like this:
```
[2025-03-07, 16:40:34 UTC] {ecs.py:515} INFO - EcsOperator overrides: {'containerOverrides': [{'name': None, 'command': ['dbt', '--no-partial-parse', 'run', '--models', 'my_first_dbt_model'], 'environment': [{'name': 'AIRFLOW_CTX_DAG_OWNER', 'value': '***'}, {'name': 'AIRFLOW_CTX_DAG_ID', 'value': 'example_cosmos'}, {'name': 'AIRFLOW_CTX_TASK_ID', 'value': 'dbt_task_group.my_first_dbt_model.run'}, {'name': 'AIRFLOW_CTX_EXECUTION_DATE', 'value': '2025-03-07T16:40:31.716155+00:00'}, {'name': 'AIRFLOW_CTX_TRY_NUMBER', 'value': '1'}, {'name': 'AIRFLOW_CTX_DAG_RUN_ID', 'value': 'manual__2025-03-07T16:40:31.716155+00:00'}, {'name': 'EXTRA_VAR', 'value': 'extra_value'}]}]}
```
The container name is None, leading to a failure in how boto3 invokes
the container.
The issue was due to the fact that `container_name` was not passed to
the kwargs, therefore, the container_name was not set properly to the
value that was set to cosmos.
### Full logs
<pre>
2025-03-10, 12:49:41 UTC] {ecs.py:512} INFO - Running ECS Task - Task
definition: dbt - on cluster dbt
[2025-03-10, 12:49:41 UTC] {ecs.py:515} INFO - EcsOperator overrides:
{'containerOverrides': [{'name': None, 'command': ['dbt',
'--no-partial-parse', 'run', '--models', 'my_first_dbt_model'],
'environment': [{'name': 'AIRFLOW_CTX_DAG_OWNER', 'value': '***'},
{'name': 'AIRFLOW_CTX_DAG_ID', 'value': 'example_cosmos'}, {'name':
'AIRFLOW_CTX_TASK_ID', 'value':
'dbt_task_group.my_first_dbt_model.run'}, {'name':
'AIRFLOW_CTX_EXECUTION_DATE', 'value':
'2025-03-10T12:49:28.878404+00:00'}, {'name': 'AIRFLOW_CTX_TRY_NUMBER',
'value': '1'}, {'name': 'AIRFLOW_CTX_DAG_RUN_ID', 'value':
'manual__2025-03-10T12:49:28.878404+00:00'}, {'name': 'EXTRA_VAR',
'value': 'extra_value'}]}]}
[2025-03-10, 12:49:41 UTC] {base.py:84} INFO - Retrieving connection
'aws_default'
[2025-03-10, 12:49:44 UTC] {credentials.py:1147} INFO - Found
credentials in environment variables.
[2025-03-10, 12:49:44 UTC] {taskinstance.py:3313} ERROR - Task failed
with exception
Traceback (most recent call last):
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/models/taskinstance.py",
line 768, in _execute_task
result = _execute_callable(context=context, **execute_callable_kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/models/taskinstance.py",
line 734, in _execute_callable
return ExecutionCallableRunner(
^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/utils/operator_helpers.py",
line 252, in run
return self.func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/cosmos/operators/base.py",
line 278, in execute
self.build_and_run_cmd(context=context, cmd_flags=self.add_cmd_flags())
File
"/home/airflow/.local/lib/python3.12/site-packages/cosmos/operators/aws_ecs.py",
line 98, in build_and_run_cmd
result = EcsRunTaskOperator.execute(self, context)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/models/baseoperator.py",
line 424, in wrapper
return func(self, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/providers/amazon/aws/operators/ecs.py",
line 526, in execute
self._start_task()
File
"/home/airflow/.local/lib/python3.12/site-packages/airflow/providers/amazon/aws/operators/ecs.py",
line 626, in _start_task
response = self.client.run_task(**run_opts)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/botocore/client.py",
line 569, in _api_call
return self._make_api_call(operation_name, kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/botocore/client.py",
line 980, in _make_api_call
request_dict = self._convert_to_request_dict(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/botocore/client.py",
line 1047, in _convert_to_request_dict
request_dict = self._serializer.serialize_to_request(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File
"/home/airflow/.local/lib/python3.12/site-packages/botocore/validate.py",
line 381, in serialize_to_request
raise ParamValidationError(report=report.generate_report())
botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter overrides.containerOverrides[0].name, value:
None, type: <class 'NoneType'>, valid types: <class 'str'>
</pre>
## Related Issue(s)
I didn't created any issue - but I just thought to propose a fix.
## Breaking Change?
It does because user have to use `dbt_container_name` with ECS, but it's
currently broken.
## Checklist
- [ ] I have made corresponding changes to the documentation (if
required)
- [ ] I have added tests that prove my fix is effective or that my
feature works
(cherry picked from commit 483ca7c)
Hmm, could you please test it with an older version like 9.0.0, if it works, happy to remove? I believe since this param was introduced in |
hi @nicor88 , like @pankajastro mentioned apache/airflow#46152 was only release recently in Airflow Amazon provider release 9.4.0, that is not available in previous versions of the Amazon provider. Our CI runs tests on multiple versions of Apache Airflow and we install the provider versions as per the constraints file available for that version of the Airflow installation. Hence, as you can see the PR tests failed earlier at https://github.com/astronomer/astronomer-cosmos/actions/runs/13830044214/job/38692107021#step:6:1083 for Airflow version 2.6 which installed Amazon provider 8.20.0 that does not have container_name property being assigned via kwargs in our case. |
|
Thanks for the context @pankajastro and @pankajkoti My point was simply that the self.container_name=container_name was redundant, but that was based on the latest aws providers, as we need to be back-compatible, then I suppose that leave in it's totally fine - actually a must, thanks for the full context. |
Bug Fixes * Fix import error in dbt bigquery adapter mock for ``dbt-bigquery<1.8`` for ``ExecutionMode.AIRFLOW_ASYNC`` by @pankajkoti in #1548 * Fix ``operator_args`` override configuration by @ghjklw in #1558 * Fix missing ``install_dbt_deps`` in ``ProjectConfig`` ``__init__`` method by @ghjklw in #1556 * Fix dbt project parsing ``dbt_vars`` behavior passed via ``operator_args`` by @AlexandrKhabarov in #1543 * Avoid reading the connection during DAG parsing of the async BigQuery operator by @joppevos in #1582 * Fix: Workaround to incorrectly raised ``gcsfs.retry.HttpError`` (Invalid Credentials, 401) by @tatiana in #1598 * Fix the async execution mode read sql files for dbt packages by @pankajastro in #1588 * Improve BQ async error handling by @tatiana in #1597 * Fix path selector when ``manifest.json`` is created using MS Windows by @tatiana in #1601 * Fix log that prints 'Total filtered nodes' by @tatiana in #1603 * Fix select behaviour using ``LoadMode.MANIFEST`` and a path with star by @tatiana in #1602 * Support ``on_warning_callback`` with ``TestBehavior.BUILD`` and ``ExecutionMode.LOCAL`` by @corsettigyg in #1571 * Fix ``DbtRunLocalOperator.partial()`` support by @tatiana @ashb in #1609 * fix: ``container_name`` is null for ecs integration by @nicor88 in #1592 Docs * Improve MWAA getting-started docs by removing unused imports by @jx2lee in #1562 Others * Disable ``example_cosmos_dbt_build.py`` DAG in CI by @pankajastro in #1567 * Upgrade GitHub Actions Ubuntu version by @tatiana in #1561 * Update GitHub bug issue template by @pankajastro in #1586 * Enable DAG ``example_cosmos_dbt_build.py`` in CI by @pankajastro in #1573 * Run async DAG in DAG without setup/teardown task by @pankajastro in #1599 * Add test case that fully covers recent select issue by @tatiana in #1604 * Add CI job to test multiple dbt versions for the async DAG by @pankajkoti in #1535 * Improve unit tests speed from 89s to 14s by @tatiana in #1600 * Pre-commit updates: #1560, #1583, #1596 Closes: #1550 Mergeable version of #1607 Co-authored-by: Pankaj Singh <98807258+pankajastro@users.noreply.github.com> Co-authored-by: Pankaj Koti <pankajkoti699@gmail.com>
Description
TL/DR
container_nameto kwargsLong version
The current implementation of ECS integration implies passing the
container_nameas part of the operator_args.e.g.
Anyhow, this lead to errors like this:
The container name is None, leading to a failure in how boto3 invokes the container.
The issue was due to the fact that
container_namewas not passed to the kwargs, therefore, the container_name was not set properly to the value that was set to cosmos.Full logs
2025-03-10, 12:49:41 UTC] {ecs.py:512} INFO - Running ECS Task - Task definition: dbt - on cluster dbt [2025-03-10, 12:49:41 UTC] {ecs.py:515} INFO - EcsOperator overrides: {'containerOverrides': [{'name': None, 'command': ['dbt', '--no-partial-parse', 'run', '--models', 'my_first_dbt_model'], 'environment': [{'name': 'AIRFLOW_CTX_DAG_OWNER', 'value': '***'}, {'name': 'AIRFLOW_CTX_DAG_ID', 'value': 'example_cosmos'}, {'name': 'AIRFLOW_CTX_TASK_ID', 'value': 'dbt_task_group.my_first_dbt_model.run'}, {'name': 'AIRFLOW_CTX_EXECUTION_DATE', 'value': '2025-03-10T12:49:28.878404+00:00'}, {'name': 'AIRFLOW_CTX_TRY_NUMBER', 'value': '1'}, {'name': 'AIRFLOW_CTX_DAG_RUN_ID', 'value': 'manual__2025-03-10T12:49:28.878404+00:00'}, {'name': 'EXTRA_VAR', 'value': 'extra_value'}]}]} [2025-03-10, 12:49:41 UTC] {base.py:84} INFO - Retrieving connection 'aws_default' [2025-03-10, 12:49:44 UTC] {credentials.py:1147} INFO - Found credentials in environment variables. [2025-03-10, 12:49:44 UTC] {taskinstance.py:3313} ERROR - Task failed with exception Traceback (most recent call last): File "/home/airflow/.local/lib/python3.12/site-packages/airflow/models/taskinstance.py", line 768, in _execute_task result = _execute_callable(context=context, **execute_callable_kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/airflow/models/taskinstance.py", line 734, in _execute_callable return ExecutionCallableRunner( ^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/airflow/utils/operator_helpers.py", line 252, in run return self.func(*args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/cosmos/operators/base.py", line 278, in execute self.build_and_run_cmd(context=context, cmd_flags=self.add_cmd_flags()) File "/home/airflow/.local/lib/python3.12/site-packages/cosmos/operators/aws_ecs.py", line 98, in build_and_run_cmd result = EcsRunTaskOperator.execute(self, context) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/airflow/models/baseoperator.py", line 424, in wrapper return func(self, *args, **kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/airflow/providers/amazon/aws/operators/ecs.py", line 526, in execute self._start_task() File "/home/airflow/.local/lib/python3.12/site-packages/airflow/providers/amazon/aws/operators/ecs.py", line 626, in _start_task response = self.client.run_task(**run_opts) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/botocore/client.py", line 569, in _api_call return self._make_api_call(operation_name, kwargs) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/botocore/client.py", line 980, in _make_api_call request_dict = self._convert_to_request_dict( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/botocore/client.py", line 1047, in _convert_to_request_dict request_dict = self._serializer.serialize_to_request( ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/home/airflow/.local/lib/python3.12/site-packages/botocore/validate.py", line 381, in serialize_to_request raise ParamValidationError(report=report.generate_report()) botocore.exceptions.ParamValidationError: Parameter validation failed: Invalid type for parameter overrides.containerOverrides[0].name, value: None, type: , valid types:Related Issue(s)
I didn't created any issue - but I just thought to propose a fix.
Breaking Change?
It does because user have to use
dbt_container_namewith ECS, but it's currently broken.Checklist