Restructure the project & public API clarification from code#1103
Restructure the project & public API clarification from code#1103
Conversation
fe07190 to
c9713f3
Compare
I think re-exports are very convenient, would keep it. Also, I don't think it is necessary to prefix all modules inside
I would prefer the second option, so you don't have to remember |
I have already introduced the docs for it in this PR, as it does also clarifies the public API more concretely, but I can move it out into a separate one if desired.
Sure, I agree
|
6c79f27 to
4befe5e
Compare
3732af7 to
d53012d
Compare
We most definitely should move that into a separate PR and release it in v12 |
d53012d to
2ec4186
Compare
PerchunPak
left a comment
There was a problem hiding this comment.
I like the idea of test_compat.py
f9de07f to
f33c86f
Compare
ffb435f to
fb91214
Compare
This refactors the mcstatus codebase to clearly separate public API,
private internals, and deprecated compatibility paths. Additionally, it
improves naming of various internals for better consistency and clarity,
and splits some larger files into more manageable modules.
Major structural changes:
- Introduced private internal namespaces:
- `_net` for networking and DNS utils
- `_protocol` for protocol clients and connections
- `_utils` for internal helpers
- Split the monolithic `responses.py` into a proper package. This
separates the raw response models (type-dicts) into a single
stand-alone module within this package, adds a module with shared base
classes, and then specific response modules.
- Renamed protocol-facing classes to reflect their actual role
(`ServerPinger` -> `JavaClient`, `BedrockServerStatus` ->
`BedrockClient`, etc.)
Compatibility and deprecation handling:
- Added `mcstatus/_compat` with explicit compatibility shims for
deprecated public modules.
- Compat files are injected at build time via hatch force-include
directive, keeping the deprecated paths out of the source tree for
cleaner file organization for contributors, while still providing the
same accessible deprecations to end users of mcstatus.
- Added ruff rules to prevent accidental internal imports from
`_compat`.
Behavioral and semantic adjustments:
- Raw response TypedDicts are now centralized under `responses._raw`
- MOTD transformers and simplifiers and no longer a part of the public
API and are explicitly treated as internal.
Tests, docs and toolings:
- Updated all imports and test paths to reflect the new structure
- Adjusted sphinx config and docs for moved internals
These deprecations are no longer relevant as the entire module was deprecated
fb91214 to
520f582
Compare
This PR performs a major restructure of the mcstatus codebase with the goal of making API boundaries explicit and producing an overall less cluttered, more structured code tree.
High-level changes overview
_prefixed namespaces (package, module, or object names), instead of living alongside public modules.responses.pyfile has been split into a proper package with shared base classes, per-response type modules and a separate_rawmodule for TypedDict definitions.ServerPinger->JavaClientLegacyServerStatus->LegacyClientServerQuerier->QueryClientBedrockServerStatus->BedrockClientCompatibility handling
Deprecated public modules are no longer present directly in the source tree. Instead, they live under an explicit compatibility shims package (
mcstatus/_compat) and are injected into the sdist at build time via Hatchlingforce-includesetting.This preserves deprecated import paths accessible for end users of the library while preventing internal code from accidentally depending on them, keeping these shims from cluttering the new source tree.
Additionally, Ruff configuration was added to explicitly forbid importing from
_compatinternally, helping prevent accidental usage (like via LSP auto-imports).Warning
If for some reason, someone relies on using mcstatus as a direct git submodule, this deprecation behavior will not function for them. I don't think we need to consider this as an issue, but I wanted to mention it.
This also moves the already deprecated
mcstatus/status_responses.pyunder_compatand explicitly deprecatesmcstatus/motd/transformers.pyon a module import level, not just with the existing the class usage deprecation messages.Breaking changes / Explicit public API boundaries
This PR attempts to establish a new code-style guideline for mcstatus where private internals live under
_prefixed packages or modules.If a package itself is
_prefixed, modules inside it are considered private by default and generally do not need to be_prefixed themselves. If a_prefixed module exists inside a_prefixed package, it should be considered internal to that package and should not be imported even by other mcstatus internals (except in tests).Additionally, this PR tightens the public contract where appropriate by introducing
__all__declarations. By default, non-_prefixed names are importable, but when__all__is defined, only explicitly exported objects are considered public. This avoids confusion around helper constants or typing helpers (for example variables likeT = TypeVar("T")or constants likeMINECRAFT_COLOR_TO_RGB_JAVA). This approach allows type checkers like basedpyright to warn users when importing non-public symbols.The changes this PR introduces in terms of public / private API separation were primarily based on what was or wasn't documented in the public API sphinx docs. That said, the documentation was actually missing some things that definitely should have been included. This PR fixes those instances too.
Specific, potentially breaking changes
TypedDictclasses) now live under an internal module name and are explicitly not public API. They were never documented as public, so no backwards compatibility is provided for these, however, they were previously importable directly frommcstatus.responses, which could have lead people to think these were public types. This PR will intentionally break these imports.mcstatus, tomcstatus._netpackage. Similarly, these were never intended to be public, this naming just makes that expectation clear. Again, no deprecations are provided here, people relying on these internals directly will face an immediate breakage.protocolpackage (originally only housing theprotocol/connection.pyfile) was renamed to_protocol, making it clear that this is an internal util._protocolpackage, with no deprecations provided for their original import paths.mcstatus.forge_data) were not previously explicitly documented in the docs as public. This is however very odd, as those types are useful to people that rely on type annotations (which is why all of the other response classes are public). For clean code organization purposes, this module was moved intomcstatus.responses.forge, and even though it wasn't documented to be public, compatibility shims for oldmcstatus.forge_dataimports will be provided.Follow-ups and open questions
Once merged, mcstatus should bump the major version to
v13(not immediately after). That said, before this PR can be merged, several questions need to be resolved:Should the re-exported response classes frommcstatus/responses/__init__.pyonly be considered as something there for deprecation purposes, meaning using them should emit deprecation warnings, guiding users towards importing from the specific public submodules inside of theresponsesnew package structure, or should they remain re-exported with the intention to stay re-exported? If they are to remain re-exported, should the forge response classes (which were never a part ofresponses.pyand rather had their own file (forge_data.py) be kept reachable only udner the newmcstatus.responses.forgenamespace, or should re-exports for it be included intomcstatus.responsesdirectly? Additionally, if explicit re-exports are desired, should the structure of the package be considered entirely internal, meaning all of the submodules inside ofmcstatus.responsesbecome_prefixed and not even meant for users to import directly?Is the approach with using__all__to separate out public vs private parts of modules sufficient, or should this PR simply add_prefixes for everything within the public modules that isn't supposed to be public?_prefix to as many internal variables in public moduels as possible, to avoid confusion from users. However, due to it's runtime behavior with star imports,__all__will remain defined everywhere (including where it previously wasn't)._prefix anywhere in the fully qualified path of the given symbol,__all__does not play a strict role as to whether or not something is public API for us, but anything that is public API should be present in__all__, and anything that isn't, shouldn't be.Review whether additional deprecated paths should gain explicit compat shims, even if they point to now private internals before their complete removal. This might still be worth it since our public API contract was very poorly established, even if we always consider some of these parts to be mcstatus internals.The main thing in question is whether the raw type-dict response types should gain deprecations.Though this does also apply to true internals that people could've assumed were public, e.g.Connection,Address, pinger/status classes, ...Merge notes
fixup!commits (github's rebase strategy doesn't perform--autosquash, needs to be done manually with a force-push once ready)