Skip to content

apollo_integration_tests,apollo_node_config: load integration-test node config via native format#14629

Open
nimrod-starkware wants to merge 1 commit into
nimrod/jsonnet/harness-expose-evaluatorfrom
nimrod/jsonnet/harness-native-config
Open

apollo_integration_tests,apollo_node_config: load integration-test node config via native format#14629
nimrod-starkware wants to merge 1 commit into
nimrod/jsonnet/harness-expose-evaluatorfrom
nimrod/jsonnet/harness-native-config

Conversation

@nimrod-starkware

Copy link
Copy Markdown
Contributor

Make the integration-test harness emit a nested native config artifact plus a
secrets file and boot the node with --config_format native (two --config_file
paths: nested base first, flat secrets second), instead of the legacy preset
single-file path. Adds as_native_value/dump_native_config_file alongside the
retained preset as_value, a {} secrets file, and node_config_paths() returning
[base, secrets]; both spawn_run_node call sites pass the two-file vector.

This is the prerequisite that lets the integration suite exercise the native
load path before the preset path is torn down.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

nimrod-starkware commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@reviewable-StarkWare

Copy link
Copy Markdown

This change is Reviewable

@nimrod-starkware nimrod-starkware left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nimrod-starkware made 1 comment.
Reviewable status: 0 of 4 files reviewed, 1 unresolved discussion.


crates/apollo_integration_tests/src/executable_setup.rs line 109 at r1 (raw file):

    /// Config files passed to the native loader, base first then secrets, matching the
    /// `[base, secret]` arity expected by `load_native`.
    pub fn node_config_paths(&self) -> Vec<PathBuf> {

Suggestion:

 pub(crate) fn node_config_paths(&self) -> [PathBuf; 2] {

@nimrod-starkware nimrod-starkware force-pushed the nimrod/jsonnet/harness-native-config branch from ad01662 to 81e18a7 Compare June 28, 2026 08:19
@nimrod-starkware nimrod-starkware force-pushed the nimrod/jsonnet/native-override-mainnet branch from 2e11c32 to aa981cd Compare June 28, 2026 10:14
@nimrod-starkware nimrod-starkware force-pushed the nimrod/jsonnet/harness-native-config branch from 81e18a7 to 4614c66 Compare June 28, 2026 10:14
@nimrod-starkware nimrod-starkware force-pushed the nimrod/jsonnet/native-override-mainnet branch from aa981cd to cfcf68f Compare June 28, 2026 10:30
@nimrod-starkware nimrod-starkware force-pushed the nimrod/jsonnet/harness-native-config branch from 4614c66 to 76ce25d Compare June 28, 2026 10:30
@nimrod-starkware nimrod-starkware force-pushed the nimrod/jsonnet/native-override-mainnet branch from cfcf68f to ab93940 Compare June 28, 2026 14:04
@nimrod-starkware nimrod-starkware force-pushed the nimrod/jsonnet/harness-native-config branch from 76ce25d to 0797ffc Compare June 28, 2026 14:04
@nimrod-starkware nimrod-starkware self-assigned this Jun 29, 2026
@nimrod-starkware nimrod-starkware marked this pull request as ready for review June 29, 2026 06:45
@cursor

cursor Bot commented Jun 29, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Test-harness and config serialization helpers only; production node loading is unchanged unless callers adopt the same pattern.

Overview
Integration tests now exercise the native config load path instead of the legacy preset single-file flow.

Config artifacts: The harness writes a nested base JSON (via new as_native_value / dump_native_config_file on DeploymentBaseAppConfig) plus a second flat secrets file ({} when tests carry no secrets). ExecutableSetup exposes node_config_paths() as [base, secrets] to match load_native's expected arity.

Node spawn: spawn_run_node passes --config_format native and two --config_file arguments (base first, secrets second), replacing one preset file. Both run_service and run use the two-path vector.

Preset dumping (as_value / dump_config_file) remains for other callers; only the integration harness path switched.

Reviewed by Cursor Bugbot for commit 3918577. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread crates/apollo_node_config/src/config_utils.rs

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 402a491. Configure here.

Comment thread crates/apollo_integration_tests/src/executable_setup.rs
Comment thread crates/apollo_integration_tests/src/executable_setup.rs
…de config via native format

Make the integration-test harness emit a nested native config artifact plus a
secrets file and boot the node with --config_format native (two --config_file
paths: nested base first, flat secrets second), instead of the legacy preset
single-file path. Adds as_native_value/dump_native_config_file alongside the
retained preset as_value, a {} secrets file, and node_config_paths() returning
[base, secrets]; both spawn_run_node call sites pass the two-file vector.

This is the prerequisite that lets the integration suite exercise the native
load path before the preset path is torn down.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants