Fix six quality findings from code review
sitecustomize.py:
- Restore _record_status count barrier: write the status file only after all
patches have reported. middlewared.plugins.cloud_backup.restic is imported
lazily (only when a backup task runs), so without this barrier verify would
declare "all patches active" based solely on the B2 patch that fires at
startup. Barrier now gates on _Finder._targets rather than the removed
_PATCHES dict.
- Add comment in exec_module noting the if/elif must stay in sync with
_Finder._targets, making the coupling visible.
patch_ui.py:
- Merge find_webui into find_bundle: previously find_bundle(None) would crash
with os.walk(None) if main()'s guard were removed. Merged function returns
a 3-tuple (webui_dir, path, content); webui_dir=None means no candidate
directory found, path=None means directory found but pattern absent.
main() still produces distinct messages for each failure mode.
create_task.py:
- Split triple-chained .get() in cmd_list_tasks into two lines; the or {}
handling for None credentials was buried inside a one-liner.
uninstall.sh:
- Fix find loop: replace "for x in $(find ...)" with "while IFS= read -r"
to handle paths containing spaces or newlines.
- Add #!/usr/bin/env shebang form to Python detection, matching apply.sh.
Without this, uninstall on a system where middlewared uses the env form
would silently leave sitecustomize.py in the wrong site-packages.
This commit is contained in:
@@ -139,7 +139,8 @@ def cmd_list_tasks(client, _args):
|
|||||||
print(f"{'ID':>4} {'Enabled':<8} {'Provider':<14} Name")
|
print(f"{'ID':>4} {'Enabled':<8} {'Provider':<14} Name")
|
||||||
print("─" * 60)
|
print("─" * 60)
|
||||||
for t in sorted(tasks, key=lambda x: x["id"]):
|
for t in sorted(tasks, key=lambda x: x["id"]):
|
||||||
ptype = (t.get("credentials") or {}).get("provider", {}).get("type", "?")
|
creds = t.get("credentials") or {}
|
||||||
|
ptype = creds.get("provider", {}).get("type", "?")
|
||||||
enabled = "yes" if t.get("enabled") else "no"
|
enabled = "yes" if t.get("enabled") else "no"
|
||||||
print(f"{t['id']:>4} {enabled:<8} {ptype:<14} {t.get('description', '')}")
|
print(f"{t['id']:>4} {enabled:<8} {ptype:<14} {t.get('description', '')}")
|
||||||
|
|
||||||
|
|||||||
+14
-16
@@ -40,19 +40,17 @@ REPLACE = r'\1["STORJ_IX","S3","B2"]'
|
|||||||
MARKER = '"STORJ_IX","S3","B2"'
|
MARKER = '"STORJ_IX","S3","B2"'
|
||||||
|
|
||||||
|
|
||||||
def find_webui():
|
def find_bundle():
|
||||||
for d in WEBUI_CANDIDATES:
|
|
||||||
if os.path.isdir(d):
|
|
||||||
return d
|
|
||||||
return None
|
|
||||||
|
|
||||||
|
|
||||||
def find_bundle(webui):
|
|
||||||
"""
|
"""
|
||||||
Walk the webui directory looking for the JS file that contains the
|
Search WEBUI_CANDIDATES for the JS bundle containing the filterByProviders
|
||||||
filterByProviders binding. Returns (path, content) or (None, None).
|
binding. Returns (webui_dir, path, content). webui_dir is None if no
|
||||||
Only .js files are read; binary files and permission errors are skipped.
|
candidate directory exists; path is None if the directory exists but the
|
||||||
|
pattern is not found in any bundle.
|
||||||
"""
|
"""
|
||||||
|
webui = next((d for d in WEBUI_CANDIDATES if os.path.isdir(d)), None)
|
||||||
|
if webui is None:
|
||||||
|
return None, None, None
|
||||||
|
|
||||||
matches = []
|
matches = []
|
||||||
for root, _dirs, names in os.walk(webui):
|
for root, _dirs, names in os.walk(webui):
|
||||||
for name in sorted(names): # deterministic order
|
for name in sorted(names): # deterministic order
|
||||||
@@ -68,7 +66,7 @@ def find_bundle(webui):
|
|||||||
continue
|
continue
|
||||||
|
|
||||||
if not matches:
|
if not matches:
|
||||||
return None, None
|
return webui, None, None
|
||||||
|
|
||||||
if len(matches) > 1:
|
if len(matches) > 1:
|
||||||
# Unexpected — log all matches so the operator can investigate.
|
# Unexpected — log all matches so the operator can investigate.
|
||||||
@@ -79,19 +77,19 @@ def find_bundle(webui):
|
|||||||
for p, _ in matches:
|
for p, _ in matches:
|
||||||
print(f"[truecloud-patch] {p}")
|
print(f"[truecloud-patch] {p}")
|
||||||
|
|
||||||
return matches[0]
|
path, content = matches[0]
|
||||||
|
return webui, path, content
|
||||||
|
|
||||||
|
|
||||||
def main():
|
def main():
|
||||||
webui = find_webui()
|
webui, path, content = find_bundle()
|
||||||
if not webui:
|
if webui is None:
|
||||||
print(
|
print(
|
||||||
"[truecloud-patch] WARNING: webui directory not found; skipping UI patch.\n"
|
"[truecloud-patch] WARNING: webui directory not found; skipping UI patch.\n"
|
||||||
"[truecloud-patch] Searched: " + ", ".join(WEBUI_CANDIDATES)
|
"[truecloud-patch] Searched: " + ", ".join(WEBUI_CANDIDATES)
|
||||||
)
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
path, content = find_bundle(webui)
|
|
||||||
if path is None:
|
if path is None:
|
||||||
print(
|
print(
|
||||||
"[truecloud-patch] WARNING: filterByProviders pattern not found in any JS bundle.\n"
|
"[truecloud-patch] WARNING: filterByProviders pattern not found in any JS bundle.\n"
|
||||||
|
|||||||
@@ -105,6 +105,7 @@ class _Loader:
|
|||||||
self._finder._mark_done(fullname)
|
self._finder._mark_done(fullname)
|
||||||
|
|
||||||
# exec_module succeeded — apply our patch.
|
# exec_module succeeded — apply our patch.
|
||||||
|
# Must stay in sync with _Finder._targets.
|
||||||
try:
|
try:
|
||||||
if fullname == "middlewared.rclone.remote.b2":
|
if fullname == "middlewared.rclone.remote.b2":
|
||||||
_patch_b2(module)
|
_patch_b2(module)
|
||||||
@@ -187,6 +188,8 @@ def _record_status(fullname: str, ok: bool, detail: str = "") -> None:
|
|||||||
if fullname in _hook_status:
|
if fullname in _hook_status:
|
||||||
return # idempotent: first call wins
|
return # idempotent: first call wins
|
||||||
_hook_status[fullname] = {"ok": ok, "detail": detail}
|
_hook_status[fullname] = {"ok": ok, "detail": detail}
|
||||||
|
if len(_hook_status) < len(_Finder._targets):
|
||||||
|
return # wait until all patches have reported before writing
|
||||||
|
|
||||||
payload = {
|
payload = {
|
||||||
"patched_at": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()),
|
"patched_at": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()),
|
||||||
|
|||||||
+5
-3
@@ -53,6 +53,8 @@ if [ -x /usr/bin/middlewared ]; then
|
|||||||
shebang=$(dd if=/usr/bin/middlewared bs=256 count=1 2>/dev/null | head -1 || true)
|
shebang=$(dd if=/usr/bin/middlewared bs=256 count=1 2>/dev/null | head -1 || true)
|
||||||
if [[ "$shebang" =~ ^'#!'(/[^[:space:]]+python[^[:space:]]*) ]]; then
|
if [[ "$shebang" =~ ^'#!'(/[^[:space:]]+python[^[:space:]]*) ]]; then
|
||||||
PYTHON="${BASH_REMATCH[1]}"
|
PYTHON="${BASH_REMATCH[1]}"
|
||||||
|
elif [[ "$shebang" =~ ^'#!/usr/bin/env '(python[^[:space:]]*) ]]; then
|
||||||
|
PYTHON=$(command -v "${BASH_REMATCH[1]}" 2>/dev/null || echo "python3")
|
||||||
fi
|
fi
|
||||||
fi
|
fi
|
||||||
|
|
||||||
@@ -81,13 +83,13 @@ echo ""
|
|||||||
echo "Restoring UI bundle backup ..."
|
echo "Restoring UI bundle backup ..."
|
||||||
|
|
||||||
RESTORED=0
|
RESTORED=0
|
||||||
for backup in $(find /usr/share/truenas /var/www/truenas \
|
while IFS= read -r backup; do
|
||||||
-name "*.js.pre-truecloud-patch" 2>/dev/null); do
|
|
||||||
original="${backup%.pre-truecloud-patch}"
|
original="${backup%.pre-truecloud-patch}"
|
||||||
mv "$backup" "$original"
|
mv "$backup" "$original"
|
||||||
echo " Restored: $original"
|
echo " Restored: $original"
|
||||||
RESTORED=1
|
RESTORED=1
|
||||||
done
|
done < <(find /usr/share/truenas /var/www/truenas \
|
||||||
|
-name "*.js.pre-truecloud-patch" 2>/dev/null)
|
||||||
|
|
||||||
if [ "$RESTORED" -eq 0 ]; then
|
if [ "$RESTORED" -eq 0 ]; then
|
||||||
echo " No backup files found."
|
echo " No backup files found."
|
||||||
|
|||||||
Reference in New Issue
Block a user