-
Notifications
You must be signed in to change notification settings - Fork 892
feat: enhance config options, support env vars #385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Thanks for your work @Cheelax. I have rebase upon main
and added a commit with the following:
Refactor
- Deduplicate
_get_env_var
by moving it toutils/config_utils.py
. - Remove redundant
local_path
parameter from_process_file
.
Fix
- Add missing
tag
parameter to_async_main
,main
, and_CLIArgs
. - Introduce the missing
--tag
CLI flag.
Docs and consistency
- Update
README.md
formarkdownlint
compliance and other minor tweaks. - Add missing argument docs to
_async_main
docstring. - Re-order global variables in
config.py
for consistency. - Swap the order of
include_patterns
andignore_patterns
inparse_query
andingest_async
. - Tidy docstrings for
_async_main
,IngestionQuery
,parse_query
,ingest_async
, andingest
.
Tests
- Temporarily disable
[tool.ruff.lint.isort]
due to conflict with theisort
pre-commit hook. - Add new arguments to
expected
intest_parse_query_without_host
. - Run
pre-commit
hooks.
TODO
-
The environment variable names such as
GITINGEST_MAX_FILE_SIZE) do not seem to align with the names used and looked for in
_get_env_var`. -
The
_get_env_var
function returnsint | str
which causes downstream problems when the variables set with this functions are used (Type "int | str" is incompatible with type "int"
).
This comment was marked as resolved.
This comment was marked as resolved.
@filipchristiansen Hello, any news on this?
@filipchristiansen Hello, any news on this?
The code crashes when running the server locally, possibly due to a circular import. I will have a look at it.
refactor(config,lint): centralize env utils, switch to ruff.isort, drop unused constants
- remove
isort
in favour ofruff.lint.isort
- remove unused constants
BASE_DIR
andTEMPLATE_DIR
intests/test_flow_integration.py
- rename constant
templates
toJINJA_TEMPLATES
insrc/server/server_config.py
- move function definition of
_get_int_env_var
fromsrc/server/server_config.py
tosrc/gitingest/utils/config_utils.py
- remove duplicated function definition of
_get_int_env_var
insrc/gitingest/config.py
- rename function
_get_env_var
to_get_str_env_var
for clarity - move
Colors
fromsrc/server/server_utils.py
tosrc/gitingest/utils/colors.py
to break circular import chain
@Cheelax The branch needs to be rebase upon main
Update: Done
Closes coderamp-labs#285 * add support for environment variables (`GITINGEST_*`) to override `config.py` defaults * implement a precedence hierarchy: CLI/Python args → environment variables → default values * introduce new CLI options (`--max-files`, `--max-total-size`, `--max-directory-depth`). * centralise environment variable utilities in `utils/config_utils.py` with functions `_get_str_env_var` and `_get_int_env_var` * add configuration examples to `README.md` * tidy and update docstrings * update tests * add missing `--tag` CLI flag * remove `isort` in favour of `ruff.lint.isort` * remove unused constants `BASE_DIR` and `TEMPLATE_DIR` in `tests/server/test_flow_integration.py` * rename constant `templates` to `JINJA_TEMPLATES` in `src/server/server_config.py` * move `Colors` from `src/server/server_utils.py` to `src/gitingest/utils/colors.py` to break circular import chain Co-authored-by: Cheelax <thomas.belloc@gmail.com>
a720ad2
to
adda1ce
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@filipchristiansen i'll let add your review and merge it.
This pull request has merge conflicts that must be resolved before it can be merged.
This pull request has resolved merge conflicts and is ready for review.
This pull request has merge conflicts that must be resolved before it can be merged.
Uh oh!
There was an error while loading. Please reload this page.
Closes #285
GITINGEST_*
) to overrideconfig.py
defaults--max-files
,--max-total-size
,--max-directory-depth
).utils/config_utils.py
with functions_get_str_env_var
and_get_int_env_var
README.md
--tag
CLI flagisort
in favour ofruff.lint.isort
BASE_DIR
andTEMPLATE_DIR
intests/server/test_flow_integration.py
templates
toJINJA_TEMPLATES
insrc/server/server_config.py
Colors
fromsrc/server/server_utils.py
tosrc/gitingest/utils/colors.py
to break circular import chain