Skip to content

Commit 5f9a9a0

Browse files
committed
Fix ECS monitoring and scroll validation issues
Update ecs create-express-gateway-service, ecs update-express-gateway-service, and ecs delete-express-gateway-service commands to not output API response when run with the --monitor-resources flag. Fix scrolling bounds calculations when line wrapping is present.
1 parent e54b5ff commit 5f9a9a0

File tree

8 files changed

+221
-56
lines changed

8 files changed

+221
-56
lines changed
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"type": "enhancement",
3+
"category": "``ecs``",
4+
"description": "Update ``ecs create-express-gateway-service``, ``ecs update-express-gateway-service``, and ``ecs delete-express-gateway-service`` commands to not output API response when run with the ``--monitor-resources`` flag. Also fixes scrolling bounds calculations when line wrapping is present."
5+
}

awscli/customizations/ecs/monitorexpressgatewayservice.py

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,6 @@ class ECSExpressGatewayServiceWatcher:
182182
service_arn (str): ARN of the service to monitor
183183
mode (str): Monitoring mode - 'RESOURCE' or 'DEPLOYMENT'
184184
timeout_minutes (int): Maximum monitoring time in minutes (default: 30)
185-
exit_hook (callable): Optional callback function on exit
186185
"""
187186

188187
def __init__(
@@ -191,24 +190,24 @@ def __init__(
191190
service_arn,
192191
mode,
193192
timeout_minutes=30,
194-
exit_hook=None,
195193
display=None,
196194
use_color=True,
197195
):
198196
self._client = client
199197
self.service_arn = service_arn
200198
self.mode = mode
201199
self.timeout_minutes = timeout_minutes
202-
self.exit_hook = exit_hook or self._default_exit_hook
203200
self.last_described_gateway_service_response = None
204201
self.last_execution_time = 0
205202
self.cached_monitor_result = None
206203
self.start_time = time.time()
207204
self.use_color = use_color
208205
self.display = display or Display()
209206

210-
def _default_exit_hook(self, x):
211-
return x
207+
@staticmethod
208+
def is_monitoring_available():
209+
"""Check if monitoring is available (requires TTY)."""
210+
return sys.stdout.isatty()
212211

213212
def exec(self):
214213
"""Start monitoring the express gateway service with progress display."""
@@ -226,7 +225,13 @@ async def _execute_with_progress_async(
226225
"""Execute monitoring loop with animated progress display."""
227226
spinner_chars = "⠋⠙⠹⠸⠼⠴⠦⠧⠇⠏"
228227
spinner_index = 0
229-
current_output = "Waiting for initial data"
228+
229+
# Initialize with basic service resource
230+
service_resource = ManagedResource("Service", self.service_arn)
231+
initial_output = service_resource.get_status_string(
232+
spinner_char="{SPINNER}", use_color=self.use_color
233+
)
234+
current_output = initial_output
230235

231236
async def update_data():
232237
nonlocal current_output

awscli/customizations/ecs/monitormutatinggatewayservice.py

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,14 @@ def after_call(self, parsed, context, http_response, **kwargs):
171171
).get('serviceArn'):
172172
return
173173

174+
# Check monitoring availability
175+
if not self._watcher_class.is_monitoring_available():
176+
uni_print(
177+
"Monitoring is not available (requires TTY). Skipping monitoring.\n",
178+
out_file=sys.stderr,
179+
)
180+
return
181+
174182
if not self.session or not self.parsed_globals:
175183
uni_print(
176184
"Unable to create ECS client. Skipping monitoring.",
@@ -188,22 +196,15 @@ def after_call(self, parsed, context, http_response, **kwargs):
188196
# Get service ARN from response
189197
service_arn = parsed.get('service', {}).get('serviceArn')
190198

191-
# Define exit hook to replace parsed response
192-
def exit_hook(new_response):
193-
if new_response:
194-
parsed.clear()
195-
parsed.update(new_response)
199+
# Clear output when monitoring is invoked
200+
parsed.clear()
196201

197202
try:
198-
# Determine if color should be used
199-
use_color = self._should_use_color(self.parsed_globals)
200-
201203
self._watcher_class(
202204
ecs_client,
203205
service_arn,
204206
self.effective_resource_view,
205-
exit_hook=exit_hook,
206-
use_color=use_color,
207+
use_color=self._should_use_color(self.parsed_globals),
207208
).exec()
208209
except Exception as e:
209210
uni_print(

awscli/customizations/ecs/prompt_toolkit_display.py

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import asyncio
2+
import re
23

34
from prompt_toolkit.application import Application
45
from prompt_toolkit.formatted_text import ANSI
@@ -8,6 +9,12 @@
89
from prompt_toolkit.widgets import Frame
910

1011

12+
def _get_visual_line_length(line):
13+
"""Get the visual length of a line, excluding ANSI escape codes."""
14+
ansi_escape = re.compile(r'\x1B(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])')
15+
return len(ansi_escape.sub('', line))
16+
17+
1118
class Display:
1219
def __init__(self):
1320
self.control = FormattedTextControl(text="")
@@ -21,6 +28,7 @@ def __init__(self):
2128
text="up/down to scroll, q to quit"
2229
)
2330
self.content_lines = 0
31+
self.raw_text = ""
2432
kb = KeyBindings()
2533

2634
@kb.add('q')
@@ -35,12 +43,24 @@ def scroll_up(event):
3543

3644
@kb.add('down')
3745
def scroll_down(event):
38-
window_height = (
39-
event.app.output.get_size().rows - 3
40-
) # Frame top, frame bottom, status bar
41-
if self.window.vertical_scroll < max(
42-
0, self.content_lines - window_height
43-
):
46+
# Frame top, frame bottom, status bar
47+
window_height = event.app.output.get_size().rows - 3
48+
49+
# Account for frame borders and scrollbar
50+
window_width = event.app.output.get_size().columns - 4
51+
total_display_lines = 0
52+
53+
for line in self.raw_text.split('\n'):
54+
visual_length = _get_visual_line_length(line)
55+
if visual_length == 0:
56+
total_display_lines += 1
57+
else:
58+
total_display_lines += max(
59+
1, (visual_length + window_width - 1) // window_width
60+
)
61+
62+
max_scroll = max(0, total_display_lines - window_height)
63+
if self.window.vertical_scroll < max_scroll:
4464
self.window.vertical_scroll += 1
4565

4666
self.app = Application(
@@ -58,12 +78,42 @@ def scroll_down(event):
5878

5979
def display(self, text, status_text=""):
6080
"""Update display with ANSI colored text."""
81+
self.raw_text = text
6182
self.control.text = ANSI(text)
6283
self.content_lines = len(text.split('\n'))
84+
85+
self._validate_scroll_position()
86+
6387
if status_text:
6488
self.status_control.text = status_text
6589
self.app.invalidate()
6690

91+
def _validate_scroll_position(self):
92+
"""Ensure scroll position is valid for current content."""
93+
if not hasattr(self.app, 'output') or not self.app.output:
94+
return
95+
96+
try:
97+
window_height = self.app.output.get_size().rows - 3
98+
window_width = self.app.output.get_size().columns - 4
99+
100+
total_display_lines = 0
101+
for line in self.raw_text.split('\n'):
102+
visual_length = _get_visual_line_length(line)
103+
if visual_length == 0:
104+
total_display_lines += 1
105+
else:
106+
total_display_lines += max(
107+
1, (visual_length + window_width - 1) // window_width
108+
)
109+
110+
max_scroll = max(0, total_display_lines - window_height)
111+
if self.window.vertical_scroll > max_scroll:
112+
self.window.vertical_scroll = max_scroll
113+
except (AttributeError, OSError):
114+
# If we can't determine terminal size, leave scroll position unchanged
115+
pass
116+
67117
async def run(self):
68118
"""Run the display app."""
69119
await self.app.run_async()

tests/functional/ecs/test_monitormutatinggatewayservice.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,12 @@ def test_add_to_parser(self):
3232

3333
class TestMonitorMutatingGatewayService:
3434
def setup_method(self):
35+
self.mock_watcher_class = Mock()
36+
self.mock_watcher_class.is_monitoring_available.return_value = True
3537
self.handler = MonitorMutatingGatewayService(
36-
'create-gateway-service', 'DEPLOYMENT'
38+
'create-gateway-service',
39+
'DEPLOYMENT',
40+
watcher_class=self.mock_watcher_class,
3741
)
3842

3943
def test_init(self):

tests/unit/customizations/ecs/test_monitorexpressgatewayservice.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,22 @@ def test_interactive_mode_requires_tty(self, mock_isatty, capsys):
105105
class TestECSExpressGatewayServiceWatcher:
106106
"""Test the watcher class through public interface"""
107107

108+
@patch('sys.stdout.isatty')
109+
def test_is_monitoring_available_with_tty(self, mock_isatty):
110+
"""Test is_monitoring_available returns True when TTY is available"""
111+
mock_isatty.return_value = True
112+
assert (
113+
ECSExpressGatewayServiceWatcher.is_monitoring_available() is True
114+
)
115+
116+
@patch('sys.stdout.isatty')
117+
def test_is_monitoring_available_without_tty(self, mock_isatty):
118+
"""Test is_monitoring_available returns False when TTY is not available"""
119+
mock_isatty.return_value = False
120+
assert (
121+
ECSExpressGatewayServiceWatcher.is_monitoring_available() is False
122+
)
123+
108124
def setup_method(self):
109125
self.mock_client = Mock()
110126
self.service_arn = (

tests/unit/customizations/ecs/test_monitormutatinggatewayservice.py

Lines changed: 48 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,12 @@ class TestMonitorMutatingGatewayService:
7272
"""Test the event handler for monitoring gateway service mutations."""
7373

7474
def setup_method(self):
75+
self.mock_watcher_class = Mock()
76+
self.mock_watcher_class.is_monitoring_available.return_value = True
7577
self.handler = MonitorMutatingGatewayService(
76-
'create-gateway-service', 'DEPLOYMENT'
78+
'create-gateway-service',
79+
'DEPLOYMENT',
80+
watcher_class=self.mock_watcher_class,
7781
)
7882

7983
def test_init(self):
@@ -242,6 +246,7 @@ def test_after_call_successful_monitoring(self):
242246
'https://ecs.us-west-2.amazonaws.com'
243247
)
244248
mock_parsed_globals.verify_ssl = True
249+
mock_parsed_globals.color = 'off'
245250
handler.session = mock_session
246251
handler.parsed_globals = mock_parsed_globals
247252

@@ -272,18 +277,17 @@ def test_after_call_successful_monitoring(self):
272277
mock_client,
273278
service_arn,
274279
'DEPLOYMENT',
275-
exit_hook=ANY,
276280
use_color=False,
277281
)
278282
mock_watcher.exec.assert_called_once()
283+
# Verify parsed response was cleared
284+
assert parsed == {}
279285

280-
def test_after_call_exception_handling(self, capsys):
281-
"""Test exception handling in after_call method."""
286+
def test_after_call_monitoring_not_available(self, capsys):
287+
"""Test that monitoring is skipped when not available (no TTY)."""
282288
# Setup handler state
283289
mock_watcher_class = Mock()
284-
mock_watcher = Mock()
285-
mock_watcher.exec.side_effect = Exception("Test exception")
286-
mock_watcher_class.return_value = mock_watcher
290+
mock_watcher_class.is_monitoring_available.return_value = False
287291

288292
handler = MonitorMutatingGatewayService(
289293
'create-gateway-service',
@@ -299,6 +303,7 @@ def test_after_call_exception_handling(self, capsys):
299303
'https://ecs.us-west-2.amazonaws.com'
300304
)
301305
mock_parsed_globals.verify_ssl = True
306+
mock_parsed_globals.color = 'off'
302307
handler.session = mock_session
303308
handler.parsed_globals = mock_parsed_globals
304309

@@ -309,57 +314,67 @@ def test_after_call_exception_handling(self, capsys):
309314
# Setup call parameters
310315
service_arn = 'arn:aws:ecs:us-west-2:123456789012:service/test-service'
311316
parsed = {'service': {'serviceArn': service_arn}}
317+
original_parsed = dict(parsed)
312318
context = Mock()
313319
http_response = Mock()
314320
http_response.status_code = 200
315321

316-
# Execute - should not raise exception
322+
# Execute
317323
handler.after_call(parsed, context, http_response)
318324

325+
# Verify parsed response was not cleared
326+
assert parsed == original_parsed
327+
328+
# Verify warning message was printed
319329
captured = capsys.readouterr()
320-
assert "Encountered an error, terminating monitoring" in captured.err
321-
assert "Test exception" in captured.err
330+
assert (
331+
"Monitoring is not available (requires TTY). Skipping monitoring.\n"
332+
in captured.err
333+
)
322334

323-
def test_exit_hook_functionality(self):
324-
"""Test that exit hook properly updates parsed response."""
335+
def test_after_call_exception_handling(self, capsys):
336+
"""Test exception handling in after_call method."""
325337
# Setup handler state
326-
self.handler.effective_resource_view = 'DEPLOYMENT'
338+
mock_watcher_class = Mock()
339+
mock_watcher = Mock()
340+
mock_watcher.exec.side_effect = Exception("Test exception")
341+
mock_watcher_class.return_value = mock_watcher
342+
343+
handler = MonitorMutatingGatewayService(
344+
'create-gateway-service',
345+
'DEPLOYMENT',
346+
watcher_class=mock_watcher_class,
347+
)
348+
handler.effective_resource_view = 'DEPLOYMENT'
349+
327350
mock_session = Mock()
328351
mock_parsed_globals = Mock()
329352
mock_parsed_globals.region = 'us-west-2'
330353
mock_parsed_globals.endpoint_url = (
331354
'https://ecs.us-west-2.amazonaws.com'
332355
)
333356
mock_parsed_globals.verify_ssl = True
334-
self.handler.session = mock_session
335-
self.handler.parsed_globals = mock_parsed_globals
357+
mock_parsed_globals.color = 'off'
358+
handler.session = mock_session
359+
handler.parsed_globals = mock_parsed_globals
336360

337361
# Setup mocks
338362
mock_client = Mock()
339363
mock_session.create_client.return_value = mock_client
340364

341-
# Test exit hook functionality by directly calling it
365+
# Setup call parameters
342366
service_arn = 'arn:aws:ecs:us-west-2:123456789012:service/test-service'
343367
parsed = {'service': {'serviceArn': service_arn}}
368+
context = Mock()
369+
http_response = Mock()
370+
http_response.status_code = 200
344371

345-
# Create a simple exit hook function
346-
def test_exit_hook(new_response):
347-
if new_response:
348-
parsed.clear()
349-
parsed.update(new_response)
350-
351-
# Test with new response
352-
new_response = {
353-
'service': {'serviceArn': service_arn, 'status': 'ACTIVE'}
354-
}
355-
test_exit_hook(new_response)
356-
357-
assert parsed == new_response
372+
# Execute - should not raise exception
373+
handler.after_call(parsed, context, http_response)
358374

359-
# Test with None response (should not update)
360-
original_parsed = dict(parsed)
361-
test_exit_hook(None)
362-
assert parsed == original_parsed
375+
captured = capsys.readouterr()
376+
assert "Encountered an error, terminating monitoring" in captured.err
377+
assert "Test exception" in captured.err
363378

364379
def test_events(self):
365380
"""Test that correct events are returned for CLI integration."""

0 commit comments

Comments
 (0)