Allow configuration of the broker in local environments#74
Allow configuration of the broker in local environments#74josephmckinsey merged 14 commits intomainfrom
Conversation
d0d7785 to
6bf5f74
Compare
|
I just realized that we don't pass the broker configuration to the broker when we make the run script. Will fix tomorrow. |
There was a problem hiding this comment.
Pull request overview
Adds a type-safe HELICS federate/broker configuration layer and threads it through wiring diagrams and the CLI so local builds can override broker settings (port/core type/key) while multi-container builds remain governed by the broker service.
Changes:
- Introduce
HELICSFederateConfig/SharedFederateConfig/HELICSBrokerConfigand integrate them into runner-config generation and component config emission. - Add CLI flags for local broker overrides (
--helics-port,--helics-core-type,--helics-broker-key) and reject these for multi-container builds. - Add capability gating (
capabilities.broker_config) and expand tests around broker command/config generation.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_tests/test_oedisi_tools/test_cli.py | Adds CLI tests for local HELICS broker override flags and multi-container rejection. |
| tests/unit_tests/test_oedisi_tools/test_broker_config.py | New unit tests covering broker command generation, config serialization, and capability validation. |
| tests/unit_tests/test_oedisi_tools/component2/server.py | Updates test component server to construct federate config via from_multicontainer. |
| tests/unit_tests/test_oedisi_tools/component2/component_definition.json | Declares broker-config capability for component2. |
| tests/unit_tests/test_oedisi_tools/component2/component2.py | Updates federate creation to use HELICSFederateConfig.apply_to_federate_info. |
| tests/unit_tests/test_oedisi_tools/component1/server.py | Updates test component server to construct federate config via from_multicontainer. |
| tests/unit_tests/test_oedisi_tools/component1/component_definition.json | Declares broker-config capability for component1. |
| tests/unit_tests/test_oedisi_tools/component1/component1.py | Updates federate creation to use HELICSFederateConfig.apply_to_federate_info. |
| tests/test_mock_system/test_mock_component.py | Adjusts mock component construction to pass HELICSFederateConfig. |
| tests/test_broker_integration/test_system_configuration.py | Adds an integration-style check of generated broker + component configs (currently implemented as a script). |
| tests/test_broker_integration/system.json | Wiring diagram fixture containing shared_helics_config. |
| tests/test_broker_integration/helics-run.sh | Helper script to run HELICS with the generated runner config. |
| src/oedisi/types/helics_config.py | New Pydantic models for HELICS federate/broker/shared configuration + helpers. |
| src/oedisi/types/init.py | Exposes HELICS config types from oedisi.types. |
| src/oedisi/tools/cli_tools.py | Adds local HELICS broker override CLI options and multi-container validations. |
| src/oedisi/componentframework/system_configuration.py | Adds shared/per-component HELICS config support, capability validation, and broker command construction. |
| src/oedisi/componentframework/mock_component.py | Uses base federate config to generate helics_config.json; declares broker-config capability. |
| src/oedisi/componentframework/basic_component.py | Adds capability parsing and optionally includes federate config data in generated static_inputs.json. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if self.core_type is not None: | ||
| info.core_type = self.core_type | ||
| if self.core_name is not None: | ||
| info.core_name = self.core_name | ||
| if self.core_init_string is not None: | ||
| info.core_init_string = self.core_init_string | ||
| if self.broker is not None: | ||
| if self.broker.host is not None: | ||
| info.broker = self.broker.host | ||
| if self.broker.port is not None: | ||
| info.broker_port = self.broker.port | ||
| if self.broker.key is not None: | ||
| info.broker_key = self.broker.key |
There was a problem hiding this comment.
apply_to_federate_info() sets attributes like info.core_init_string and info.broker_key, but other code in this repo configures HelicsFederateInfo via core_init / broker_port (e.g., existing test federates). If these attribute names don’t map to real HELICS FederateInfo properties, the config may silently not apply. Consider using the established attributes (core_init, etc.) or the official helics setter functions to ensure the configuration is actually applied.
There was a problem hiding this comment.
Turns out that pyhelics calls it coreInitString in the Json and the C++, but it's called in core_init in Python. I've changed it to core_init everywhere.
AadilLatif
left a comment
There was a problem hiding this comment.
@josephmckinsey MOst things lok good. I thing I think we should improve is the ability to set the logging level for both the broker and the federates
claude did this one mainly
600a67c to
56e1b19
Compare
Creates a new HELICSFederateConfig which people are intended to subclass from.
helics.helicsFederateInfoLoadFromStringwhen you specify thestatic_inputs.jsonfile path (although this does not set all the required properties most federates need like period information).The WiringDiagram now supports setting the broker information in
shared_helics_configor in components underhelics_config_overrideas aSharedFederateConfigclass which includes only the broker information and no names.The multicontainer does not support setting the broker information with an override right now, since the configurations are slightly different. We may be able to deprecate the multicontainer
BrokerConfigat a later date.