fix(sdk): remove five dead wire forms, register TXN/FT.AGGREGATE, guard the SDK in CI #501
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,49 @@ | |
| await async_client.vector.search("my_idx", [0.1, 0.2, ...]) | ||
| """ | ||
|
|
||
| def _resolve_version() -> str: | ||
| """The version this package was published as. | ||
|
|
||
| Derived, never restated. A hand-maintained literal here drifted once | ||
| already — the package shipped as 0.1.1 while this module kept answering | ||
| "0.1.0" — and nothing could catch it, because a literal that is merely | ||
| stale is still syntactically perfect. | ||
|
|
||
| Installed (the normal case, including wheels): the installer wrote | ||
| `pyproject.toml`'s version into distribution metadata, so that IS the | ||
| published number. | ||
|
|
||
| Not installed (a source checkout run in place, e.g. `pytest` from | ||
| `sdk/python`): fall back to reading `pyproject.toml` itself, the same | ||
| source of truth the installer would have used. | ||
| """ | ||
| try: | ||
| from importlib.metadata import version | ||
|
|
||
| return version("moondb") | ||
| except Exception: # noqa: BLE001 - any metadata failure falls through | ||
| pass | ||
|
|
||
| try: | ||
| import pathlib | ||
| import sys | ||
|
|
||
| if sys.version_info >= (3, 11): | ||
| import tomllib | ||
| else: # pragma: no cover - Python 3.10 and older | ||
| import tomli as tomllib | ||
|
|
||
| pyproject = pathlib.Path(__file__).resolve().parent.parent / "pyproject.toml" | ||
| with pyproject.open("rb") as fh: | ||
| return str(tomllib.load(fh)["project"]["version"]) | ||
| except Exception: # noqa: BLE001 - neither source available | ||
| # Deliberately shaped like a version so callers that parse or compare | ||
| # it keep working, and deliberately 0.0.0 so nothing mistakes it for a | ||
| # real release. | ||
| return "0.0.0.unknown" | ||
|
Comment on lines
+54
to
+66
There was a problem hiding this comment. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,180p' sdk/python/pyproject.toml
rg -n -C 3 'requires-python|tomli|dependencies' sdk/python/pyproject.toml
rg -n -C 3 'import tomli|tomllib' sdk/python/moondb/__init__.py sdk/python/testsRepository: pilotspace/moon Length of output: 5762 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import ast
import re
root = Path("sdk/python")
pyproject = root / "pyproject.toml"
init_file = root / "moondb/__init__.py"
test_file = root / "tests/test_client.py"
text = pyproject.read_text()
init = init_file.read_text()
test = test_file.read_text()
requires_python = re.search(r'requires-python\s*=\s*"([^"]+)"', text).group(1)
dependencies = re.search(r'(?ms)^dependencies\s*=\s*\[(.*?)\]', text).group(1)
print(f"requires-python: {requires_python}")
print(f"tomli declared in runtime dependencies: {'tomli' in dependencies}")
tree = ast.parse(init)
imports = [
node.names[0].name
for node in ast.walk(tree)
if isinstance(node, ast.Import)
and node.names
and node.names[0].name in {"tomli", "tomllib"}
]
print(f"version-loader imports: {imports}")
print(f"fallback literal: {re.findall(r'return\\s+[\"\\']([^\"\\']+)[\"\\']', init)[-1]}")
print(f"test_client imports tomli: {'import tomli as tomllib' in test}")
PYRepository: pilotspace/moon Length of output: 434 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import ast
import re
root = Path("sdk/python")
pyproject = root / "pyproject.toml"
init_file = root / "moondb/__init__.py"
test_file = root / "tests/test_client.py"
project = pyproject.read_text()
init = init_file.read_text()
test = test_file.read_text()
requires_python = re.search(r'requires-python\s*=\s*"([^"]+)"', project).group(1)
dependencies = re.search(r'(?ms)^dependencies\s*=\s*\[(.*?)\]', project).group(1)
tree = ast.parse(init)
imports = [
alias.name
for node in ast.walk(tree)
if isinstance(node, ast.Import)
for alias in node.names
if alias.name in {"tomli", "tomllib"}
]
fallbacks = re.findall(r"""return\s+["']([^"']+)["']""", init)
print(f"requires-python: {requires_python}")
print(f"tomli declared in runtime dependencies: {'tomli' in dependencies}")
print(f"version-loader imports: {imports}")
print(f"string return values: {fallbacks}")
print(f"test_client imports tomli: {'import tomli as tomllib' in test}")
PYRepository: pilotspace/moon Length of output: 340 Declare the Python 3.10 TOML parser dependency.
🤖 Prompt for AI Agents
Comment on lines
+62
to
+66
There was a problem hiding this comment. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- sdk/python/moondb/__init__.py ---'
sed -n '1,110p' sdk/python/moondb/__init__.py
printf '%s\n' '--- sdk/python/tests/test_version.py ---'
sed -n '1,120p' sdk/python/tests/test_version.py
printf '%s\n' '--- version-related references ---'
rg -n --glob '*.py' '(__version__|0\.0\.0\.unknown|importlib\.metadata|Version\()' sdk/pythonRepository: pilotspace/moon Length of output: 8701 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from importlib.metadata import version
from packaging.version import Version, InvalidVersion
values = ["0.0.0.unknown", "0.0.0+unknown", "0.0.0.dev0", "0.0.0"]
for value in values:
try:
parsed = Version(value)
print(f"{value!r}: accepted as {parsed}, is_prerelease={parsed.is_prerelease}, local={parsed.local!r}")
except InvalidVersion as exc:
print(f"{value!r}: rejected: {exc}")
PYRepository: pilotspace/moon Length of output: 283 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
from packaging.version import Version, InvalidVersion
for value in ["0.0.0.unknown", "0.0.0+unknown", "0.0.0.dev0", "0.0.0"]:
try:
parsed = Version(value)
print(f"{value!r}: accepted as {parsed}")
except InvalidVersion as exc:
print(f"{value!r}: rejected: {exc}")
PYRepository: pilotspace/moon Length of output: 283 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- sdk/python/pyproject.toml ---'
sed -n '1,180p' sdk/python/pyproject.toml
printf '%s\n' '--- version-test references and fixtures ---'
rg -n -C 4 --glob '*.py' 'test_swf5c|_resolve_version|importlib\.metadata|monkeypatch|__version__' sdk/python/tests sdk/python/moondb
printf '%s\n' '--- regex and candidate fallback behavior ---'
python3 - <<'PY'
import re
pattern = re.compile(r"\d+\.\d+\.\d+([.-]?\w+)*")
for value in ("0.0.0.unknown", "0.0.0+unknown", "0.0.0.dev0", "0.0.0"):
print(f"{value!r}: custom_regex={bool(pattern.fullmatch(value))}")
PYRepository: pilotspace/moon Length of output: 13621 🌐 Web query:
💡 Result: The version string 0.0.0+unknown is a valid PEP 440 version [1][2][3]. According to PEP 440, a local version identifier is constructed by appending a plus sign (+) followed by a local version label to a public version identifier [4][5]. In the string 0.0.0+unknown: - 0.0.0 is the public version identifier. - + indicates the start of the local version identifier. - unknown is the local version label. The local version label "unknown" consists of ASCII letters, which is permitted under PEP 440's requirements for local version labels (which must consist of ASCII letters, digits, and periods, and must start and end with an ASCII letter or digit) [4][5][6]. Because 0.0.0+unknown conforms to these syntactic rules, it is a valid PEP 440 version string. It is commonly used by Python developers as a sentinel value in code to provide a fallback version when a package's metadata cannot be retrieved—for instance, when the package is imported from a source checkout that has not been installed [1][2][3]. While 0.0.0+unknown is a valid PEP 440 version, local version identifiers themselves are generally not permitted when publishing packages to public index servers like PyPI [4][7]. However, this restriction applies to the publication of distributions, not to the validity of the version string format itself or its use in local code environments [4][8]. Citations:
Use a PEP 440-compatible fallback and test the fallback path. 📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
|
|
||
| __version__: str = _resolve_version() | ||
|
|
||
| from .client import AsyncMoonClient, MoonClient | ||
| from .text import AsyncTextCommands, TextCommands | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -172,8 +172,30 @@ class TestVersionExported: | |
| """Test package metadata.""" | ||
|
|
||
| def test_version(self) -> None: | ||
| """`__version__` must be the version the package publishes as. | ||
|
|
||
| This assertion used to be `== "0.1.0"`, a second copy of the literal in | ||
| `moondb/__init__.py`. When the package was published as 0.1.1 the | ||
| literal was not updated, and because the test restated it rather than | ||
| deriving it, the suite stayed green while every caller reading | ||
| `__version__` was told the wrong release. Pinned to the packaging | ||
| source of truth instead; see `tests/test_version.py` for the guard on | ||
| the derivation itself. | ||
| """ | ||
| import pathlib | ||
| import sys | ||
|
|
||
| if sys.version_info >= (3, 11): | ||
| import tomllib | ||
| else: # pragma: no cover - Python 3.10 and older | ||
| import tomli as tomllib | ||
|
|
||
| import moondb | ||
|
|
||
| root = pathlib.Path(__file__).resolve().parent.parent | ||
| with (root / "pyproject.toml").open("rb") as fh: | ||
| published = tomllib.load(fh)["project"]["version"] | ||
| assert moondb.__version__ == published | ||
|
|
||
| def test_all_exports(self) -> None: | ||
| import moondb | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| """ADD task `sdk-wire-form-fixes` — GUARD 3: the version a caller reads is the | ||
| version that was published. | ||
|
|
||
| `moondb.__version__` was a hand-maintained string literal, and it drifted: the | ||
| package shipped to PyPI as 0.1.1 while `__version__` still answered "0.1.0". | ||
| Anything that keys on it — a bug report, a server-side compatibility check, a | ||
| user pinning a workaround to "SDK >= x" — was reading a number that had not | ||
| been true since the previous release. The test that covered it asserted the | ||
| same stale literal, so the suite stayed green through two releases. | ||
|
|
||
| Asserting the two are equal would only catch the drift AFTER it happened, and | ||
| would then be "fixed" by hand-editing the same literal that caused it. So the | ||
| fix is structural: `__version__` is derived from the installed distribution | ||
| metadata, which is `pyproject.toml`'s `version` by construction. These tests | ||
| guard the derivation, not a snapshot of the number. | ||
|
|
||
| Written as `unittest.TestCase` deliberately, so it needs NOTHING beyond the | ||
| standard library: the CI runner (Ubuntu 24.04 / Python 3.14) has no pytest, and | ||
| PEP 668 blocks `pip install --user` while `python3-venv` is not installed — | ||
| verified on the runner, not assumed. `unittest` collects this, and so does | ||
| pytest, so the same file serves CI and local development. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import pathlib | ||
| import re | ||
| import sys | ||
| import unittest | ||
|
|
||
| import moondb | ||
|
|
||
| if sys.version_info >= (3, 11): | ||
| import tomllib | ||
| else: # pragma: no cover - Python 3.10 and older | ||
| import tomli as tomllib | ||
|
|
||
|
|
||
| def _pyproject_version() -> str: | ||
| """The version as declared in the packaging source of truth.""" | ||
| root = pathlib.Path(__file__).resolve().parent.parent | ||
| with (root / "pyproject.toml").open("rb") as fh: | ||
| return str(tomllib.load(fh)["project"]["version"]) | ||
|
|
||
|
|
||
| class VersionDerivationTest(unittest.TestCase): | ||
| """`__version__` must equal what ships, by construction rather than by memory.""" | ||
|
|
||
| def test_swf5_version_matches_pyproject(self) -> None: | ||
| published = _pyproject_version() | ||
| self.assertEqual( | ||
| moondb.__version__, | ||
| published, | ||
| f"moondb.__version__ is {moondb.__version__!r} but the package " | ||
| f"publishes as {published!r} — a caller reading __version__ is " | ||
| f"being told the wrong release.", | ||
| ) | ||
|
|
||
| def test_swf5b_version_is_not_a_hardcoded_literal(self) -> None: | ||
| """The equality above must hold by construction, not by remembering. | ||
|
|
||
| A literal that happens to match today is exactly the state this package | ||
| was already in once, and it silently stopped being true. | ||
| """ | ||
| src = ( | ||
| pathlib.Path(moondb.__file__).read_text(encoding="utf-8") | ||
| if moondb.__file__ | ||
| else "" | ||
| ) | ||
| self.assertIsNone( | ||
| re.search(r'^__version__\s*(:\s*str\s*)?=\s*["\']', src, re.MULTILINE), | ||
| "__version__ is assigned a string literal in moondb/__init__.py. " | ||
| "Derive it from the installed distribution metadata " | ||
| "(importlib.metadata.version) so it cannot drift from pyproject.toml.", | ||
| ) | ||
|
|
||
| def test_swf5c_version_is_a_usable_release_string(self) -> None: | ||
| """Whatever the derivation returns must still look like a version. | ||
|
|
||
| Guards the fallback path: imported from a source tree that was never | ||
| installed, `importlib.metadata` raises, and a fallback returning `""` | ||
| or `"unknown"` would satisfy both tests above while handing callers | ||
| something useless. | ||
| """ | ||
| self.assertRegex( | ||
| moondb.__version__, | ||
| r"^\d+\.\d+\.\d+([.-]?\w+)*$", | ||
| f"moondb.__version__ is {moondb.__version__!r}, which is not a " | ||
| f"release string a caller can compare or report.", | ||
| ) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| unittest.main() |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| [package] | ||
| name = "moondb" | ||
| version = "0.3.0" | ||
| edition = "2024" | ||
| rust-version = "1.85" | ||
| description = "Rust client SDK for Moon — high-performance Redis-compatible server with vector search and graph engine" | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.