Fix critical bugs and add Windows compatibility - #149
Open
YanCheng-go wants to merge 6 commits into
Open
YanCheng-go wants to merge 6 commits into
YanCheng-go wants to merge 6 commits into
Conversation
- Fix KeyError crash when BASE_DIR env var is unset (predict_controller.py) - Replace NotImplementedError in verify_model_path with working validation - Log warning on embedding load failure instead of silently swallowing - Move logger to module level in app.py to prevent NameError in pre_check() - Remove unused --host/--port args from Dockerfile CMD (app.py doesn't parse them) - Fix bare except clause in plugin.py to catch specific exceptions - Update test_sam.py to match current Sam class API signatures - Remove duplicate if __name__ block in test_alive_controller.py - Fix DeveloperGuide.md: wrong script name and invalid JSON in curl example - Fix "lastest" typo in deploy.yml Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Plugin (plugin.py): - Change server_url from 0.0.0.0 to 127.0.0.1 (Windows can't connect to 0.0.0.0) - Add Windows Docker Desktop path fallback for docker_path - Replace all shell=True subprocess calls for Docker with list args (cross-platform) - Add Windows process kill via netstat/taskkill in stop_server() - Add Windows env download path (win-end-v2 release) in start_server() - Add Windows local server launch using Python directly (no bash dependency) - Replace macOS tar shell command with Python tarfile module - Fix path validation: os.path.commonpath -> os.path.normpath (works on Windows) - Add launch_server_local.bat for Windows parity Server (predict_controller.py): - Replace f-string path joins with os.path.join for geojson output paths Addresses #4 (Windows support) and partially #105 (cross-platform paths) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Tests cover verify_image_path, verify_model_path, reorganize_prompts, reproject_prompts, BASE_DIR fallback logic, and geojson path construction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add 22 more tests (47 total) covering: - verify_model_path edge cases (nested HF paths, spaces, whitespace) - reorganize_prompts with multiple points/boxes/text, missing data, empty fields - reproject_prompts with scaled transforms, negative coords, multiple points/boxes - predict() endpoint error handling (missing/invalid image, unknown model, empty JSON) - app.py pre_check() with writable/nonexistent/default cache dirs - geojson path construction with all model prefixes and spaces in names Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- DeveloperGuide: Add Windows setup and launch instructions, document BASE_DIR fallback and auto-created subdirectories - APIReference: Fix AOI format to valid JSON, clarify model_path accepts HuggingFace IDs or local directory paths - TROUBLESHOOTING: Add Windows-specific setup instructions and notes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… stop+rm - predict_controller: Move BASE_DIR/temp_dir/embeddings_dir to module-level constants computed once at import, removing per-request os.makedirs and os.environ.get calls. Rename from ALL_CAPS to _lowercase (not constants). - plugin.py: Reuse self.cache_dir instead of re-deriving MODEL_CACHE_DIR from USERPROFILE env var on Windows. - plugin.py: Collapse sequential docker stop + docker rm into single docker rm -f call. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
verify_model_path()(was raisingNotImplementedError), fixBASE_DIRcrash when env var is missing (now falls back to~/.easyearth), useos.path.join()for file paths instead of f-strings, fix typo in deploy workflow.batlaunch script) and server (remove CLI args from Dockerfile CMD, bind to127.0.0.1instead of0.0.0.0)test_sam.pyto use current method signatures (model_path=kwarg, simplifiedget_image_embeddings/get_maskscalls), remove duplicate__main__block intest_alive_controller.pytarfilemodule instead of shelltar, use specific exception types instead of bareexceptDeveloperGuide.mdto reference correct script name and updated AOI formatCloses #19
Closes #105
Test plan
python -m easyearth.applaunch_server_local.battest_alive_controller.pyandtest_sam.pytest_predict_controller.py(47 unit tests)BASE_DIRenv var (should fallback gracefully)🤖 Generated with Claude Code