fix: Improve config path resolution and error messages (fixes #262)
Three critical UX improvements for custom config handling: 1. User config directory support: - Added ~/.config/skill-seekers/configs/ to search path - Users can now place custom configs in their home directory - Path resolution order: exact path → ./configs/ → user config dir → API 2. Better error messages: - Show all searched absolute paths when config not found - Added get_last_searched_paths() function to track locations - Clear guidance on where to place custom configs 3. Auto-create config.json: - ConfigManager now creates config.json on first initialization - Creates configs/ subdirectory for user custom configs - Display shows custom configs directory path Fixes reported by @melamers in issue #262 where: - Config path shown by `skill-seekers config` didn't exist - Unclear where to save custom configs - Error messages didn't show exact paths searched Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,9 @@ logger = logging.getLogger(__name__)
|
|||||||
|
|
||||||
API_BASE_URL = "https://api.skillseekersweb.com"
|
API_BASE_URL = "https://api.skillseekersweb.com"
|
||||||
|
|
||||||
|
# Track last searched paths for better error messages
|
||||||
|
_last_searched_paths = []
|
||||||
|
|
||||||
|
|
||||||
def fetch_config_from_api(
|
def fetch_config_from_api(
|
||||||
config_name: str, destination: str = "configs", timeout: float = 30.0
|
config_name: str, destination: str = "configs", timeout: float = 30.0
|
||||||
@@ -138,8 +141,9 @@ def resolve_config_path(config_path: str, auto_fetch: bool = True) -> Optional[P
|
|||||||
|
|
||||||
Tries to find config in this order:
|
Tries to find config in this order:
|
||||||
1. Exact path as provided
|
1. Exact path as provided
|
||||||
2. With 'configs/' prefix added
|
2. With 'configs/' prefix added (current directory)
|
||||||
3. Fetch from API (if auto_fetch=True)
|
3. User config directory (~/.config/skill-seekers/configs/)
|
||||||
|
4. Fetch from API (if auto_fetch=True)
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
config_path: Config file path or name
|
config_path: Config file path or name
|
||||||
@@ -154,18 +158,35 @@ def resolve_config_path(config_path: str, auto_fetch: bool = True) -> Optional[P
|
|||||||
... with open(path) as f:
|
... with open(path) as f:
|
||||||
... config = json.load(f)
|
... config = json.load(f)
|
||||||
"""
|
"""
|
||||||
|
# Track searched paths for better error messages
|
||||||
|
global _last_searched_paths
|
||||||
|
_last_searched_paths = []
|
||||||
|
|
||||||
# 1. Try exact path
|
# 1. Try exact path
|
||||||
exact_path = Path(config_path)
|
exact_path = Path(config_path)
|
||||||
|
_last_searched_paths.append(exact_path.resolve())
|
||||||
if exact_path.exists():
|
if exact_path.exists():
|
||||||
return exact_path.resolve()
|
return exact_path.resolve()
|
||||||
|
|
||||||
# 2. Try with configs/ prefix
|
# 2. Try with configs/ prefix (current directory)
|
||||||
if not config_path.startswith("configs/"):
|
if not config_path.startswith("configs/"):
|
||||||
with_prefix = Path("configs") / config_path
|
with_prefix = Path("configs") / config_path
|
||||||
|
_last_searched_paths.append(with_prefix.resolve())
|
||||||
if with_prefix.exists():
|
if with_prefix.exists():
|
||||||
return with_prefix.resolve()
|
return with_prefix.resolve()
|
||||||
|
|
||||||
# 3. Try API fetch (if enabled)
|
# 3. Try user config directory
|
||||||
|
user_config_dir = Path.home() / ".config" / "skill-seekers" / "configs"
|
||||||
|
|
||||||
|
# Extract just the filename if path contains directory separators
|
||||||
|
config_filename = Path(config_path).name
|
||||||
|
user_config_path = user_config_dir / config_filename
|
||||||
|
_last_searched_paths.append(user_config_path)
|
||||||
|
|
||||||
|
if user_config_path.exists():
|
||||||
|
return user_config_path.resolve()
|
||||||
|
|
||||||
|
# 4. Try API fetch (if enabled)
|
||||||
if auto_fetch:
|
if auto_fetch:
|
||||||
# Extract config name (remove .json, remove configs/ prefix)
|
# Extract config name (remove .json, remove configs/ prefix)
|
||||||
config_name = config_path
|
config_name = config_path
|
||||||
@@ -182,3 +203,19 @@ def resolve_config_path(config_path: str, auto_fetch: bool = True) -> Optional[P
|
|||||||
return fetched_path.resolve()
|
return fetched_path.resolve()
|
||||||
|
|
||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def get_last_searched_paths() -> list[Path]:
|
||||||
|
"""
|
||||||
|
Get the list of paths that were searched in the last resolve_config_path call.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
List of absolute paths that were checked for the config file
|
||||||
|
|
||||||
|
Example:
|
||||||
|
>>> resolve_config_path('myconfig.json', auto_fetch=False)
|
||||||
|
>>> paths = get_last_searched_paths()
|
||||||
|
>>> for p in paths:
|
||||||
|
... print(f"Searched: {p}")
|
||||||
|
"""
|
||||||
|
return _last_searched_paths.copy()
|
||||||
|
|||||||
@@ -43,15 +43,28 @@ class ConfigManager:
|
|||||||
self.config_file = self.CONFIG_FILE
|
self.config_file = self.CONFIG_FILE
|
||||||
self.progress_dir = self.PROGRESS_DIR
|
self.progress_dir = self.PROGRESS_DIR
|
||||||
self._ensure_directories()
|
self._ensure_directories()
|
||||||
|
|
||||||
|
# Check if config file exists before loading
|
||||||
|
config_exists = self.config_file.exists()
|
||||||
self.config = self._load_config()
|
self.config = self._load_config()
|
||||||
|
|
||||||
|
# Save config file if it was just created with defaults
|
||||||
|
if not config_exists:
|
||||||
|
self.save_config()
|
||||||
|
|
||||||
def _ensure_directories(self):
|
def _ensure_directories(self):
|
||||||
"""Ensure configuration and progress directories exist with secure permissions."""
|
"""Ensure configuration and progress directories exist with secure permissions."""
|
||||||
|
# Create main config and progress directories
|
||||||
for directory in [self.config_dir, self.progress_dir]:
|
for directory in [self.config_dir, self.progress_dir]:
|
||||||
directory.mkdir(parents=True, exist_ok=True)
|
directory.mkdir(parents=True, exist_ok=True)
|
||||||
# Set directory permissions to 700 (rwx------)
|
# Set directory permissions to 700 (rwx------)
|
||||||
directory.chmod(stat.S_IRWXU)
|
directory.chmod(stat.S_IRWXU)
|
||||||
|
|
||||||
|
# Also create configs subdirectory for user custom configs
|
||||||
|
configs_dir = self.config_dir / "configs"
|
||||||
|
configs_dir.mkdir(exist_ok=True)
|
||||||
|
configs_dir.chmod(stat.S_IRWXU)
|
||||||
|
|
||||||
def _load_config(self) -> dict[str, Any]:
|
def _load_config(self) -> dict[str, Any]:
|
||||||
"""Load configuration from file or create default."""
|
"""Load configuration from file or create default."""
|
||||||
if not self.config_file.exists():
|
if not self.config_file.exists():
|
||||||
@@ -391,6 +404,7 @@ class ConfigManager:
|
|||||||
"""Display current configuration summary."""
|
"""Display current configuration summary."""
|
||||||
print("\n📋 Skill Seekers Configuration\n")
|
print("\n📋 Skill Seekers Configuration\n")
|
||||||
print(f"Config file: {self.config_file}")
|
print(f"Config file: {self.config_file}")
|
||||||
|
print(f"Custom configs dir: {self.config_dir / 'configs'}")
|
||||||
print(f"Progress dir: {self.progress_dir}\n")
|
print(f"Progress dir: {self.progress_dir}\n")
|
||||||
|
|
||||||
# GitHub profiles
|
# GitHub profiles
|
||||||
|
|||||||
@@ -30,7 +30,11 @@ from bs4 import BeautifulSoup
|
|||||||
# Add parent directory to path for imports when run as script
|
# Add parent directory to path for imports when run as script
|
||||||
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
|
||||||
|
|
||||||
from skill_seekers.cli.config_fetcher import list_available_configs, resolve_config_path
|
from skill_seekers.cli.config_fetcher import (
|
||||||
|
get_last_searched_paths,
|
||||||
|
list_available_configs,
|
||||||
|
resolve_config_path,
|
||||||
|
)
|
||||||
from skill_seekers.cli.config_validator import ConfigValidator
|
from skill_seekers.cli.config_validator import ConfigValidator
|
||||||
from skill_seekers.cli.constants import (
|
from skill_seekers.cli.constants import (
|
||||||
CONTENT_PREVIEW_LENGTH,
|
CONTENT_PREVIEW_LENGTH,
|
||||||
@@ -1775,20 +1779,32 @@ def load_config(config_path: str) -> dict[str, Any]:
|
|||||||
if resolved_path is None:
|
if resolved_path is None:
|
||||||
# Config not found locally and fetch failed
|
# Config not found locally and fetch failed
|
||||||
available = list_available_configs()
|
available = list_available_configs()
|
||||||
|
searched_paths = get_last_searched_paths()
|
||||||
|
|
||||||
logger.error("❌ Error: Config file not found: %s", config_path)
|
logger.error("❌ Error: Config file not found: %s", config_path)
|
||||||
logger.error(" Tried:")
|
|
||||||
logger.error(" 1. Local path: %s", config_path)
|
|
||||||
logger.error(" 2. With prefix: configs/%s", config_path)
|
|
||||||
logger.error(" 3. SkillSeekersWeb.com API")
|
|
||||||
logger.error("")
|
logger.error("")
|
||||||
|
logger.error(" Searched in these locations:")
|
||||||
|
for i, path in enumerate(searched_paths, 1):
|
||||||
|
logger.error(" %d. %s", i, path)
|
||||||
|
logger.error(" %d. SkillSeekersWeb.com API", len(searched_paths) + 1)
|
||||||
|
logger.error("")
|
||||||
|
|
||||||
|
# Show where user should place custom configs
|
||||||
|
user_config_dir = Path.home() / ".config" / "skill-seekers" / "configs"
|
||||||
|
logger.error(" 💡 To use a custom config, place it in one of these locations:")
|
||||||
|
logger.error(" • Current directory: ./configs/%s", Path(config_path).name)
|
||||||
|
logger.error(" • User config directory: %s", user_config_dir / Path(config_path).name)
|
||||||
|
logger.error(" • Absolute path: /full/path/to/%s", Path(config_path).name)
|
||||||
|
logger.error("")
|
||||||
|
|
||||||
if available:
|
if available:
|
||||||
logger.error(" 📋 Available configs from API (%d total):", len(available))
|
logger.error(" 📋 Or use a preset config from API (%d total):", len(available))
|
||||||
for cfg in available[:10]: # Show first 10
|
for cfg in available[:10]: # Show first 10
|
||||||
logger.error(" • %s", cfg)
|
logger.error(" • %s", cfg)
|
||||||
if len(available) > 10:
|
if len(available) > 10:
|
||||||
logger.error(" ... and %d more", len(available) - 10)
|
logger.error(" ... and %d more", len(available) - 10)
|
||||||
logger.error("")
|
logger.error("")
|
||||||
logger.error(" 💡 Use any config name: skill-seekers scrape --config <name>.json")
|
logger.error(" 💡 Use any preset: skill-seekers scrape --config <name>.json")
|
||||||
logger.error(" 🌐 Browse all: https://skillseekersweb.com/")
|
logger.error(" 🌐 Browse all: https://skillseekersweb.com/")
|
||||||
else:
|
else:
|
||||||
logger.error(" ⚠️ Could not connect to API to list available configs")
|
logger.error(" ⚠️ Could not connect to API to list available configs")
|
||||||
|
|||||||
Reference in New Issue
Block a user