Skip to content

feat: make Juju.temp_dir public, improve docs #205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

feat: make Juju.temp_dir public, improve docs #205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
54efbd6
feat: make Juju.temp_dir public, improve docs
benhoyt Oct 8, 2025
0dbd7ee
oops, update references
benhoyt Oct 8, 2025
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
55 changes: 36 additions & 19 deletions jubilant/_juju.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,36 @@ def __repr__(self) -> str:
]
return f'Juju({", ".join(args)})'

# Keep the public methods in alphabetical order, so we don't have to think
@functools.cached_property
def temp_dir(self) -> str:
Copy link

Choose a reason for hiding this comment

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

Probably nicer to return a pathlib.Path object instead of a str -- path-like objects can be used with the dir argument of the tempfile functions.

Copy link

Choose a reason for hiding this comment

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

Maybe the name default_temp_dir would make the semantics clearer, though it would mean that what we're asking users to write whenever they use tempfile methods would be a little bit longer.

"""Path of a temporary directory accessible by the Juju CLI.

The directory is created and cached on first use. If :attr:`cli_binary` points to a Juju
Comment on lines +102 to +104
Copy link

Choose a reason for hiding this comment

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

We should make it clear that this returns the path to the conventional system or snap temporary directory. The fact that it's a cached property helps, but I do wonder if users will at first think of using it like you would with tempfile.TemporaryDirectory() ....

Maybe the initial line could end with 'typically /tmp' to make it clear via an example.

I wonder if 'The directory is created and cached on first use.' should be expanded to include:

  • The directory is created on first use if it doesn't exist.
  • The cached path is returned on later uses.
  • This will be the same directory for all Juju instances with the same binary, and indeed across multiple runs of the program.
  • We don't clean up any files created there.

CLI installed as a snap, this will be a directory accessible to the snap (under
``~/snap/juju/common``), otherwise it will be under ``/tmp``.

Example::

juju = jubilant.Juju()
with tempfile.NamedTemporaryFile('w+', dir=juju.temp_dir) as file:
file.write('contents')
file.flush()
juju.scp(file.name, 'ubuntu/0:/path/to/destination')
Comment on lines +111 to +114
Copy link
Contributor

Choose a reason for hiding this comment

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

If temp_dir is a pathlib.Path, I'd personally use it like this:

Suggested change
with tempfile.NamedTemporaryFile('w+', dir=juju.temp_dir) as file:
file.write('contents')
file.flush()
juju.scp(file.name, 'ubuntu/0:/path/to/destination')
(juju.temp_dir / 'cont').write_text('contents')
juju.scp(juju.temp_dir / 'cont', 'ubuntu/0:/path/to/destination')

Practicality over purity: if it's an integration test, there will be only a few named files.

The named temp file would be still useful in a case where Jubilant is used in production and the process is long-lived. In that case very many scp calls could be expected.

Copy link

Choose a reason for hiding this comment

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

Brainstorming a little bit, I agree that this is a nice UX -- probably the way we'd normally do it in tests is using pytest's tmp_path fixture, which takes care of cleanup when the fixture exits. But of course that specific fixture won't work with snap Juju. I'm wondering if we could provide cleanup functionality in a similar way though. For example, Juju.temp_dir could create and return a subdir of the current temp location and record created directories, and the juju fixture could clean them up on exiting.

"""
if self._juju_is_snap:
# If Juju is running as a snap, we can't use /tmp, so put temp files here instead.
temp_dir = os.path.expanduser('~/snap/juju/common')
os.makedirs(temp_dir, exist_ok=True)
return temp_dir
else:
return tempfile.gettempdir()

@functools.cached_property
def _juju_is_snap(self) -> bool:
which = shutil.which(self.cli_binary)
return which is not None and '/snap/' in which

# Keep the public methods below in alphabetical order, so we don't have to think
# about where to put each new method.

def add_model(
Expand Down Expand Up @@ -162,7 +191,7 @@ def add_secret(
if info is not None:
args.extend(['--info', info])

with tempfile.NamedTemporaryFile('w+', dir=self._temp_dir) as file:
with tempfile.NamedTemporaryFile('w+', dir=self.temp_dir) as file:
_yaml.safe_dump(content, file)
file.flush()
args.extend(['--file', file.name])
Expand Down Expand Up @@ -935,7 +964,7 @@ def run(
args.extend(['--wait', f'{wait}s'])

with (
tempfile.NamedTemporaryFile('w+', dir=self._temp_dir)
tempfile.NamedTemporaryFile('w+', dir=self.temp_dir)
if params is not None
else contextlib.nullcontext()
) as params_file:
Expand Down Expand Up @@ -977,6 +1006,9 @@ def scp(
) -> None:
"""Securely transfer files within a model.

A local *source* or *destination* must be accessible by the Juju CLI (important if Juju
is installed as a confined snap). See :attr:`temp_dir` for an example.
Copy link

Choose a reason for hiding this comment

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

Suggested change
is installed as a confined snap). See :attr:`temp_dir` for an example.
is installed as a confined snap). See :attr:`temp_dir` for an example of how to create a
temporary file accessible by Juju.


Args:
source: Source of file, in format ``[[<user>@]<target>:]<path>``.
destination: Destination for file, in format ``[<user>@]<target>[:<path>]``.
Expand Down Expand Up @@ -1177,7 +1209,7 @@ def update_secret(
if auto_prune:
args.append('--auto-prune')

with tempfile.NamedTemporaryFile('w+', dir=self._temp_dir) as file:
with tempfile.NamedTemporaryFile('w+', dir=self.temp_dir) as file:
_yaml.safe_dump(content, file)
file.flush()
args.extend(['--file', file.name])
Expand Down Expand Up @@ -1269,21 +1301,6 @@ def wait(
raise TimeoutError(f'wait timed out after {timeout}s')
raise TimeoutError(f'wait timed out after {timeout}s\n{status}')

@functools.cached_property
def _juju_is_snap(self) -> bool:
which = shutil.which(self.cli_binary)
return which is not None and '/snap/' in which

@functools.cached_property
def _temp_dir(self) -> str:
if self._juju_is_snap:
# If Juju is running as a snap, we can't use /tmp, so put temp files here instead.
temp_dir = os.path.expanduser('~/snap/juju/common')
os.makedirs(temp_dir, exist_ok=True)
return temp_dir
else:
return tempfile.gettempdir()


def _format_config(k: str, v: ConfigValue) -> str:
if v is None: # type: ignore
Expand Down
7 changes: 4 additions & 3 deletions tests/integration/test_execution.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,10 @@ def test_ssh_and_scp(juju: jubilant.Juju):
output = juju.ssh('snappass-test/0', 'ls', '/charm/container', container='redis')
assert 'pebble' in output.split()

juju.scp('snappass-test/0:agents/unit-snappass-test-0/charm/src/charm.py', 'charm.py')
charm_src = pathlib.Path('charm.py').read_text()
assert 'class Snappass' in charm_src
pathlib.Path('scptest').write_text('SCPTEST')
juju.scp('scptest', 'snappass-test/0:/tmp/scptest2')
juju.scp('snappass-test/0:/tmp/scptest2', 'scptest3')
assert pathlib.Path('scptest3').read_text() == 'SCPTEST'

juju.scp('snappass-test/0:/etc/passwd', 'passwd', container='redis')
passwd = pathlib.Path('passwd').read_text()
Expand Down
14 changes: 14 additions & 0 deletions tests/integration/test_machine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
from __future__ import annotations

import pathlib
import tempfile

import pytest

import jubilant
Expand Down Expand Up @@ -33,6 +36,17 @@ def test_ssh(juju: jubilant.Juju):
assert output == 'MACHINE\n'


def test_scp(juju: jubilant.Juju):
with tempfile.NamedTemporaryFile('w+', dir=juju.temp_dir) as file:
file.write('SCPTEST_MACHINE')
file.flush()
juju.scp(file.name, 'ubuntu/0:/tmp/scptest')

with tempfile.NamedTemporaryFile('w+', dir=juju.temp_dir) as file:
juju.scp('ubuntu/0:/tmp/scptest', file.name)
assert pathlib.Path(file.name).read_text() == 'SCPTEST_MACHINE'


def test_add_and_remove_unit(juju: jubilant.Juju):
juju.add_unit('ubuntu')
juju.wait(lambda status: jubilant.all_active(status) and len(status.apps['ubuntu'].units) == 2)
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/test_juju_class.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ def test_default_tempdir(monkeypatch: pytest.MonkeyPatch):
monkeypatch.setattr('shutil.which', lambda _: '/bin/juju') # type: ignore
juju = jubilant.Juju()

assert 'snap' not in juju._temp_dir
assert 'snap' not in juju.temp_dir


def test_snap_tempdir(monkeypatch: pytest.MonkeyPatch):
monkeypatch.setattr('shutil.which', lambda _: '/snap/bin/juju') # type: ignore
juju = jubilant.Juju()

assert 'snap' in juju._temp_dir
assert 'snap' in juju.temp_dir