fix(sp37-followup): add SftpClient.stat() method
The SftpClient wrapper previously exposed write_file, list_inbound, list_inbound_names, download_inbound, and read_file — but NO stat(). The canonical cyclone.submission.submit_file helper needs to compare the local file size against the remote file size for idempotency (SKIPPED outcome short-circuit); the lack of stat() forced the helper to bypass the wrapper and open paramiko directly via submission/core.py:_default_sftp_factory. This commit adds: - SftpStat frozen dataclass: narrow projection of paramiko's SFTPAttributes (size + modified_at). Callers don't need paramiko. - SftpClient.stat(remote_path) -> SftpStat public method - _stat_stub + _stat_paramiko backends (mirrors the existing _read_file_stub / _read_file_paramiko pattern) - 8 new tests covering dataclass shape, frozen invariant, stub size, stub mtime, missing-file FileNotFoundError, large-file size, and the actual SKIPPED-outcome idempotency check from submit_file Also adds backend/var/ to .gitignore — contains HCPF-delivered inbound production files that should never be committed (same rationale as the existing ingest/ entry).
This commit is contained in:
@@ -7,3 +7,4 @@ __pycache__/
|
|||||||
venv/
|
venv/
|
||||||
build/
|
build/
|
||||||
dist/
|
dist/
|
||||||
|
var/
|
||||||
|
|||||||
@@ -94,6 +94,22 @@ def _op_timeout_seconds() -> float:
|
|||||||
return value
|
return value
|
||||||
|
|
||||||
|
|
||||||
|
@dataclass(frozen=True)
|
||||||
|
class SftpStat:
|
||||||
|
"""File metadata returned by :meth:`SftpClient.stat`.
|
||||||
|
|
||||||
|
Mirrors the subset of paramiko's ``SFTPAttributes`` that callers
|
||||||
|
actually need (``size``, ``modified_at``). Keeping the surface
|
||||||
|
narrow means callers don't need to import paramiko to consume
|
||||||
|
the result, and the wrapper's stub mode can populate it from a
|
||||||
|
plain ``os.stat_result`` without any paramiko dance.
|
||||||
|
|
||||||
|
Frozen so callers can hash / cache the result if they need to.
|
||||||
|
"""
|
||||||
|
size: int
|
||||||
|
modified_at: datetime | None = None
|
||||||
|
|
||||||
|
|
||||||
@dataclass
|
@dataclass
|
||||||
class InboundFile:
|
class InboundFile:
|
||||||
"""A single file observed in the inbound MFT path."""
|
"""A single file observed in the inbound MFT path."""
|
||||||
@@ -216,6 +232,30 @@ class SftpClient:
|
|||||||
return self._read_file_stub(remote_path)
|
return self._read_file_stub(remote_path)
|
||||||
return self._read_file_paramiko(remote_path)
|
return self._read_file_paramiko(remote_path)
|
||||||
|
|
||||||
|
def stat(self, remote_path: str) -> SftpStat:
|
||||||
|
"""Return file metadata for ``remote_path``.
|
||||||
|
|
||||||
|
Mirrors the subset of paramiko's ``SFTPAttributes`` that
|
||||||
|
callers need (``size`` for idempotency checks; ``modified_at``
|
||||||
|
for cache-busting). The SP37 submission helper uses
|
||||||
|
``stat().size`` to short-circuit re-uploads of already-uploaded
|
||||||
|
files (the SKIPPED outcome path).
|
||||||
|
|
||||||
|
Stub mode: reads ``os.stat_result`` from
|
||||||
|
``{staging_dir}/{remote_path}``.
|
||||||
|
|
||||||
|
Real mode: calls ``sftp.stat(remote_path)`` on a paramiko
|
||||||
|
connection.
|
||||||
|
|
||||||
|
Raises:
|
||||||
|
FileNotFoundError: if ``remote_path`` does not exist
|
||||||
|
(stub: missing local file; real: paramiko raises
|
||||||
|
``IOError`` which is a ``FileNotFoundError`` subclass).
|
||||||
|
"""
|
||||||
|
if self._stub:
|
||||||
|
return self._stat_stub(remote_path)
|
||||||
|
return self._stat_paramiko(remote_path)
|
||||||
|
|
||||||
def _read_file_stub(self, remote_path: str) -> bytes:
|
def _read_file_stub(self, remote_path: str) -> bytes:
|
||||||
"""Read bytes from ``{staging_dir}/{remote_path}`` (SP16 stub)."""
|
"""Read bytes from ``{staging_dir}/{remote_path}`` (SP16 stub)."""
|
||||||
staging = Path(self._block.staging_dir).resolve()
|
staging = Path(self._block.staging_dir).resolve()
|
||||||
@@ -224,6 +264,25 @@ class SftpClient:
|
|||||||
raise FileNotFoundError(f"inbound stub file not found: {target}")
|
raise FileNotFoundError(f"inbound stub file not found: {target}")
|
||||||
return target.read_bytes()
|
return target.read_bytes()
|
||||||
|
|
||||||
|
def _stat_stub(self, remote_path: str) -> SftpStat:
|
||||||
|
"""Return ``SftpStat`` for ``{staging_dir}/{remote_path}`` (SP37 stub).
|
||||||
|
|
||||||
|
Mirrors what paramiko's ``sftp.stat()`` returns in real mode:
|
||||||
|
``size`` from ``st.st_size`` and ``modified_at`` from
|
||||||
|
``st.st_mtime``. Raises ``FileNotFoundError`` if the local
|
||||||
|
file is missing — matches paramiko's ``IOError`` behavior
|
||||||
|
so the caller doesn't need to special-case stub mode.
|
||||||
|
"""
|
||||||
|
staging = Path(self._block.staging_dir).resolve()
|
||||||
|
target = staging / remote_path.lstrip("/")
|
||||||
|
if not target.is_file():
|
||||||
|
raise FileNotFoundError(f"inbound stub file not found: {target}")
|
||||||
|
st = target.stat()
|
||||||
|
return SftpStat(
|
||||||
|
size=st.st_size,
|
||||||
|
modified_at=datetime.fromtimestamp(st.st_mtime),
|
||||||
|
)
|
||||||
|
|
||||||
def get_secret(self, name: str) -> Optional[str]:
|
def get_secret(self, name: str) -> Optional[str]:
|
||||||
"""Fetch the auth secret from Keychain. Returns the stub secret if absent."""
|
"""Fetch the auth secret from Keychain. Returns the stub secret if absent."""
|
||||||
value = secrets.get_secret(name)
|
value = secrets.get_secret(name)
|
||||||
@@ -506,6 +565,20 @@ class SftpClient:
|
|||||||
shutil.copyfileobj(f, buf, length=64 * 1024)
|
shutil.copyfileobj(f, buf, length=64 * 1024)
|
||||||
return buf.getvalue()
|
return buf.getvalue()
|
||||||
|
|
||||||
|
def _stat_paramiko(self, remote_path: str) -> SftpStat:
|
||||||
|
"""Return ``SftpStat`` for ``remote_path`` via paramiko.
|
||||||
|
|
||||||
|
Paramiko's ``SFTPAttributes`` already provides ``st_size`` and
|
||||||
|
``st_mtime``; we project them into our narrow public
|
||||||
|
``SftpStat`` shape so callers don't need to know about paramiko.
|
||||||
|
"""
|
||||||
|
with self._connect() as (ssh, sftp):
|
||||||
|
attr = sftp.stat(remote_path)
|
||||||
|
return SftpStat(
|
||||||
|
size=attr.st_size or 0,
|
||||||
|
modified_at=datetime.fromtimestamp(attr.st_mtime or 0),
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# ---------------------------------------------------------------------------
|
# ---------------------------------------------------------------------------
|
||||||
# Module-level helper
|
# Module-level helper
|
||||||
|
|||||||
@@ -0,0 +1,150 @@
|
|||||||
|
"""SP37 follow-up #5: SftpClient.stat() method.
|
||||||
|
|
||||||
|
The SftpClient wrapper previously exposed ``write_file``,
|
||||||
|
``list_inbound``, ``list_inbound_names``, ``download_inbound``, and
|
||||||
|
``read_file`` — but NO ``stat()``. The canonical
|
||||||
|
``cyclone.submission.submit_file`` helper needs to compare the local
|
||||||
|
file size against the remote file size for idempotency; the lack of
|
||||||
|
``stat()`` forced the helper to bypass the wrapper and open paramiko
|
||||||
|
directly (see ``submission/core.py:_default_sftp_factory``).
|
||||||
|
|
||||||
|
This file adds a ``stat()`` method to the wrapper so the helper can
|
||||||
|
use ``SftpClient(block=b)`` instead of bypassing the wrapper. Tests
|
||||||
|
cover the stub implementation only (paramiko real-mode tests would
|
||||||
|
need a real SFTP server).
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from cyclone.clearhouse import SftpClient, SftpStat
|
||||||
|
from cyclone.providers import SftpBlock
|
||||||
|
|
||||||
|
|
||||||
|
def _stub_block(staging_dir: Path) -> SftpBlock:
|
||||||
|
return SftpBlock(
|
||||||
|
host="mft.example.com",
|
||||||
|
port=22,
|
||||||
|
username="user",
|
||||||
|
auth={"password_keychain_account": "x"},
|
||||||
|
paths={"inbound": "/inbound", "outbound": "/outbound"},
|
||||||
|
staging_dir=str(staging_dir),
|
||||||
|
stub=True,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def staging(tmp_path: Path) -> Path:
|
||||||
|
"""A staging dir pre-populated with one file. Mirrors what the
|
||||||
|
SP9 stub layout looks like when an operator has dropped a real
|
||||||
|
inbound file for testing."""
|
||||||
|
inbound = tmp_path / "inbound"
|
||||||
|
inbound.mkdir(parents=True)
|
||||||
|
(inbound / "test.txt").write_bytes(b"hello world\n")
|
||||||
|
return tmp_path
|
||||||
|
|
||||||
|
|
||||||
|
# ---- SftpStat dataclass ----------------------------------------------------- #
|
||||||
|
|
||||||
|
|
||||||
|
def test_sftp_stat_dataclass_exists():
|
||||||
|
"""The wrapper exposes a small SftpStat dataclass for the result.
|
||||||
|
|
||||||
|
Mirrors the paramiko ``SFTPAttributes`` shape (at least ``size``)
|
||||||
|
but keeps the public surface narrow so callers can use it
|
||||||
|
without depending on paramiko.
|
||||||
|
"""
|
||||||
|
assert SftpStat is not None
|
||||||
|
stat = SftpStat(size=42, modified_at=None)
|
||||||
|
assert stat.size == 42
|
||||||
|
assert stat.modified_at is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_sftp_stat_is_frozen():
|
||||||
|
"""SftpStat is a frozen dataclass so it can be hashed / used as a
|
||||||
|
cache key by future callers without surprise mutation."""
|
||||||
|
from dataclasses import FrozenInstanceError
|
||||||
|
|
||||||
|
stat = SftpStat(size=1, modified_at=None)
|
||||||
|
with pytest.raises(FrozenInstanceError):
|
||||||
|
stat.size = 2 # type: ignore[misc]
|
||||||
|
|
||||||
|
|
||||||
|
# ---- Public method --------------------------------------------------------- #
|
||||||
|
|
||||||
|
|
||||||
|
def test_sftp_client_has_stat_method():
|
||||||
|
"""``SftpClient`` exposes a ``stat(remote_path)`` public method.
|
||||||
|
|
||||||
|
Guards the API surface — a future contributor who renames or
|
||||||
|
removes ``stat()`` (forcing the helper to bypass the wrapper
|
||||||
|
again) would break this test, prompting them to update the
|
||||||
|
helper instead.
|
||||||
|
"""
|
||||||
|
assert hasattr(SftpClient, "stat")
|
||||||
|
assert callable(SftpClient.stat)
|
||||||
|
|
||||||
|
|
||||||
|
def test_sftp_client_stat_returns_size_in_stub_mode(staging: Path):
|
||||||
|
"""Stub mode: ``stat(remote_path)`` returns the local staging
|
||||||
|
file's size via ``os.stat_result.st_size``. Mirrors what a real
|
||||||
|
paramiko ``SFTPAttributes.st_size`` would return for a remote file.
|
||||||
|
"""
|
||||||
|
client = SftpClient(block=_stub_block(staging))
|
||||||
|
stat = client.stat("/inbound/test.txt")
|
||||||
|
assert stat.size == len(b"hello world\n")
|
||||||
|
|
||||||
|
|
||||||
|
def test_sftp_client_stat_returns_modified_at_in_stub_mode(staging: Path):
|
||||||
|
"""Stub mode: ``stat`` populates ``modified_at`` from the local
|
||||||
|
file's mtime. Mirrors paramiko's behavior for parity."""
|
||||||
|
client = SftpClient(block=_stub_block(staging))
|
||||||
|
stat = client.stat("/inbound/test.txt")
|
||||||
|
assert stat.modified_at is not None
|
||||||
|
|
||||||
|
|
||||||
|
def test_sftp_client_stat_raises_for_missing_file(staging: Path):
|
||||||
|
"""Stub mode: ``stat`` raises ``FileNotFoundError`` for a path
|
||||||
|
that doesn't exist locally — matches the real paramiko behavior
|
||||||
|
(which raises ``IOError``/``FileNotFoundError``). The helper's
|
||||||
|
SKIPPED-outcome short-circuit depends on this exception type.
|
||||||
|
"""
|
||||||
|
client = SftpClient(block=_stub_block(staging))
|
||||||
|
with pytest.raises(FileNotFoundError):
|
||||||
|
client.stat("/inbound/missing.txt")
|
||||||
|
|
||||||
|
|
||||||
|
def test_sftp_client_stat_handles_large_file(staging: Path):
|
||||||
|
"""Stub mode: ``stat`` returns the correct size for non-trivial files.
|
||||||
|
|
||||||
|
Sanity check: the size path doesn't truncate or accidentally
|
||||||
|
cast (e.g. to int8) on larger files.
|
||||||
|
"""
|
||||||
|
payload = b"x" * (1024 * 1024) # 1 MiB
|
||||||
|
(staging / "inbound" / "big.bin").write_bytes(payload)
|
||||||
|
client = SftpClient(block=_stub_block(staging))
|
||||||
|
stat = client.stat("/inbound/big.bin")
|
||||||
|
assert stat.size == 1024 * 1024
|
||||||
|
|
||||||
|
|
||||||
|
def test_sftp_client_stat_supports_idempotency_check(staging: Path):
|
||||||
|
"""The submission helper's idempotency check works against the
|
||||||
|
wrapper now.
|
||||||
|
|
||||||
|
Replays the SKIPPED-outcome check from
|
||||||
|
``cyclone.submission.submit_file`` (lines 178-182) using the
|
||||||
|
wrapper directly. Before this follow-up, the helper had to open
|
||||||
|
paramiko directly because the wrapper lacked ``stat()``.
|
||||||
|
"""
|
||||||
|
payload = b"identical contents"
|
||||||
|
(staging / "outbound").mkdir(parents=True, exist_ok=True)
|
||||||
|
(staging / "outbound" / "claim.x12").write_bytes(payload)
|
||||||
|
|
||||||
|
client = SftpClient(block=_stub_block(staging))
|
||||||
|
local_size = len(payload)
|
||||||
|
# stat() returns the size that was previously uploaded.
|
||||||
|
remote_stat = client.stat("/outbound/claim.x12")
|
||||||
|
assert remote_stat.size == local_size # idempotency check passes
|
||||||
|
# → SKIPPED outcome in the submission helper
|
||||||
Reference in New Issue
Block a user