Infer cluster from tags
ClosedPublic
Actions

Authored by • jordansanders on Jul 20 2021, 9:50 PM.

Details

Reviewers

• dgibson
• johann
• max

Commits

R1:00d73bb34601: Infer cluster from tags

Summary

In addition to the credentials error exposed in
https://dagster.phacility.com/D8977, I noticed that we don't actually
need to query self._task_metadata to get the cluster name when
terminating. This is because we tag ECS launched pipeline runs with both
the ARN and the cluster. If it's lacking either, we can't terminate it
regardless of what self._task_metadata says.

Test Plan

unit

Diff Detail

Repository

R1 dagster

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• jordansanders created this revision.Jul 20 2021, 9:50 PM

• jordansanders added a parent revision: D8977: Give dagit permission to interact with ECS.Jul 20 2021, 9:50 PM

Harbormaster completed remote builds in B33949: Diff 41936.Jul 20 2021, 10:17 PM

• jordansanders requested review of this revision.Jul 20 2021, 10:17 PM

• dgibson mentioned this in D8977: Give dagit permission to interact with ECS.Jul 21 2021, 7:44 PM

• dgibson added inline comments.

python_modules/libraries/dagster-aws/dagster_aws/ecs/launcher.py
140	should we check that ["tasks"] is non-empty too to avoid an index error

• dgibson accepted this revision.Jul 21 2021, 7:56 PM

This revision is now accepted and ready to land.Jul 21 2021, 7:56 PM

• jordansanders mentioned this in D8995: More narrowly scope IAM policies.Jul 21 2021, 9:23 PM

• jordansanders added inline comments.Jul 21 2021, 9:25 PM

python_modules/libraries/dagster-aws/dagster_aws/ecs/launcher.py
140	I haven't been able to reproduce a case where the arn and cluster tags both exist, the request succeeds, but the tasks key is empty. I'm not entirely sure what we'd do in that case - we could retry (but for how long?) or we could return False (but that might not actually be accurate). I think for now, I'd prefer to keep it as is and if we start to see this bubble up, we can reconsider.