Introduce text only mode #9927
Introduce text only mode #9927
Conversation
jutyler1
force-pushed
the
ecs-pr-v5-04
branch
from
a94f5f7 to
532d26c
Compare
aemous
left a comment
aemous
left a comment
There was a problem hiding this comment.
Does this handle miscellaneous errors that get encountered? For example, let's say a service exists in region A, but I configure a different service B instead. Previously, the error was being swallowed. Is surfacing misc errors resolved on this branch?
Separately, I'd like to see at least 1-3 new functional tests, that test this new mode end-to-end by testing the entire command with arguments. At least one of the tests should verify functionality with --color flag.
tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py
Outdated
Show resolved
Hide resolved
tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py
Outdated
Show resolved
Hide resolved
tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py
Outdated
Show resolved
Hide resolved
tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py
Outdated
Show resolved
Hide resolved
tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py
Outdated
Show resolved
Hide resolved
That ended up getting fixed in the previous commit where we re-raise exceptions that occurred async |
added |
jutyler1
force-pushed
the
ecs-pr-v5-04
branch
from
7b5130f to
cce5450
Compare
aemous
left a comment
aemous
left a comment
There was a problem hiding this comment.
Looks good overall, requested some minor changes
jutyler1
force-pushed
the
ecs-pr-v5-04
branch
from
dc0ead0 to
7399df1
Compare
aemous
left a comment
aemous
left a comment
There was a problem hiding this comment.
Looks good, one small change requested.
aemous
left a comment
aemous
left a comment
There was a problem hiding this comment.
After further review, looks like there is at least 1 failing Windows test in GitHub CI. Can you address this in the next revision?
fixed |
aemous
merged commit 7176771
into
aws:ecs-express-gateway-text-only
2 participants
Issue #, if available:
Description of changes:
Introduce text only mode
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.