Skip to content

Commit

Permalink
Fix parallel execution (#47)
Browse files Browse the repository at this point in the history
* update naming ecs_task_family to be async compatible

* udpate tests

* pre-commit ci fixes
  • Loading branch information
venkatBala authored Oct 27, 2022
1 parent 9e08ee6 commit a80924e
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 9 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [UNRELEASED]

### Fixed

- Fixed the issue resulting from several async executions from covalent to AWS ECS revision ID

### Changed

- Update README.md
- Removed `ecs_task_family_name` argument from constructor as the family name is dynamically generated for each job submission

## [0.18.0] - 2022-10-27

### Changed
Expand Down
2 changes: 0 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ executor = ct.executor.ECSExecutor(
s3_bucket_name="covalent-fargate-task-resources",
ecr_repo_name="covalent-fargate-task-images",
ecs_cluster_name="covalent-fargate-cluster",
ecs_task_family_name="covalent-fargate-tasks",
ecs_task_execution_role_name="ecsTaskExecutionRole",
ecs_task_role_name="CovalentFargateTaskRole",
ecs_task_subnet_id="subnet-871545e1",
Expand Down Expand Up @@ -130,7 +129,6 @@ In order for workflows to leverage this executor, users must ensure that all the
| S3 Bucket | s3_bucket_name | The name of the S3 bucket where objects are stored |
| ECR repository | ecr_repo_name | The name of the ECR repository where task images are stored |
| ECS Cluster | ecs_cluster_name | The name of the ECS cluster on which your tasks are executed |
| ECS Task Family | ecs_task_family_name | The name of the task family that specifies container information for a user, project, or experiment |
| VPC Subnet | ecs_task_subnet_id | The ID of the subnet where instances are created |
| Security group | ecs_task_security_group_id | The ID of the security group for task instances |
| Cloudwatch log group | ecs_task_log_group_name | The name of the CloudWatch log group where container logs are stored |
Expand Down
6 changes: 2 additions & 4 deletions covalent_ecs_plugin/ecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ def __init__(
s3_bucket_name: str = None,
ecs_task_security_group_id: str = None,
ecs_cluster_name: str = None,
ecs_task_family_name: str = None,
ecs_task_execution_role_name: str = None,
ecs_task_role_name: str = None,
ecs_task_subnet_id: str = None,
Expand Down Expand Up @@ -116,9 +115,7 @@ def __init__(
)

self.ecs_cluster_name = ecs_cluster_name or get_config("executors.ecs.ecs_cluster_name")
self.ecs_task_family_name = ecs_task_family_name or get_config(
"executors.ecs.ecs_task_family_name"
)
self.ecs_task_family_name = ""

self.ecs_task_role_name = ecs_task_role_name or get_config(
"executors.ecs.ecs_task_role_name"
Expand Down Expand Up @@ -186,6 +183,7 @@ async def submit_task(self, task_metadata: Dict, identity: Dict) -> Any:

# Register the task definition
self._debug_log("Registering ECS task definition...")
self.ecs_task_family_name = f"{dispatch_id}-{node_id}"
partial_func = partial(
ecs.register_task_definition,
family=self.ecs_task_family_name,
Expand Down
3 changes: 0 additions & 3 deletions tests/test_ecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ class TestECSExecutor:
MOCK_PROFILE = "my_profile"
MOCK_S3_BUCKET_NAME = "s3-bucket"
MOCK_ECS_CLUSTER_NAME = "ecs-cluster"
MOCK_ECS_TASK_FAMILY_NAME = "task-family-name"
MOCK_ECS_EXECUTION_ROLE = "task-execution-role"
MOCK_ECS_TASK_ROLE_NAME = "task-role-name"
MOCK_ECS_TASK_SUBNET_ID = "sb-1234"
Expand Down Expand Up @@ -69,7 +68,6 @@ def mock_executor_config(self, tmp_path):
"profile": self.MOCK_PROFILE,
"s3_bucket_name": self.MOCK_S3_BUCKET_NAME,
"ecs_cluster_name": self.MOCK_ECS_CLUSTER_NAME,
"ecs_task_family_name": self.MOCK_ECS_TASK_FAMILY_NAME,
"ecs_task_execution_role_name": self.MOCK_ECS_EXECUTION_ROLE,
"ecs_task_role_name": self.MOCK_ECS_TASK_ROLE_NAME,
"ecs_task_subnet_id": self.MOCK_ECS_TASK_SUBNET_ID,
Expand Down Expand Up @@ -100,7 +98,6 @@ def test_init_explicit_values(self, mocker, mock_executor_config):

assert executor.profile == self.MOCK_PROFILE
assert executor.s3_bucket_name == self.MOCK_S3_BUCKET_NAME
assert executor.ecs_task_family_name == self.MOCK_ECS_TASK_FAMILY_NAME
assert executor.execution_role == self.MOCK_ECS_EXECUTION_ROLE
assert executor.ecs_task_role_name == self.MOCK_ECS_TASK_ROLE_NAME
assert executor.ecs_task_subnet_id == self.MOCK_ECS_TASK_SUBNET_ID
Expand Down

0 comments on commit a80924e

Please sign in to comment.