Skip to content

Commit 7c4ec26

Browse files
authored
Merge pull request #157 from awslabs/fix/p1-bugfixes-wave1
fix: resolve except-pass antipatterns, DI duplicate registration, and pytest-asyncio config
2 parents 8fd83db + 1fb445e commit 7c4ec26

6 files changed

Lines changed: 27 additions & 26 deletions

File tree

pytest.ini

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ norecursedirs = tests/onaws
44
python_files = test_*.py
55
python_classes = Test*
66
python_functions = test_*
7+
asyncio_mode = strict
78
addopts = --tb=short --strict-markers --disable-warnings
89
markers =
910
unit: Unit tests

src/orb/infrastructure/adapters/asg_query_adapter.py

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
"""Infrastructure adapter implementing ASGQueryPort via AWSClient."""
22

3+
import logging
34
from typing import Any
45

6+
from botocore.exceptions import ClientError
7+
58
from orb.domain.base.ports.asg_query_port import ASGQueryPort
69

710

@@ -10,6 +13,7 @@ class ASGQueryAdapter(ASGQueryPort):
1013

1114
def __init__(self, aws_client: Any) -> None:
1215
self._aws_client = aws_client
16+
self._logger = logging.getLogger(__name__)
1317

1418
async def get_asg_details(self, asg_name: str) -> dict[str, Any]:
1519
"""Return current details for the named ASG, or an empty dict if not found."""
@@ -19,5 +23,11 @@ async def get_asg_details(self, asg_name: str) -> dict[str, Any]:
1923
)
2024
groups = response.get("AutoScalingGroups", [])
2125
return groups[0] if groups else {}
22-
except Exception:
23-
return {}
26+
except ClientError as e:
27+
if e.response["Error"]["Code"] in ("ValidationError",):
28+
return {}
29+
self._logger.warning("AWS API error querying ASG %s: %s", asg_name, e)
30+
raise
31+
except Exception as e:
32+
self._logger.error("Unexpected error querying ASG %s: %s", asg_name, e)
33+
raise

src/orb/infrastructure/di/port_registrations.py

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@
55
ConfigurationPort,
66
ContainerPort,
77
ErrorHandlingPort,
8-
EventPublisherPort,
98
ProviderConfigPort,
109
ProviderSelectionPort,
11-
SchedulerPort,
1210
TemplateConfigurationPort,
1311
)
1412
from orb.domain.base.ports.logging_port import LoggingPort
@@ -61,23 +59,6 @@ def create_unit_of_work_factory(c):
6159
container.register_singleton(ErrorHandlingAdapter, lambda c: ErrorHandlingAdapter())
6260
container.register_singleton(ErrorHandlingPort, lambda c: c.get(ErrorHandlingAdapter))
6361

64-
# Register template configuration manager with manual factory (handles
65-
# optional dependencies)
66-
def create_template_configuration_manager(c):
67-
"""Create template configuration manager with dependencies."""
68-
return TemplateConfigurationManager(
69-
config_manager=c.get(
70-
ConfigurationManager
71-
), # Use ConfigurationManager directly to break circular dependency
72-
scheduler_strategy=c.get_optional(SchedulerPort),
73-
logger=c.get(LoggingPort),
74-
event_publisher=c.get_optional(EventPublisherPort),
75-
)
76-
77-
container.register_singleton(
78-
TemplateConfigurationManager, create_template_configuration_manager
79-
)
80-
8162
# Register template configuration port adapter
8263
from orb.infrastructure.adapters.template_configuration_adapter import (
8364
TemplateConfigurationAdapter,

src/orb/infrastructure/storage/json/strategy.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -355,8 +355,12 @@ def _save_data(self, entity_data: dict[str, dict[str, Any]]) -> None:
355355
full_data = self.serializer.deserialize(content)
356356
else:
357357
full_data = {}
358-
except Exception:
359-
full_data = {}
358+
except Exception as e:
359+
self.logger.error(
360+
"Cannot read existing storage file before save, aborting to prevent data loss: %s",
361+
e,
362+
)
363+
raise
360364

361365
# Ensure full_data is a dictionary
362366
if not isinstance(full_data, dict):

src/orb/providers/aws/infrastructure/aws_handler_factory.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,8 +107,10 @@ def create_handler(self, handler_type: str) -> AWSHandler:
107107
native_spec_service=container.get(NativeSpecService),
108108
config_port=config_port,
109109
)
110-
except Exception:
111-
pass
110+
except Exception as e:
111+
self._logger.warning(
112+
"AWSNativeSpecService unavailable, native spec enrichment disabled: %s", e
113+
)
112114

113115
machine_adapter = AWSMachineAdapter(
114116
aws_client=self._aws_client,

src/orb/providers/aws/infrastructure/handlers/asg/handler.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,10 @@ def detect_asg_instances(aws_client, instance_ids: list[str]) -> dict[str, list[
304304

305305
return asg_mapping
306306

307-
except Exception:
307+
except Exception as e:
308+
logging.warning(
309+
"Failed to detect ASG membership for instances, skipping capacity reduction: %s", e
310+
)
308311
return {}
309312

310313
def reduce_capacity_for_instance_ids(self, instance_ids: list[str]) -> None:

0 commit comments

Comments
 (0)