Skip to content

Drop the unused telemetry.mode config - #44

Merged
AnuradhaKaruppiah merged 3 commits into
NVIDIA:mainfrom
dagardner-nv:david-drop-cfg-keys
Jul 9, 2026
Merged

Drop the unused telemetry.mode config #44
AnuradhaKaruppiah merged 3 commits into
NVIDIA:mainfrom
dagardner-nv:david-drop-cfg-keys

Conversation

@dagardner-nv

@dagardner-nv dagardner-nv commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Overview

  • This configuration setting is unused, and how the telemetry is configured is dependent upon the adapter

Where should the reviewer start?

crates/fabric-core/src/runtime.rs

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • Closes FABRIC-47

  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.

  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Summary by CodeRabbit

  • Bug Fixes
    • Simplified relay runtime output by removing an outdated mode field from generated relay details.
    • Relay status and configuration information still appear, along with relay artifacts and emitter details.
    • Improved consistency across relay-enabled workflows so reported runtime data matches current behavior.

Signed-off-by: David Gardner <dagardner@nvidia.com>
…cfg-keys

Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
@linear

linear Bot commented Jul 9, 2026

Copy link
Copy Markdown

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

The relay runtime "mode" field is removed end-to-end: the RELAY_RUNTIME_MODE constant and its config/env generation are deleted in fabric-core, the corresponding output["relay_runtime"]["mode"] entries are removed from the codex-cli, hermes-cli, and hermes-sdk adapters, and the e2e test assertion is updated to check "emitter" instead of "mode".

Changes

Relay mode removal

Layer / File(s) Summary
Core relay config and env generation
crates/fabric-core/src/runtime.rs
Removes the RELAY_RUNTIME_MODE constant and stops emitting the "mode" field in generated relay-config.json and the FABRIC_RELAY_MODE environment variable.
Adapter relay_runtime output updates
adapters/codex-cli/.../adapter.py, adapters/hermes-cli/.../adapter.py, adapters/hermes-sdk/.../adapter.py
Removes the "mode" entry from output["relay_runtime"] in all three adapters, keeping enabled, config_path, and emitter.
E2E test assertion updates
tests/e2e/test_hermes_e2e.py
Replaces the assertion checking relay_runtime["mode"] == "sdk" with an assertion checking relay_runtime["emitter"] == "hermes.observability/nemo_relay".

Estimated code review effort: 1 (Trivial) | ~5 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Title check ❌ Error The title is descriptive, but it does not follow the required Conventional Commits format. Rewrite it as Conventional Commits, for example: fix(runtime): drop unused telemetry.mode config.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description includes Overview, reviewer start, related issue, and required checkboxes, so it mostly matches the template.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

@dagardner-nv
dagardner-nv marked this pull request as ready for review July 9, 2026 16:11
@AnuradhaKaruppiah
AnuradhaKaruppiah merged commit 5ceb133 into NVIDIA:main Jul 9, 2026
6 checks passed
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