: v1/logging: avoid spinning up ... more tests #1725
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
meta-cla
bot
added
the
CLA Signed
|
@shayne-fletcher has exported this pull request. If you are a Meta employee, you can view the originating Diff in D85969320. |
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Oct 31, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
from
b887e34 to
55521b1
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Oct 31, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
2 times, most recently
from
a10efa1 to
c3592a2
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
from
c3592a2 to
6693b43
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
from
6693b43 to
7ae1d90
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
2 times, most recently
from
2359aed to
1f89b92
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
from
1f89b92 to
4ec3c34
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
from
4ec3c34 to
bdf25d9
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
from
bdf25d9 to
719b6aa
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
from
719b6aa to
cc398a7
Compare
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
from
cc398a7 to
b2d94f2
Compare
Summary: yesterday's fix D86025607 is flaky. today's observed behavior shows the torch API still suffers from caching / runner variability. this fix is more authoritative: pin torch to 2.9.0 (stable): replace `pip install --pre torch --index-url https://download.pytorch.org/whl/nightly/cu126` with `pip install torch==2.9.0+cu126 --index-url https://download.pytorch.org/whl/cu126` pulling "nightly" exposes us to non-determinism. relying on a published version restores hermeticity. anyone can stamp/land and we should expect to recover green actions. Differential Revision: D86052365
…eta-pytorch#1722) Summary: this diff changes `LoggingMeshClient::spawn` so that log forwarding is no longer unconditional. previously we always spawned a `LogForwardActor` mesh across the `ProcMesh` and stored it in the client. now we read `MESH_ENABLE_LOG_FORWARDING` and only create that mesh if the flag is set. when forwarding is disabled we don’t spawn any `LogForwardActor` actors at all and we record `forwarder_mesh` as `None`. `logger_mesh` (the per-proc `LoggerRuntimeActor` that manages python logging state) is still always spawned. `client_actor` is still spawned locally in the caller's process. because `forwarder_mesh` is now an `Option`, the rest of the API is updated to match the new contract. `set_mode` now either propagates `stream_to_client` to the forwarders when they exist, silently accepts"don’t stream" when they don’t, or returns an error if the caller tries to enable streaming in a configuration where we never created forwarders. `flush` now no-ops successfully in the case where there is no forwarding mesh instead of assuming that those actors are always present. the diff also adds the minimal test harness needed to exercise this logic end-to-end. there is a dedicated bootstrap binary in `monarch_hyperactor` (`monarch_hyperactor_test_bootstrap`) which initializes python and then runs the mesh bootstrap protocol, so remote procs can safely construct `LoggerRuntimeActor `without panicking on `Python::with_gil`. we also add `HostMesh::local_with_bootstrap`, which lets tests stand up a single-host mesh using an explicit `BootstrapCommand` instead of relying on the implicit "current process" path. we also add `ensure_python` and `AwaitPyExt `on the rust side so the tokio tests can stand up a `ProcMesh`, call the python-facing `spawn()`, await the returned `PyPythonTask`, and inspect the resulting `LoggingMeshClient`. finally, there is a test that brings up a tiny mesh, forces the forwarding flag off and on, and in each case asserts that `forwarder_mesh` is `None` or `Some(..)` accordingly. follow-up diffs will extend coverage to `set_mode`, `flush`, and shutdown semantics, now that the plumbing exists. Differential Revision: D85919326
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
added a commit
to shayne-fletcher/monarch-1
that referenced
this pull request
Nov 2, 2025
Summary: this diff adds end-to-end tests for `LoggingMeshClient::spawn,` `set_mode`, and `flush`, covering both values of `MESH_ENABLE_LOG_FORWARDING`. with forwarding disabled we assert that `spawn()` returns a client with `forwarder_mesh == None`, that `set_mode()` refuses to enable streaming (and rejects `aggregate_window_sec` unless streaming is true), and that `flush()` returns `Ok(())` as a no-op. with forwarding enabled we assert that `forwarder_mesh.is_some()`, that `set_mode()` can enable streaming and set an aggregate window, and that `flush()` runs the sync flush barrier and returns `Ok(())`. the diff also adds `await_unit()` to `AwaitPyExt` so tests can await `PyPythonTask`s that conceptually return `None`, and tightens the rust-side docs for `LoggingMeshClient`, its `drop` semantics, and the `flush` protocol. Differential Revision: D85969320
shayne-fletcher
force-pushed
the
export-D85969320
branch
2 times, most recently
from
1264a80 to
0b1fa16
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: this diff adds end-to-end tests for
LoggingMeshClient::spawn,set_mode, andflush, covering both values ofMESH_ENABLE_LOG_FORWARDING. with forwarding disabled we assert thatspawn()returns a client withforwarder_mesh == None, thatset_mode()refuses to enable streaming (and rejectsaggregate_window_secunless streaming is true), and thatflush()returnsOk(())as a no-op. with forwarding enabled we assert thatforwarder_mesh.is_some(), thatset_mode()can enable streaming and set an aggregate window, and thatflush()runs the sync flush barrier and returnsOk(()). the diff also addsawait_unit()toAwaitPyExtso tests can awaitPyPythonTasks that conceptually returnNone, and tightens the rust-side docs forLoggingMeshClient, itsdropsemantics, and theflushprotocol.Differential Revision: D85969320