Skip to content

Commit

Permalink
Clear out 'dependsOn' when creating a default ECS task definition (#6929
Browse files Browse the repository at this point in the history
)

Summary:
https://github.com/dagster-io/dagster/pull/6850/files made it so that we no longer include by default the containers that are included in the task definition. My guess from #6926 is that some users have a task definition with multiple containers *and* container dependencies, so taking out the containers left these dangling container

Test Plan: Create a task definition with two containers and a dependsOn in it, use it to launch an ECS run using the deploy_ecs example
  • Loading branch information
gibsondan committed Mar 16, 2022
1 parent b3e776b commit 9941cbc
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ def default_ecs_task_definition(
# entryPoint and containerOverrides are specified, they're concatenated
# and the command will fail
# https://aws.amazon.com/blogs/opensource/demystifying-entrypoint-cmd-docker/
container_definitions = task_definition["containerDefinitions"]
container_definitions = [
merge_dicts(
{
Expand All @@ -71,6 +70,7 @@ def default_ecs_task_definition(
"image": image,
"entryPoint": [],
"command": command if command else [],
"dependsOn": [], # Remove any other container dependencies as well
},
secrets or {},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,23 @@ def task_definition(ecs, image, environment):
return ecs.register_task_definition(
family="dagster",
containerDefinitions=[
{"name": "dagster", "image": image, "environment": environment, "entryPoint": ["ls"]},
{"name": "other", "image": image, "entryPoint": ["ls"]},
{
"name": "dagster",
"image": image,
"environment": environment,
"entryPoint": ["ls"],
"dependsOn": [
{
"containerName": "other",
"condition": "SUCCESS",
},
],
},
{
"name": "other",
"image": image,
"entryPoint": ["ls"],
},
],
networkMode="awsvpc",
memory="512",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def test_default_launcher(
assert container_definition["name"] == "run"
assert container_definition["image"] == image
assert not container_definition.get("entryPoint")
assert not container_definition.get("dependsOn")
# But other stuff is inhereted from the parent task definition
assert container_definition["environment"] == environment

Expand Down

0 comments on commit 9941cbc

Please sign in to comment.