Fix #245: I’ll inspect the issue details first, then trace the logging/config path handling locally#255
Merged
Merged
Conversation
5c514eb to
28cde0e
Compare
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
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
litecli/main.py: audit logs already expand~, butmain.log_fileis checked and opened without normalizing user paths. I’m going to pull the issue text/comments now to make sure the fix matches the reported behavior.Issue Getting error 'Unable to open log file' when log file move to other directory other than default #245 confirms the failure is withlog_file = ~/.cache/litecli/logor-l ~/.cache/litecli/log; there are no discussion comments. Next I’m checking the existing tests around config/logging so the regression test lands in the right layer.The failing check isdir_path_exists(log_file)on the literal~/.cache/...string. I’m going to normalize the configured log path before directory creation and handler setup, then add a narrow test that exercisesinitialize_logging()withHOMEredirected to a temporary directory.The implementation is now one-line in the logging setup plus a regression test and changelog entry. I’m running the focused test first, then I’ll run a formatter/lint check if the test passes.The focused pytest invocation failed before collecting tests because the editable package metadata is not installed in this environment (PackageNotFoundError: litecli). I’m checking the package config and will install the local project editable if that’s sufficient without pulling new dependencies.This shell is picking up a parent.venvthat has Python but nopip, so the usual editable install path is blocked. I’m checking for the repo’s available runners (uv,tox, direct module path workarounds) before choosing the least invasive verification route.The repo’s tox also installs viauv, so I’m going to add only the local editable package metadata withuv pip install --no-deps -e .; that avoids dependency resolution and should unblock direct pytest in the existing environment.uvtried to use the user-level cache outside the writable sandbox. I’m retrying withUV_CACHE_DIRunder/private/tmp, still using--no-depsso it only installs this checkout’s metadata.uvis unusable in this sandbox because it panics while initializing. I’m checking alternate Python executables now; if one haspipand the test dependencies, I can run the focused test without touching the broken parent virtualenv.The usable environment has pytest and project dependencies but lacks onlyliteclipackage metadata. I’m going to run the narrow pytest in-process with a temporaryimportlib.metadata.version("litecli")shim so collection can proceed; this does not change repo files.The parent environment also lackssqlparse, so normal pytest collection can’t run there. I’m checking whether the Homebrew-installed litecli environment already has the runtime dependencies available; if so, I can point Python at those packages and still run the local test code.Normal pytest is blocked by missing dependencies in the available virtualenvs. I’m trying a disposable venv under/private/tmp; if dependency installation can’t reach packages in this sandbox, I’ll fall back to syntax checks plus a small direct logging-path exercise.Dependency installation is blocked by network restrictions, so I can’t run the normal pytest target locally. I’m switching to local checks that don’t need external packages: compile, ruff on touched Python files, and a direct stubbed execution of the modified logging method.The direct logging exercise passes, and the touched Python files compile and pass ruff. I’m doing the final review pass now: whitespace/diff check plus a quick read for unintended behavior changes.Implemented the focused fix for issue Getting error 'Unable to open log file' when log file move to other directory other than default #245:initialize_logging()now expands~before creating/checking/opening the configured log path in litecli/main.py.~/.cache/litecli/login tests/test_main.py, and updated the Unreleased changelog in CHANGELOG.md.Issue
Fixes #245
Verification
python -m py_compile litecli/main.py tests/test_main.pyruff check litecli/main.py tests/test_main.pyruff format --check litecli/main.py tests/test_main.pygit diff --checkinitialize_logging()with~/.cache/litecli/logpytestcould not run in this sandbox because the active env lacksliteclipackage metadata and runtime deps likesqlparse; dependency install was blocked by network restrictions.