Skip to content
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.

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

feat: allow custom exit function through dependency injection #389

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 11 additions & 7 deletions src/apify/_actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,17 +231,19 @@ async def exit(
event_listeners_timeout: timedelta | None = EVENT_LISTENERS_TIMEOUT,
status_message: str | None = None,
cleanup_timeout: timedelta = timedelta(seconds=30),
sys_exit: Callable[[int], None] = sys.exit,
) -> None:
"""Exit the Actor instance.

This stops the Actor instance. It cancels all the intervals for regularly sending `PERSIST_STATE` events,
sends a final `PERSIST_STATE` event, waits for all the event listeners to finish, and stops the event manager.

Args:
exit_code: The exit code with which the Actor should fail (defaults to `0`).
exit_code: The exit code with which the program should end (defaults to `0`).
event_listeners_timeout: How long should the Actor wait for Actor event listeners to finish before exiting.
status_message: The final status message that the Actor should display.
cleanup_timeout: How long we should wait for event listeners.
sys_exit: A function to use to terminate the program (defaults to `sys.exit`)
"""
self._raise_if_not_initialized()

Expand All @@ -267,29 +269,31 @@ async def finalize() -> None:
self._is_initialized = False

if is_running_in_ipython():
self.log.debug(f'Not calling sys.exit({exit_code}) because Actor is running in IPython')
self.log.debug(f'Not calling sys_exit({exit_code}) because Actor is running in IPython')
elif os.getenv('PYTEST_CURRENT_TEST', default=False): # noqa: PLW1508
self.log.debug(f'Not calling sys.exit({exit_code}) because Actor is running in an unit test')
self.log.debug(f'Not calling sys_exit({exit_code}) because Actor is running in an unit test')
elif hasattr(asyncio, '_nest_patched'):
self.log.debug(f'Not calling sys.exit({exit_code}) because Actor is running in a nested event loop')
self.log.debug(f'Not calling sys_exit({exit_code}) because Actor is running in a nested event loop')
else:
sys.exit(exit_code)
sys_exit(exit_code)
Comment on lines 271 to +278
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this logic could be simplified a fair bit.

  • we don't need to check if we're running under pytest - we can instead pass in a mock sys_exit
  • didn't @vdusek manage to remove the nesting?
  • we probably still have to check for ipython 😕

Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding the nest-asyncio ...

We managed to resolve the problem of executing the Actor & Spider together. Here is more context and the core result structure - scrapy/scrapy#6639 (reply in thread).

However, there's still one remaining challenge - executing asyncio coroutines from the synchronous Scheduler (and HTTP cache storage) code. This is in-progress.

Once that's addressed, I'll open a new PR removing nest-asyncio completely.


async def fail(
self,
*,
exit_code: int = 1,
exception: BaseException | None = None,
status_message: str | None = None,
sys_exit: Callable = sys.exit,
) -> None:
"""Fail the Actor instance.

This performs all the same steps as Actor.exit(), but it additionally sets the exit code to `1` (by default).

Args:
exit_code: The exit code with which the Actor should fail (defaults to `1`).
exit_code: The exit code with which the program should fail (defaults to `1`).
exception: The exception with which the Actor failed.
status_message: The final status message that the Actor should display.
sys_exit: A function to use to terminate the program (defaults to `sys.exit`)
"""
self._raise_if_not_initialized()

Expand All @@ -298,7 +302,7 @@ async def fail(
if exception and not is_running_in_ipython():
self.log.exception('Actor failed with an exception', exc_info=exception)

await self.exit(exit_code=exit_code, status_message=status_message)
await self.exit(exit_code=exit_code, status_message=status_message, sys_exit=sys_exit)

def new_client(
self,
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/actor/test_actor_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,4 @@ async def test_actor_logs_messages_correctly(
assert caplog.records[10].message == 'Exiting Actor'

assert caplog.records[11].levelno == logging.DEBUG
assert caplog.records[11].message == 'Not calling sys.exit(91) because Actor is running in an unit test'
assert caplog.records[11].message == 'Not calling sys_exit(91) because Actor is running in an unit test'
Loading