Skip to content
This repository was archived by the owner on Jan 23, 2026. It is now read-only.

Stream exporter logs in jmp shell #567

Merged
mangelajo merged 1 commit into from
Jul 15, 2025
Merged

Stream exporter logs in jmp shell #567

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/jumpstarter-cli/jumpstarter_cli/j.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,7 @@ async def cli():
async with BlockingPortal() as portal:
with ExitStack() as stack:
async with env_async(portal, stack) as client:
async with client.log_stream_async():
await to_thread.run_sync(lambda: client.cli()(standalone_mode=False))
await to_thread.run_sync(lambda: client.cli()(standalone_mode=False))

try:
async with create_task_group() as tg:
Expand Down
28 changes: 20 additions & 8 deletions packages/jumpstarter-cli/jumpstarter_cli/shell.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
@click.option("--lease", "lease_name")
@opt_selector
@opt_duration_partial(default=timedelta(minutes=30), show_default="00:30:00")
@click.option("--exporter-logs", is_flag=True, help="Enable exporter log streaming")
# end client specific
@handle_exceptions_with_reauthentication(relogin_client)
def shell(config, command: tuple[str, ...], lease_name, selector, duration):
def shell(config, command: tuple[str, ...], lease_name, selector, duration, exporter_logs):
"""
Spawns a shell (or custom command) connecting to a local or remote exporter

Expand All @@ -42,13 +43,24 @@ def shell(config, command: tuple[str, ...], lease_name, selector, duration):
with config.lease(selector=selector, lease_name=lease_name, duration=duration) as lease:
with lease.serve_unix() as path:
with lease.monitor():
exit_code = launch_shell(
path,
"remote",
config.drivers.allow,
config.drivers.unsafe,
command=command,
)
if exporter_logs:
with lease.connect() as client:
with client.log_stream():
exit_code = launch_shell(
path,
"remote",
config.drivers.allow,
config.drivers.unsafe,
command=command,
)
else:
exit_code = launch_shell(
path,
"remote",
config.drivers.allow,
config.drivers.unsafe,
command=command,
)
Comment on lines +46 to +63
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jul 15, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Refactor to eliminate code duplication.

The conditional logic contains significant code duplication. The launch_shell call is identical in both branches, only the context differs.

-                        if exporter_logs:
-                            with lease.connect() as client:
-                                with client.log_stream():
-                                    exit_code = launch_shell(
-                                        path,
-                                        "remote",
-                                        config.drivers.allow,
-                                        config.drivers.unsafe,
-                                        command=command,
-                                    )
-                        else:
-                            exit_code = launch_shell(
-                                path,
-                                "remote",
-                                config.drivers.allow,
-                                config.drivers.unsafe,
-                                command=command,
-                            )
+                        def run_shell():
+                            return launch_shell(
+                                path,
+                                "remote",
+                                config.drivers.allow,
+                                config.drivers.unsafe,
+                                command=command,
+                            )
+                        
+                        if exporter_logs:
+                            with lease.connect() as client:
+                                with client.log_stream():
+                                    exit_code = run_shell()
+                        else:
+                            exit_code = run_shell()
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if exporter_logs:
with lease.connect() as client:
with client.log_stream():
exit_code = launch_shell(
path,
"remote",
config.drivers.allow,
config.drivers.unsafe,
command=command,
)
else:
exit_code = launch_shell(
path,
"remote",
config.drivers.allow,
config.drivers.unsafe,
command=command,
)
def run_shell():
return launch_shell(
path,
"remote",
config.drivers.allow,
config.drivers.unsafe,
command=command,
)
if exporter_logs:
with lease.connect() as client:
with client.log_stream():
exit_code = run_shell()
else:
exit_code = run_shell()
🤖 Prompt for AI Agents 
In packages/jumpstarter-cli/jumpstarter_cli/shell.py between lines 46 and 63,
the launch_shell function is called with the same arguments in both branches of
the if-else, causing code duplication. Refactor by moving the launch_shell call
outside the if-else block and only conditionally wrap it with the
lease.connect() and client.log_stream() context managers when exporter_logs is
true, to eliminate the duplicated launch_shell call.


sys.exit(exit_code)

Expand Down
Loading