mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-12 14:14:50 +00:00
fix: docsgpt up review follow-ups
- wait_healthy starts no request once the deadline is reached, and neither its pauses nor the requests after the first run past it; the first attempt still always runs (status uses a zero timeout). - envfile writes $ as $$ inside double quotes, which Compose interpolates, and reads $$ back as $, so a value such as pa$w'rd reaches the container unchanged. - Upgrading no longer describes the working directory as the data home.
This commit is contained in:
1 parent
5e699b0168
commit
f6bf4fefd1
5 files changed
+64
-10
No files matched your search
@@ -121,8 +121,9 @@ alias, so nothing breaks on upgrade, but update these before the alias goes:
|
||||
and vectors under `application/` in the checkout, where they already are.
|
||||
- The backend is also a package now (`pip install docsgpt`, see
|
||||
[Install with pip](/Deploying/Pip-Install)). Runtime data lives in a data
|
||||
home: `DOCSGPT_HOME`, else the checkout, else the directory the process
|
||||
starts in. One consequence for a source checkout: the embedded Milvus
|
||||
home: `DOCSGPT_HOME`, else the checkout, else `~/.docsgpt/server`
|
||||
(`/opt/docsgpt` for root on Linux; see
|
||||
[pip installs: data home moved](#pip-installs-data-home-moved)). One consequence for a source checkout: the embedded Milvus
|
||||
(`MILVUS_URI`) and LanceDB (`LANCEDB_PATH`) default paths now resolve under
|
||||
the checkout instead of the start directory. If you use either store at its
|
||||
default path and start DocsGPT from another directory, the old data is at
|
||||
|
||||
@@ -125,18 +125,28 @@ def wait_healthy(
|
||||
sleep: Callable[[float], None] = time.sleep,
|
||||
clock: Callable[[], float] = time.monotonic,
|
||||
) -> bool:
|
||||
"""Poll ``url`` until it answers 2xx (True) or ``timeout`` seconds pass (False); tries at least once."""
|
||||
"""Poll ``url`` until it answers 2xx (True) or ``timeout`` seconds pass (False); tries at least once.
|
||||
|
||||
No request starts once the deadline is reached, and neither the pauses nor the
|
||||
requests after the first run past it.
|
||||
"""
|
||||
deadline = clock() + timeout
|
||||
request_timeout = min(5.0, timeout) if timeout > 0 else 5.0
|
||||
while True:
|
||||
try:
|
||||
with opener(url, timeout=5) as response:
|
||||
with opener(url, timeout=request_timeout) as response:
|
||||
if 200 <= response.status < 300:
|
||||
return True
|
||||
except (OSError, http.client.HTTPException):
|
||||
pass
|
||||
if clock() >= deadline:
|
||||
remaining = deadline - clock()
|
||||
if remaining <= 0:
|
||||
return False
|
||||
sleep(2)
|
||||
sleep(min(2.0, remaining))
|
||||
remaining = deadline - clock()
|
||||
if remaining <= 0:
|
||||
return False
|
||||
request_timeout = min(5.0, remaining)
|
||||
|
||||
|
||||
def lan_ip() -> str:
|
||||
|
||||
@@ -19,19 +19,21 @@ def _parse_value(raw: str) -> str:
|
||||
if len(value) >= 2 and value[0] == value[-1] == "'":
|
||||
return value[1:-1]
|
||||
if len(value) >= 2 and value[0] == value[-1] == '"':
|
||||
return re.sub(r'\\(["\\])', r"\1", value[1:-1])
|
||||
return re.sub(r'\\(["\\])', r"\1", value[1:-1]).replace("$$", "$")
|
||||
return value.split(" #", 1)[0].rstrip()
|
||||
|
||||
|
||||
def _format_value(value: str) -> str:
|
||||
"""``value`` quoted so it reads back unchanged."""
|
||||
"""``value`` quoted so it reads back unchanged, and so Compose passes it on unchanged."""
|
||||
if "\n" in value or "\r" in value:
|
||||
raise ValueError("a .env value cannot contain a newline")
|
||||
if _PLAIN.match(value):
|
||||
return value
|
||||
if "'" not in value:
|
||||
# Compose does not interpolate single-quoted values.
|
||||
return f"'{value}'"
|
||||
return '"' + value.replace("\\", "\\\\").replace('"', '\\"') + '"'
|
||||
# Double quotes are interpolated: $$ is Compose's literal dollar.
|
||||
return '"' + value.replace("\\", "\\\\").replace('"', '\\"').replace("$", "$$") + '"'
|
||||
|
||||
|
||||
def read(path: Path) -> dict[str, str]:
|
||||
|
||||
@@ -120,6 +120,38 @@ class TestWaitHealthy:
|
||||
sleep=lambda s: None, clock=lambda: next(clock))
|
||||
assert attempts["n"] == 3
|
||||
|
||||
def test_no_request_starts_at_or_after_the_deadline(self):
|
||||
"""A short timeout must not be overrun by a sleep and one more five-second request."""
|
||||
now = {"t": 0.0}
|
||||
starts = []
|
||||
|
||||
def opener(url, timeout):
|
||||
starts.append((now["t"], timeout))
|
||||
now["t"] += 1
|
||||
raise OSError("connection refused")
|
||||
|
||||
def sleep(seconds):
|
||||
now["t"] += seconds
|
||||
|
||||
assert not docker_module.wait_healthy("http://127.0.0.1:7091/api/health", 3, opener=opener,
|
||||
sleep=sleep, clock=lambda: now["t"])
|
||||
assert starts, "the first attempt always runs"
|
||||
for started, timeout in starts[1:]:
|
||||
assert started < 3
|
||||
assert started + timeout <= 3
|
||||
assert now["t"] <= 3
|
||||
|
||||
def test_a_zero_timeout_still_tries_once(self):
|
||||
calls = []
|
||||
|
||||
def opener(url, timeout):
|
||||
calls.append(timeout)
|
||||
return _Response(200)
|
||||
|
||||
assert docker_module.wait_healthy("http://127.0.0.1:7091/api/health", 0, opener=opener,
|
||||
sleep=lambda s: None, clock=lambda: 0.0)
|
||||
assert len(calls) == 1 and calls[0] > 0
|
||||
|
||||
def test_gives_up_after_the_timeout(self):
|
||||
def opener(url, timeout):
|
||||
raise OSError("connection refused")
|
||||
|
||||
@@ -55,12 +55,21 @@ class TestUpdate:
|
||||
envfile.update(path, {"A": "3"})
|
||||
assert path.read_text() == "A=3\nX=y\n"
|
||||
|
||||
@pytest.mark.parametrize("value", ["plain", "with space", "hash # inside", "it's", 'say "hi"', "back\\slash", ""])
|
||||
@pytest.mark.parametrize(
|
||||
"value",
|
||||
["plain", "with space", "hash # inside", "it's", 'say "hi"', "back\\slash", "", "pa$w'rd", "a$$b'c", "$HOME"],
|
||||
)
|
||||
def test_values_round_trip(self, tmp_path, value):
|
||||
path = tmp_path / ".env"
|
||||
envfile.update(path, {"VALUE": value})
|
||||
assert envfile.read(path)["VALUE"] == value
|
||||
|
||||
def test_a_dollar_in_double_quotes_is_escaped_for_compose(self, tmp_path):
|
||||
"""Compose interpolates double-quoted .env values; $$ is its literal dollar."""
|
||||
path = tmp_path / ".env"
|
||||
envfile.update(path, {"API_KEY": "pa$w'rd"})
|
||||
assert path.read_text() == "API_KEY=\"pa$$w'rd\"\n"
|
||||
|
||||
@pytest.mark.skipif(sys.platform == "win32", reason="POSIX permissions")
|
||||
def test_a_new_file_is_private(self, tmp_path):
|
||||
path = tmp_path / "stack" / ".env"
|
||||
|
||||
Reference in new issue
Block a user