Skip to content

Move JsonDict import to type-checking path only #19876

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

Open
anoadragon453 wants to merge 2 commits into
base: develop
Choose a base branch
from
+3 −4
Open

Move JsonDict import to type-checking path only #19876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b01219
Move `JsonDict` import to type-checking path only
anoadragon453 Jun 24, 2026
e09c82d
newsfile
anoadragon453 Jun 24, 2026
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
1 change: 1 addition & 0 deletions changelog.d/19876.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Prevent an obtuse error masking a more user-friendly error when trying to run Synapse with an outdated rust module in certain cases.

@erikjohnston erikjohnston Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hmm, this won't help for future cases though. The root cause is that we started importing stuff from Rust before we checked if the Rust was up to date. This is because check_rust_lib_up_to_date lives in synapse.util.rust, but we import Rust stuff in synapse.util.

I think we should move check_rust_lib_up_to_date into a top-level module and make sure that it is imported/called before anything else is imported (other than maybe the Python version check). I'm tempted to have something like synapse.startup_checks that checks the appropriate things on import with no/minimal imports from other synapse modules.

Thoughts?

@erikjohnston erikjohnston Jun 24, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(FTR you can stop an import line being resorted by adding a # isort: skip comment after it)

@anoadragon453 anoadragon453 Jun 24, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

That does sound more correct, and future-proof. I'll give it a shot!

6 changes: 2 additions & 4 deletions synapse/util/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,8 @@
from twisted.internet import defer
from twisted.python.failure import Failure

from synapse.types import JsonDict

if typing.TYPE_CHECKING:
pass
from synapse.types import JsonDict

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -189,7 +187,7 @@ def split_dict_to_fit_to_size(
*,
soft_max_size: int,
wrapping_object_size: int = 2,
) -> Iterator[tuple[dict[str, JsonDict], int]]:
) -> Iterator[tuple[dict[str, "JsonDict"], int]]:
"""Splits a dict up into a list of dicts, each of which is small enough to
fit into the given size when encoded as JSON. Every entry in the original
dict is in exactly one of the resulting dicts.
Expand Down
Loading