Fixing the URL for downloading the Vae/ETC for the remote instances - #407
Conversation
…slowly working on decoupling the KNX name from the repo - Commmited to a few more fixes but this was the biggest issue i made a break in on accident after moving the repo over.
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
PR Summary
|
ash-bluepollution95
added
the
Code Cleanup
📝 WalkthroughWalkthroughThe installer branding now uses Ecosystem terminology, and curated model synchronization targets the ChangesInstaller update
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@installer.py`:
- Line 3: Complete the Ecosystem rename across installer-facing output by
updating class and banner text, generated extra_model_paths.yaml metadata
values, and logger messages in the affected sections, including the messages
around lines 717 and 720. Rename the private _sync_knx_trainer_models helper to
the Ecosystem equivalent and update all call sites if decoupling is intended.
Preserve compatibility-sensitive YAML keys unchanged.
- Around line 705-715: Pin the Ecosystem-Models download in the
snapshot_download call to revision 52179373e251921e2bf268f020056a98b5267bfa,
preserving the existing repository, filters, and destination settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dbb1ba74-0341-4433-8ebc-c47dcbf986b9
📒 Files selected for processing (1)
installer.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (GEMINI.md)
**/*.py: Do not change 0.0.0.0 bindings in uvicorn or FastAPI startup calls - the binding is intentional for VastAI and RunPod cloud deployment
Use Python 3.10 or higherWhen spawning Python subprocesses, always inject
PYTHONIOENCODING=utf-8andPYTHONUTF8=1into the environment.
**/*.py: Write code that works on both Windows and Linux; do not target macOS.
Useasyncio.create_subprocess_execfor asynchronous subprocesses instead ofsubprocess.Popenwith a shell.
Files:
installer.py
**/*.{py,ts,tsx,js,jsx,bat}
📄 CodeRabbit inference engine (CLAUDE.md)
Keep paths cross-platform: use
os.path.join()or forward slashes, never hardcode backslashes, and preserve exact casing in paths.
Files:
installer.py
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Do not rungit submodulecommands or add Git submodules to the repository.
Do not modify Git remote configuration.
Do not add platform-specific npm packages directly, because they can causeEBADPLATFORMfailures on supported operating systems.
Files:
installer.py
**/*.{py,sh,bat}
📄 CodeRabbit inference engine (AGENTS.md)
Do not hardcode filesystem paths; use
sys.executableandos.path.join()where applicable.
Files:
installer.py
**/*.{py,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{py,ts,tsx}: Complete functions fully; do not leave methods incomplete during refactoring.
Preserve existing functionality during refactoring; UI changes are acceptable, but business logic must remain intact.
Update a function’s inline docstring when writing or modifying that function.
Files:
installer.py
🪛 Ruff (0.15.21)
installer.py
[warning] 718-718: Do not catch blind exception: Exception
(BLE001)
| #!/usr/bin/env python3 | ||
| """ | ||
| Ktiseos-Nyx-Trainer - Backend Dependency Installer | ||
| Ecosystem - Backend Dependency Installer |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Complete the rename consistently across installer output.
The changed module/CLI titles use Ecosystem, but the same path still exposes Ktiseos/KNX in the class/banner text, generated extra_model_paths.yaml metadata, _sync_knx_trainer_models, and logger messages such as Lines 717 and 720. This produces contradictory branding and stale diagnostics. Update user-visible/log labels and rename the private helper if this PR is intended to complete the decoupling; preserve any compatibility-sensitive YAML keys.
Also applies to: 689-689, 705-719, 876-876
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@installer.py` at line 3, Complete the Ecosystem rename across
installer-facing output by updating class and banner text, generated
extra_model_paths.yaml metadata values, and logger messages in the affected
sections, including the messages around lines 717 and 720. Rename the private
_sync_knx_trainer_models helper to the Ecosystem equivalent and update all call
sites if decoupling is intended. Preserve compatibility-sensitive YAML keys
unchanged.
There was a problem hiding this comment.
Yea i'll have run this through deepseek later on i just need to fix the model thing, this is biggie yes but not in this PR
There was a problem hiding this comment.
@ash-bluepollution95, understood—let’s keep this PR focused on restoring the model download. The broader Ecosystem branding/KNX cleanup can be handled separately. Would you like me to open a follow-up GitHub issue for it?
🐇💭
There was a problem hiding this comment.
@ash-bluepollution95, understood—let’s keep this PR focused on restoring the model download. The broader Ecosystem branding/KNX cleanup can be handled separately. Would you like me to open a follow-up GitHub issue for it?🐇💭
Nah i'll get deepseek onto it, :) My whole point of this is to cleanly decouple myself, in a way that changes the naming so it's easier for people to fork off and fix up later on -- I'm reitring from a lot of GenAI stuff finally and as much as i'm a LITTLE SMIDGE proud of this -- if i dont know enough how it all works i can't promise it's secure, and in the day and age of everythring that scares me.
There was a problem hiding this comment.
@ash-bluepollution95, completely understood. Keeping this PR focused on restoring the model download is sensible, and the broader decoupling deserves the deliberate pass you’re planning. You’ve clearly put meaningful work into this—stepping back responsibly when you can’t confidently stand behind the security posture is a thoughtful call. Wishing you the best with whatever you focus on next. 🐇
1 participant
Slowly working on decoupling the KNX name from the repo - Commmited to a few more fixes but this was the biggest issue i made a break in on accident after moving the repo over.
Literally just the URL swap over and some title fixes.
Am going to fix some more bugs before moving this off to someoen else
Summary by CodeRabbit
New Features
Bug Fixes