fix(omny): migrate scilog writes to bec.messaging.scilog, checking _enabled
CI for csaxs_bec / test (push) Failing after 5s
CI for csaxs_bec / test (push) Failing after 5s
write_to_scilog()/write_pdf_report() used the old bec.logbook/client.logbook. send_logbook_message() API, which has no "is scilog even configured" check. Confirmed live: on a session without scilog credentials (this dev deployment has no /etc/bec/secrets/.scilog.env at all), send() blocks indefinitely instead of failing fast -- a real tomo_scan() hung inside _write_subtomo_to_scilog() for 29+ minutes during Phase 1 live testing. flomni already migrated off this API to bec.messaging.scilog, which gates every send behind scilog._enabled (published from the deployment's actual scilog config) -- per Mirko's direction, ported the same mechanism to both OMNY call sites, matching flomni's write_pdf_report()/_scilog_write() exactly rather than merely adding an active_account check. Added 3 unit tests locking in the _enabled gate (skip when disabled, send when enabled, tolerate a send() exception). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLrD7sVYGLAzsQjLVJpCgt
This commit is contained in:
1 parent
906e9f5318
commit
2eaa609d29
2 files changed
+123
-22
No files matched your search
@@ -1066,17 +1066,36 @@ class OMNY(
|
||||
def sample_name(self):
|
||||
return dev.omny_samples.get_sample_name_in_samplestage()
|
||||
|
||||
def write_to_scilog(self, content, tags: list = None):
|
||||
def write_to_scilog(self, content, tags: list = None) -> bool:
|
||||
"""Write a text entry to scilog, tolerating scilog being disabled or
|
||||
unreachable. Mirrors flomni's _scilog_write(): checks
|
||||
scilog._enabled before attempting a send, rather than trying and
|
||||
catching whatever exception (or hang) an unconfigured/unreachable
|
||||
scilog produces -- the old bec.logbook/client.logbook.
|
||||
send_logbook_message() API used here previously had no such check,
|
||||
so a session without scilog credentials configured would block
|
||||
indefinitely on send() instead of failing fast (confirmed live: a
|
||||
real tomo_scan() hung inside _write_subtomo_to_scilog() on a
|
||||
session with no /etc/bec/secrets/.scilog.env at all).
|
||||
|
||||
Returns True if the entry was sent, False if it was skipped/failed
|
||||
(already logged as a warning either way -- callers that don't care
|
||||
about the outcome can ignore the return value).
|
||||
"""
|
||||
scilog = getattr(bec.messaging, "scilog", None)
|
||||
if scilog is None or not getattr(scilog, "_enabled", False):
|
||||
logger.warning("scilog is not enabled; skipping scilog entry.")
|
||||
return False
|
||||
if tags is not None:
|
||||
tags.append("BEC")
|
||||
else:
|
||||
tags = ["BEC"]
|
||||
try:
|
||||
if tags is not None:
|
||||
tags.append("BEC")
|
||||
else:
|
||||
tags = ["BEC"]
|
||||
msg = bec.logbook.LogbookMessage()
|
||||
msg.add_text(content).add_tag(tags)
|
||||
self.client.logbook.send_logbook_message(msg)
|
||||
except Exception:
|
||||
logger.warning("Failed to write to scilog.")
|
||||
scilog.new().add_text(content.replace("\n", "<br>")).add_tags(tags).send()
|
||||
return True
|
||||
except Exception as exc:
|
||||
logger.warning(f"Failed to write to scilog: {exc}")
|
||||
return False
|
||||
|
||||
def tomo_alignment_scan(self):
|
||||
"""
|
||||
@@ -1885,19 +1904,27 @@ class OMNY(
|
||||
"xterm /work/sls/spec/local/XOMNY/bin/upload/upload_last_pon.sh &", shell=True
|
||||
)
|
||||
# status = subprocess.run(f"cp /tmp/spec-e20131-specES1.pdf {user_target}", shell=True)
|
||||
# Same tolerance as write_to_scilog(): a session without scilog/
|
||||
# logbook configured (e.g. a dev/sim session) must not crash report
|
||||
# generation over the logbook upload -- the PDF itself is already
|
||||
# written above.
|
||||
try:
|
||||
msg = bec.logbook.LogbookMessage()
|
||||
# bec.logbook/client.logbook.send_logbook_message() was the old,
|
||||
# pre-migration scilog API -- it has no "is scilog even configured"
|
||||
# check, so on a session without scilog credentials (e.g. this dev
|
||||
# sandbox has no /etc/bec/secrets/.scilog.env at all) the send()
|
||||
# call blocks indefinitely instead of failing fast, hanging the
|
||||
# whole tomo scan. flomni's write_pdf_report() uses
|
||||
# bec.messaging.scilog instead, which is enabled/disabled based on
|
||||
# the deployment's actual published scilog config -- mirrored here.
|
||||
scilog = getattr(bec.messaging, "scilog", None)
|
||||
if scilog is None or not getattr(scilog, "_enabled", False):
|
||||
logger.warning("scilog is not enabled; skipping PDF report entry.")
|
||||
else:
|
||||
logo_path = os.path.join(os.path.dirname(os.path.abspath(__file__)), "OMNY.png")
|
||||
msg.add_file(logo_path).add_text("".join(content).replace("\n", "</p><p>")).add_tag(
|
||||
["BEC", "tomo_parameters", f"dataset_id_{dataset_id}", "OMNY", self.sample_name]
|
||||
)
|
||||
self.client.logbook.send_logbook_message(msg)
|
||||
except Exception:
|
||||
logger.warning("Failed to write PDF report to scilog.")
|
||||
try:
|
||||
scilog.new().add_attachment(logo_path, width=200).add_text(
|
||||
content.replace("\n", "<br>")
|
||||
).add_tags(
|
||||
["BEC", "tomo_parameters", f"dataset_id_{dataset_id}", "OMNY", self.sample_name]
|
||||
).send()
|
||||
except Exception as exc:
|
||||
logger.warning(f"Failed to write PDF report to scilog: {exc}")
|
||||
|
||||
def vcs_show_all():
|
||||
dev.omny_vcs.show_all()
|
||||
|
||||
@@ -181,3 +181,77 @@ def test_tomo_scan_sets_timing_fields_on_new_scan(monkeypatch):
|
||||
assert omny.progress["estimated_remaining_time"] is None
|
||||
assert omny.progress["estimated_finish_time"] is None
|
||||
assert omny.progress["accumulated_idle_time"] == 0.0
|
||||
|
||||
|
||||
# --- write_to_scilog(): checks scilog._enabled before attempting a send -----
|
||||
#
|
||||
# The old bec.logbook/client.logbook.send_logbook_message() API had no such
|
||||
# check -- on a session without scilog credentials configured (e.g. no
|
||||
# /etc/bec/secrets/.scilog.env at all) its send() call blocked indefinitely
|
||||
# instead of failing fast, hanging a real tomo_scan() inside
|
||||
# _write_subtomo_to_scilog() (confirmed live). Mirrors flomni's
|
||||
# _scilog_write(), which gates on bec.messaging.scilog._enabled instead.
|
||||
|
||||
|
||||
class _FakeSciLogMessage:
|
||||
def __init__(self, calls):
|
||||
self._calls = calls
|
||||
|
||||
def add_text(self, text):
|
||||
self._calls.append(("add_text", text))
|
||||
return self
|
||||
|
||||
def add_tags(self, tags):
|
||||
self._calls.append(("add_tags", tags))
|
||||
return self
|
||||
|
||||
def send(self):
|
||||
self._calls.append(("send",))
|
||||
|
||||
|
||||
def _make_omny_with_scilog(monkeypatch, enabled: bool, raise_on_send: bool = False):
|
||||
omny = object.__new__(OMNY)
|
||||
calls = []
|
||||
|
||||
def _new():
|
||||
if raise_on_send:
|
||||
|
||||
class _Boom(_FakeSciLogMessage):
|
||||
def send(self):
|
||||
raise RuntimeError("scilog unreachable")
|
||||
|
||||
return _Boom(calls)
|
||||
return _FakeSciLogMessage(calls)
|
||||
|
||||
fake_scilog = types.SimpleNamespace(_enabled=enabled, new=_new)
|
||||
fake_bec = types.SimpleNamespace(messaging=types.SimpleNamespace(scilog=fake_scilog))
|
||||
monkeypatch.setattr(omny_module, "bec", fake_bec, raising=False)
|
||||
return omny, calls
|
||||
|
||||
|
||||
def test_write_to_scilog_skips_when_scilog_disabled(monkeypatch):
|
||||
omny, calls = _make_omny_with_scilog(monkeypatch, enabled=False)
|
||||
|
||||
result = omny.write_to_scilog("hello")
|
||||
|
||||
assert result is False
|
||||
assert calls == []
|
||||
|
||||
|
||||
def test_write_to_scilog_sends_when_scilog_enabled(monkeypatch):
|
||||
omny, calls = _make_omny_with_scilog(monkeypatch, enabled=True)
|
||||
|
||||
result = omny.write_to_scilog("hello", tags=["mytag"])
|
||||
|
||||
assert result is True
|
||||
assert ("add_text", "hello") in calls
|
||||
assert ("add_tags", ["mytag", "BEC"]) in calls
|
||||
assert ("send",) in calls
|
||||
|
||||
|
||||
def test_write_to_scilog_tolerates_send_exception(monkeypatch):
|
||||
omny, calls = _make_omny_with_scilog(monkeypatch, enabled=True, raise_on_send=True)
|
||||
|
||||
result = omny.write_to_scilog("hello")
|
||||
|
||||
assert result is False
|
||||
Reference in new issue
Block a user