Skip to content

Commit 4e4fd9d

Browse files
committed
fix: wire orb config show missing sections, fix fleetRole ARN expansion, add scheduler mismatch flag
orb config show: - Add get_scripts_dir() to ConfigurationPort/ConfigurationAdapter - scripts_dir, template_search_paths, logging, request_limits, backup_path, data_path all wired - grace_period and circuit_breaker defaults wired - Fix empty except clauses with debug logging - Fix mismatch_action initialisation before try block fleetRole: - Short name expanded to full ARN from template or config Scheduler mismatch: - on_scheduler_mismatch: ignore|warn|fail flag added to SchedulerConfig - _delegate_load_to_strategy enforces the flag - _validate_basic_template_structure checks scheduler compatibility
1 parent e65a45a commit 4e4fd9d

File tree

9 files changed

+141
-37
lines changed

9 files changed

+141
-37
lines changed

src/orb/application/dto/system.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ class PathsSectionDTO(BaseDTO):
229229
config_dir: str
230230
work_dir: str
231231
log_dir: str
232+
scripts_dir: Optional[str] = None
232233
loaded_config_file: Optional[str] = None
233234
loaded_templates_file: Optional[str] = None
234235
template_search_paths: Optional[list[str]] = Field(default=None)

src/orb/application/queries/system_handlers.py

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -489,8 +489,7 @@ async def execute_query(self, query: GetSystemConfigQuery) -> SystemConfigDTO:
489489
loaded_templates_file = path
490490
break
491491

492-
if query.verbose:
493-
template_search_paths = all_paths
492+
template_search_paths = all_paths
494493
except Exception: # noqa: BLE001 — template path resolution is best-effort; fall back to None
495494
pass
496495

@@ -499,6 +498,7 @@ async def execute_query(self, query: GetSystemConfigQuery) -> SystemConfigDTO:
499498
config_dir=cfg.get_config_dir() if hasattr(cfg, "get_config_dir") else "",
500499
work_dir=cfg.get_work_dir() if hasattr(cfg, "get_work_dir") else "",
501500
log_dir=cfg.get_log_dir() if hasattr(cfg, "get_log_dir") else "",
501+
scripts_dir=cfg.get_scripts_dir() if hasattr(cfg, "get_scripts_dir") else None,
502502
loaded_config_file=cast(Any, cfg).get_loaded_config_file()
503503
if hasattr(cfg, "get_loaded_config_file")
504504
else None,
@@ -554,35 +554,43 @@ async def execute_query(self, query: GetSystemConfigQuery) -> SystemConfigDTO:
554554
port=server_cfg.get("port"),
555555
)
556556

557-
# --- verbose sections ---
557+
# --- always-shown sections ---
558+
log_cfg = cfg.get_logging_config() if hasattr(cfg, "get_logging_config") else {}
558559
logging_section: LoggingSectionDTO | None = None
560+
if isinstance(log_cfg, dict):
561+
logging_section = LoggingSectionDTO(
562+
level=str(log_cfg.get("level", "INFO")),
563+
log_file=log_cfg.get("file_path"),
564+
console_enabled=bool(log_cfg.get("console_enabled", True)),
565+
)
566+
567+
req_cfg = cfg.get_request_config() if hasattr(cfg, "get_request_config") else {}
559568
request_limits: RequestLimitsSectionDTO | None = None
560-
circuit_breaker: CircuitBreakerSectionDTO | None = None
569+
if isinstance(req_cfg, dict):
570+
request_limits = RequestLimitsSectionDTO(
571+
max_machines=req_cfg.get("max_machines_per_request"),
572+
default_timeout=req_cfg.get("default_timeout"),
573+
grace_period=req_cfg.get("default_grace_period"),
574+
)
561575

576+
# --- verbose-only sections ---
577+
circuit_breaker: CircuitBreakerSectionDTO | None = None
562578
if query.verbose:
563-
log_cfg = cfg.get_logging_config() if hasattr(cfg, "get_logging_config") else {}
564-
if isinstance(log_cfg, dict):
565-
logging_section = LoggingSectionDTO(
566-
level=str(log_cfg.get("level", "INFO")),
567-
log_file=log_cfg.get("file"),
568-
console_enabled=bool(log_cfg.get("console_enabled", True)),
569-
)
570-
571-
req_cfg = cfg.get_request_config() if hasattr(cfg, "get_request_config") else {}
572-
if isinstance(req_cfg, dict):
573-
request_limits = RequestLimitsSectionDTO(
574-
max_machines=req_cfg.get("max_machines_per_request"),
575-
default_timeout=req_cfg.get("default_timeout"),
576-
grace_period=req_cfg.get("grace_period"),
577-
)
578-
579-
cb_cfg = cfg.get("circuit_breaker", {})
580-
if isinstance(cb_cfg, dict):
579+
try:
580+
cb = cfg.app_config.circuit_breaker # type: ignore[union-attr]
581581
circuit_breaker = CircuitBreakerSectionDTO(
582-
enabled=bool(cb_cfg.get("enabled", False)),
583-
failure_threshold=cb_cfg.get("failure_threshold"),
584-
recovery_timeout=cb_cfg.get("recovery_timeout"),
582+
enabled=cb.enabled,
583+
failure_threshold=cb.failure_threshold,
584+
recovery_timeout=cb.recovery_timeout,
585585
)
586+
except Exception: # noqa: BLE001
587+
cb_cfg = cfg.get("circuit_breaker", {})
588+
if isinstance(cb_cfg, dict):
589+
circuit_breaker = CircuitBreakerSectionDTO(
590+
enabled=bool(cb_cfg.get("enabled", True)),
591+
failure_threshold=cb_cfg.get("failure_threshold", 5),
592+
recovery_timeout=cb_cfg.get("recovery_timeout", 60),
593+
)
586594

587595
return SystemConfigDTO(
588596
paths=paths,

src/orb/config/loader.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,9 @@ def _process_scheduler_directories(
457457

458458
# Propagate scripts_dir written by `orb init` back into the config model
459459
if "scripts_dir" in config and config["scripts_dir"]:
460-
get_config_logger().debug("scripts_dir from config: %s", config["scripts_dir"])
460+
scripts_dir = config["scripts_dir"]
461+
get_config_logger().debug("scripts_dir from config: %s", scripts_dir)
462+
os.environ.setdefault("ORB_SCRIPTS_DIR", scripts_dir)
461463

462464
# Set up storage paths
463465
if scheduler_dir:

src/orb/config/schemas/scheduler_schema.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@ class SchedulerConfig(BaseModel):
2424
templates_filename: Optional[str] = Field(
2525
None, description="Override templates filename (null = use scheduler default)"
2626
)
27+
on_scheduler_mismatch: Literal["ignore", "warn", "fail"] = Field(
28+
"warn",
29+
description="Behaviour when a template's scheduler_type doesn't match the active scheduler",
30+
)
2731

2832
def get_config_root(self) -> str:
2933
"""Get config root with automatic environment variable expansion."""

src/orb/domain/base/ports/configuration_port.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,10 @@ def get_root_dir(self) -> str:
176176
"""Get the root directory path."""
177177
return ""
178178

179+
def get_scripts_dir(self) -> str:
180+
"""Get the scripts directory path."""
181+
return ""
182+
179183
@property
180184
def app_config(self) -> Any:
181185
"""Get the full application configuration."""

src/orb/infrastructure/adapters/configuration_adapter.py

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ def get_request_config(self) -> dict[str, Any]:
9191
"max_machines_per_request": getattr(
9292
request_config, "max_machines_per_request", 100
9393
),
94-
"default_timeout": getattr(request_config, "default_timeout", 300),
94+
"default_timeout": getattr(request_config, "default_timeout", 3600),
95+
"default_grace_period": getattr(request_config, "default_grace_period", 300),
9596
"min_timeout": getattr(request_config, "min_timeout", 30),
9697
"max_timeout": getattr(request_config, "max_timeout", 3600),
9798
"fulfillment_max_retries": request_config.fulfillment_max_retries,
@@ -103,7 +104,8 @@ def get_request_config(self) -> dict[str, Any]:
103104
self._logger.warning("Failed to load request config, using defaults: %s", e)
104105
return {
105106
"max_machines_per_request": 100,
106-
"default_timeout": 300,
107+
"default_timeout": 3600,
108+
"default_grace_period": 300,
107109
"min_timeout": 30,
108110
"max_timeout": 3600,
109111
"fulfillment_max_retries": 3,
@@ -177,23 +179,52 @@ def get_root_dir(self) -> str:
177179

178180
return str(get_root_location())
179181

182+
def get_scripts_dir(self) -> str:
183+
"""Get the scripts directory path."""
184+
from orb.config.platform_dirs import get_scripts_location
185+
186+
return str(get_scripts_location())
187+
180188
def get_storage_config(self) -> dict[str, Any]:
181189
"""Get storage configuration."""
182190
try:
191+
from pathlib import Path
192+
183193
storage_config = self._config_manager.get("storage", {})
184194
strategy = storage_config.get("strategy", storage_config.get("type", "json"))
185195
if strategy == "json":
186196
json_cfg = storage_config.get("json_strategy", {})
187-
data_path = json_cfg.get("base_path")
197+
raw_data_path = json_cfg.get("base_path")
188198
backup_enabled = json_cfg.get("backup_enabled", True)
199+
raw_backup_path = json_cfg.get("backup_path") or storage_config.get("backup_path")
189200
elif strategy == "sql":
190201
sql_cfg = storage_config.get("sql_strategy", {})
191-
data_path = sql_cfg.get("name")
202+
raw_data_path = sql_cfg.get("name")
192203
backup_enabled = False
204+
raw_backup_path = storage_config.get("backup_path")
193205
else:
194-
data_path = None
206+
raw_data_path = None
195207
backup_enabled = storage_config.get("backup_enabled", True)
196-
backup_path = storage_config.get("backup_path")
208+
raw_backup_path = storage_config.get("backup_path")
209+
210+
work_dir = Path(self._config_manager.get_work_dir())
211+
212+
# Resolve data_path to absolute
213+
if raw_data_path:
214+
p = Path(raw_data_path)
215+
data_path = str(p if p.is_absolute() else work_dir / p)
216+
else:
217+
data_path = None
218+
219+
# Resolve backup_path; default to work_dir/backups
220+
if raw_backup_path:
221+
bp = Path(raw_backup_path)
222+
backup_path = str(bp if bp.is_absolute() else work_dir / bp)
223+
elif backup_enabled:
224+
backup_path = str(work_dir / "backups")
225+
else:
226+
backup_path = None
227+
197228
return {
198229
"strategy": strategy,
199230
"data_path": data_path,
@@ -231,6 +262,7 @@ def get_logging_config(self) -> dict[str, Any]:
231262
"format": logging_config.get(
232263
"format", "%(asctime)s - %(name)s - %(levelname)s - %(message)s"
233264
),
265+
"file_path": logging_config.get("file_path"),
234266
"file_enabled": logging_config.get("file_enabled", True),
235267
"console_enabled": logging_config.get("console_enabled", True),
236268
}
@@ -239,6 +271,7 @@ def get_logging_config(self) -> dict[str, Any]:
239271
return {
240272
"level": "INFO",
241273
"format": "%(asctime)s - %(name)s - %(levelname)s - %(message)s",
274+
"file_path": None,
242275
"file_enabled": True,
243276
"console_enabled": True,
244277
}

src/orb/infrastructure/scheduler/base/strategy.py

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -372,10 +372,32 @@ def _delegate_load_to_strategy(
372372

373373
registry = get_scheduler_registry()
374374
if not registry.is_registered(scheduler_type):
375-
self.logger.warning(
376-
"Scheduler type '%s' not registered, cannot delegate template loading",
377-
scheduler_type,
378-
)
375+
mismatch_action = "warn"
376+
if self._config_manager is not None:
377+
try:
378+
mismatch_action = getattr(
379+
self._config_manager.app_config.scheduler,
380+
"on_scheduler_mismatch",
381+
"warn",
382+
)
383+
except Exception as e:
384+
# Config unavailable — default to warn so mismatch doesn't hard-fail
385+
self.logger.debug("Could not read on_scheduler_mismatch config: %s", e)
386+
387+
if mismatch_action == "fail":
388+
from orb.infrastructure.template.configuration_manager import (
389+
TemplateConfigurationError,
390+
)
391+
392+
raise TemplateConfigurationError(
393+
f"Scheduler type '{scheduler_type}' not registered and on_scheduler_mismatch=fail"
394+
)
395+
elif mismatch_action == "warn":
396+
self.logger.warning(
397+
"Scheduler type '%s' not registered, cannot delegate template loading",
398+
scheduler_type,
399+
)
400+
# "ignore" — suppress the warning entirely
379401
return None
380402

381403
try:

src/orb/infrastructure/template/configuration_manager.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,33 @@ def _validate_basic_template_structure(
581581
"Max instances is very high (>1000), consider if this is intentional"
582582
)
583583

584+
# Check scheduler_type metadata mismatch
585+
template_scheduler_type = (
586+
template.metadata.get("scheduler_type") if isinstance(template.metadata, dict) else None
587+
)
588+
if template_scheduler_type is not None:
589+
active_scheduler_type = None
590+
mismatch_action = "warn"
591+
try:
592+
active_scheduler_type = self.config_manager.app_config.scheduler.type
593+
mismatch_action = getattr(
594+
self.config_manager.app_config.scheduler, "on_scheduler_mismatch", "warn"
595+
)
596+
except Exception as e:
597+
self.logger.debug("Could not read scheduler config for mismatch check: %s", e)
598+
599+
if active_scheduler_type and template_scheduler_type != active_scheduler_type:
600+
msg = (
601+
f"Template '{template.template_id}' has scheduler_type "
602+
f"'{template_scheduler_type}' but active scheduler is '{active_scheduler_type}'"
603+
)
604+
if mismatch_action == "fail":
605+
result["is_valid"] = False
606+
result["errors"].append(msg)
607+
elif mismatch_action == "warn":
608+
result["warnings"].append(msg)
609+
# "ignore" — add nothing
610+
584611
self.logger.debug("Basic validation completed for template %s", template.template_id)
585612

586613
async def _validate_with_provider_registry(

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,10 @@ def _resolve_fleet_role(self, aws_template: AWSTemplate) -> AWSTemplate:
225225
f"spotfleet.amazonaws.com/AWSServiceRoleForEC2SpotFleet"
226226
)
227227
self._logger.info("Converted EC2Fleet role to SpotFleet role: %s", resolved)
228-
elif fleet_role == "AWSServiceRoleForEC2SpotFleet":
228+
elif fleet_role in (
229+
"AWSServiceRoleForEC2SpotFleet",
230+
"AmazonEC2SpotFleetTaggingRole",
231+
):
229232
account_id = self.aws_client.sts_client.get_caller_identity()["Account"]
230233
resolved = (
231234
f"arn:aws:iam::{account_id}:role/aws-service-role/"

0 commit comments

Comments
 (0)