[VanyaGlazunov]

Summary

Closes #847.

ModelExpress accepts DNS hostnames for advertised worker endpoints, while NIXL's socket metadata API requires numeric IP addresses. Resolve hostnames at the shared metadata-fetch boundary before passing them to NIXL.

This complements NIXL #2236, which adds IPv6 socket support but explicitly disables DNS resolution through AI_NUMERICHOST. Hostnames resolving to IPv6 therefore require both changes.

• Resolve DNS hostnames with getaddrinfo(AF_UNSPEC, SOCK_STREAM) and select the first IPv4/IPv6 result in system resolver order.
• Preserve numeric IPv4/IPv6 addresses without DNS, normalize bracketed literals, and preserve IPv6 scope IDs.
• Report DNS failures before calling NIXL and retain the resolved endpoint in loaded-peer tracking.
• Add 13 regression cases and update the architecture/deployment documentation.

This change uses only the Python standard library. It selects one address; multi-address connection failover is not implemented.

Validation

• 10 new regression cases failed on hostname resolution, bracket normalization, and DNS failure handling; 3 existing literal behaviors passed.
• Native smoke test using an IPv6-capable NIXL 1.5.0 build: metadata fetching and a verified 512 KiB DRAM READ passed through 127.0.0.1, ::1, [::1], and localhost, invalidating remote metadata between address cases.

Summary by CodeRabbit

• Features

  • Worker hostnames are resolved to IPv4 or IPv6 addresses before metadata exchange. Numeric addresses bypass DNS, and IPv6 scope IDs are preserved.
  • The first address returned by the resolver is used; alternate addresses are not tried if the connection fails. Resolution errors are reported before metadata exchange.

• Documentation

  • Added guidance on endpoint address handling and IPv6 support requirements for metadata exchange.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: ai-dynamo/modelexpress/.coderabbit.yaml
• Review profile: CHILL
• Plan: Enterprise
• Run ID: 98ec8dae-8525-4801-9ac5-28152dcec3f3

📥 Commits

Reviewing files that changed from the base of the PR and between f148036 and 8a12bc5.

📒 Files selected for processing (4)

• docs/ARCHITECTURE.md
• docs/DEPLOYMENT.md
• modelexpress_client/python/modelexpress/nixl_transfer.py
• modelexpress_client/python/tests/test_nixl_metadata_addresses.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.

Walkthrough

The NIXL metadata-fetch path now resolves hostnames to numeric IPv4 or IPv6 addresses. It normalizes bracketed IP literals, preserves IPv6 scope IDs, and records the resolved endpoint for the loaded peer. Tests and documentation cover address selection, DNS errors, and IPv6 support requirements.

Changes

Metadata Endpoint Resolution

Layer / File(s)	Summary
Resolve and fetch metadata endpoint modelexpress_client/python/modelexpress/nixl_transfer.py, modelexpress_client/python/tests/test_nixl_metadata_addresses.py, docs/ARCHITECTURE.md, docs/DEPLOYMENT.md	The client passes the first resolved IPv4 or IPv6 address to NIXL and records that address for the peer. Numeric addresses bypass DNS; bracketed IPv6 is normalized, and scope IDs are preserved. Tests cover address selection and DNS failures. The documentation describes address handling and the NIXL IPv6 support requirement.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~12 minutes

Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to 8a12b

Hostname resolution does not appear to bypass existing transfer failure handling. No merge-blocking issue is established.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 2 files. (2 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		[#847] fetch_remote_and_wait resolves the endpoint before calling NIXL. _resolve_metadata_host preserves numeric IPv4/IPv6 literals, unwraps bracketed literals, uses `getaddrinfo(AF_UNSPEC, SOCK_S…
Out of Scope Changes check		The implementation, regression tests, and architecture/deployment documentation all support the directly linked objective [#847]. The documented first-address behavior also matches the issue's stated …
Title check		The title clearly and concisely summarizes the main change: resolving NIXL metadata hostnames before fetching metadata.
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.

Full details: Linked Issues check

Explanation

[#847] fetch_remote_and_wait resolves the endpoint before calling NIXL. _resolve_metadata_host preserves numeric IPv4/IPv6 literals, unwraps bracketed literals, uses getaddrinfo(AF_UNSPEC, SOCK_STREAM) for hostnames, selects the first IPv4/IPv6 result, and preserves IPv6 scope IDs. DNS errors and results without a usable address fail before NIXL is called. The resolved endpoint is stored for peer lifecycle tracking. The new regression tests cover these behaviors. docs/DEPLOYMENT.md documents the NIXL IPv6 support requirement and lack of address retry; docs/ARCHITECTURE.md documents endpoint resolution.

Full details: Out of Scope Changes check

Explanation

The implementation, regression tests, and architecture/deployment documentation all support the directly linked objective [#847]. The documented first-address behavior also matches the issue's stated absence of multi-address failover. No unrelated changes appear in the reviewed diff.

Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 12 functions across 2 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{A rabbit checks the host by name,
Then finds an address for the frame.
IPv4 or IPv6,
Scope IDs stay in the mix.
NIXL gets the number, clear and plain.}

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[VanyaGlazunov]

Hello, @zhengluo-nv @nv-hwoo, could you take a look when you have time, please?

[VanyaGlazunov]

@KavinKrishnan Thanks for the approval! Can I get ok to test?

[KavinKrishnan]

@KavinKrishnan Thanks for the approval! Can I get ok to test?

Could you rebase with main, apologies

[KavinKrishnan]

/ok to test e1c1f71

[VanyaGlazunov]

Hi, @KavinKrishnan, thanks again for helping me put together this contribution. As far as I am concerned remaining CI failures appear related to pytest version change rather than this pr content.

Is there anything else needed from my side before this can be merged? Happy to make further changes if needed!