Skip to content

Always forward Gator warnings/errors to user regardless of --all-msg flag #27

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
EdNutting wants to merge 1 commit into from
Closed

Always forward Gator warnings/errors to user regardless of --all-msg flag #27

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
5 changes: 4 additions & 1 deletion gator/common/logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,11 @@ async def log(
# Generate a timestamp if required
if timestamp is None:
timestamp = datetime.now()
# Always forward warnings, errors, and critical messages from Gator itself
# (not forwarded from children), regardless of forward setting
should_forward = forward or (not forwarded and severity >= LogSeverity.WARNING)
Copy link
Owner

@Intuity Intuity Jan 15, 2026

Choose a reason for hiding this comment

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

If I've not missed something, won't this only forward gator messages from the first child layer - anything below that would have the forwarded flag set and be lost?

I suspect there would need to be some special marker on messages raising up the stack indicating they were coming from gator's own channel, so that they're always forwarded.

Copy link
Contributor Author

@EdNutting EdNutting Jan 15, 2026

Choose a reason for hiding this comment

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

Ugh yeah...I forgot Gator invokes itself in infinite nesting. I need to ask you about that architecture as I'm not sure I fully get why it needs to do nested invocation rather than just using one Python instance to recurse down the tree of jobs / manage invocations accordingly.

Copy link
Contributor Author

@EdNutting EdNutting Jan 15, 2026

Choose a reason for hiding this comment

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

Maybe I still don't quite get what it's doing - local scheduler invokes itself for nested job groups/arrays (I think). Slurm I'm not quite sure if it's dispatching JobArray/JobGroup to a node of Slurm to be executed or just aggregating locally then dispacting the end Jobs to Slurm??

# If linked to parent and forwarding requested, push log upwards
if forward and self.ws_cli.linked and severity >= self.verbosity:
if should_forward and self.ws_cli.linked and severity >= self.verbosity:
await self.ws_cli.log(
timestamp=int(timestamp.timestamp()),
hierarchy=hierarchy,
Expand Down
71 changes: 71 additions & 0 deletions tests/common/test_logger.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,77 @@ async def test_local(self, logger_local):
)
logger._Logger__console.log.reset_mock()

@pytest.mark.asyncio
async def test_linked_no_forward(self, logger_linked):
"""When forward=False, warnings and errors are still forwarded"""
logger = logger_linked
logger.forward = False
logger.verbosity = LogSeverity.DEBUG
# Debug should not be forwarded
await logger.debug("Testing debug")
assert not logger.ws_cli.log.called
logger._Logger__console.log.assert_called_with(
"[bold cyan]DEBUG [/bold cyan] " + TestLogger.ROOT_STR + " Testing debug"
)
logger._Logger__console.log.reset_mock()
# Info should not be forwarded
await logger.info("Testing info")
assert not logger.ws_cli.log.called
logger._Logger__console.log.assert_called_with(
"[bold]INFO [/bold] " + TestLogger.ROOT_STR + " Testing info"
)
logger._Logger__console.log.reset_mock()
# Warning should be forwarded (even though forward=False)
await logger.warning("Testing warning")
logger.ws_cli.log.assert_called_with(
timestamp=1234,
hierarchy="root",
severity="WARNING",
message="Testing warning",
posted=True,
)
logger._Logger__console.log.assert_called_with(
"[bold yellow]WARNING[/bold yellow] " + TestLogger.ROOT_STR + " Testing warning"
)
logger.ws_cli.log.reset_mock()
logger._Logger__console.log.reset_mock()
# Error should be forwarded (even though forward=False)
await logger.error("Testing error")
logger.ws_cli.log.assert_called_with(
timestamp=1234,
hierarchy="root",
severity="ERROR",
message="Testing error",
posted=True,
)
logger._Logger__console.log.assert_called_with(
"[bold red]ERROR [/bold red] " + TestLogger.ROOT_STR + " Testing error"
)
logger.ws_cli.log.reset_mock()
logger._Logger__console.log.reset_mock()
# Critical should be forwarded (even though forward=False)
await logger.critical("Testing critical")
logger.ws_cli.log.assert_called_with(
timestamp=1234,
hierarchy="root",
severity="CRITICAL",
message="Testing critical",
posted=True,
)
logger._Logger__console.log.assert_called_with(
"[bold white on red]CRITICAL[/bold white on red] "
+ TestLogger.ROOT_STR
+ " Testing critical"
)
logger.ws_cli.log.reset_mock()
logger._Logger__console.log.reset_mock()
# Forwarded messages should only be forwarded when forward=True
await logger.warning("Forwarded warning", forwarded=True)
assert not logger.ws_cli.log.called
logger._Logger__console.log.assert_called_with(
"[bold yellow]WARNING[/bold yellow] " + TestLogger.ROOT_STR + " Forwarded warning"
)

@pytest.mark.asyncio
async def test_linked(self, logger_linked):
"""Local logging goes to the console"""
Expand Down
Loading