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

fix: Add missing type annotations in utils/ directory #6687

Open
wants to merge 3 commits into
base: main
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
20 changes: 17 additions & 3 deletions openhands/utils/http_session.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
from dataclasses import dataclass, field
from typing import Any, cast

import requests
from requests.structures import CaseInsensitiveDict

from openhands.core.logger import openhands_logger as logger

Expand All @@ -15,13 +17,25 @@ class HttpSession:

session: requests.Session | None = field(default_factory=requests.Session)

def __getattr__(self, name):
def __getattr__(self, name: str) -> Any:
if self.session is None:
logger.error(
'Session is being used after close!', stack_info=True, exc_info=True
)
return object.__getattribute__(self.session, name)
raise RuntimeError('Session is being used after close!')
return getattr(self.session, name)

def close(self):
@property
def headers(self) -> CaseInsensitiveDict[str]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this is necessary because it is used, e.g. in this file:
openhands/runtime/builder/remote.py

if self.session is None:
logger.error(
'Session is being used after close!', stack_info=True, exc_info=True
)
raise RuntimeError('Session is being used after close!')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like we had this code before, and it was causing errors in eval, but I could be wrong. It's reasonable to require session, and I don't see why it would be None...

# Cast to CaseInsensitiveDict[str] since mypy doesn't know the exact type
return cast(CaseInsensitiveDict[str], self.session.headers)

def close(self) -> None:
if self.session is not None:
self.session.close()
self.session = None
4 changes: 2 additions & 2 deletions openhands/utils/prompt.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def __init__(
if name not in self.disabled_microagents:
self.repo_microagents[name] = microagent

def load_microagents(self, microagents: list[BaseMicroAgent]):
def load_microagents(self, microagents: list[BaseMicroAgent]) -> None:
"""Load microagents from a list of BaseMicroAgents.

This is typically used when loading microagents from inside a repo.
Expand All @@ -135,7 +135,7 @@ def _load_template(self, template_name: str) -> Template:
def get_system_message(self) -> str:
return self.system_template.render().strip()

def set_runtime_info(self, runtime: Runtime):
def set_runtime_info(self, runtime: Runtime) -> None:
self.runtime_info.available_hosts = runtime.web_hosts

def set_repository_info(
Expand Down
10 changes: 5 additions & 5 deletions openhands/utils/shutdown_listener.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@
_shutdown_listeners: dict[UUID, Callable] = {}


def _register_signal_handler(sig: signal.Signals):
def _register_signal_handler(sig: signal.Signals) -> None:
original_handler = None

def handler(sig_: int, frame: FrameType | None):
def handler(sig_: int, frame: FrameType | None) -> None:
logger.debug(f'shutdown_signal:{sig_}')
global _should_exit
if not _should_exit:
Expand All @@ -39,7 +39,7 @@ def handler(sig_: int, frame: FrameType | None):
original_handler = signal.signal(sig, handler)


def _register_signal_handlers():
def _register_signal_handlers() -> None:
global _should_exit
if _should_exit is not None:
return
Expand All @@ -66,7 +66,7 @@ def should_continue() -> bool:
return not _should_exit


def sleep_if_should_continue(timeout: float):
def sleep_if_should_continue(timeout: float) -> None:
if timeout <= 1:
time.sleep(timeout)
return
Expand All @@ -75,7 +75,7 @@ def sleep_if_should_continue(timeout: float):
time.sleep(1)


async def async_sleep_if_should_continue(timeout: float):
async def async_sleep_if_should_continue(timeout: float) -> None:
if timeout <= 1:
await asyncio.sleep(timeout)
return
Expand Down
2 changes: 1 addition & 1 deletion openhands/utils/tenacity_stop.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,4 @@ class stop_if_should_exit(stop_base):
"""Stop if the should_exit flag is set."""

def __call__(self, retry_state: 'RetryCallState') -> bool:
return should_exit()
return bool(should_exit())
Loading