Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Enhance configuration options#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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Cheelax wants to merge7 commits intocoderamp-labs:main
base:main
Choose a base branch
Loading
fromCheelax:improve-env

Conversation

Cheelax
Copy link

closes#285

  • Added examples for configuring processing limits in README.md.
  • Introduced new CLI options for max files, max total size, and max directory depth.
  • Updated environment variable support in config.py for various limits.
  • Modified ingestion functions to accept new parameters for file processing limits.
  • Enhanced limit checks in ingestion logic to utilize new configuration options.

- Added examples for configuring processing limits in README.md.- Introduced new CLI options for max files, max total size, and max directory depth.- Updated environment variable support in config.py for various limits.- Modified ingestion functions to accept new parameters for file processing limits.- Enhanced limit checks in ingestion logic to utilize new configuration options.
Refactor* Deduplicate `_get_env_var` by moving it to `utils/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` for `markdownlint` 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` and `ignore_patterns` in `parse_query` and `ingest_async`.* Tidy docstrings for `_async_main`, `IngestionQuery`, `parse_query`, `ingest_async`, and `ingest`.Tests* Temporarily disable `[tool.ruff.lint.isort]` due to conflict with the `isort` pre-commit hook.* Add new arguments to `expected` in `test_parse_query_without_host`.* Run `pre-commit` hooks.
@filipchristiansen
Copy link
Contributor

Thanks for your work@Cheelax. I have rebase uponmain and added a commit with the following:

Refactor

  • Deduplicate_get_env_var by moving it toutils/config_utils.py.
  • Remove redundantlocal_path parameter from_process_file.

Fix

  • Add missingtag parameter to_async_main,main, and_CLIArgs.
  • Introduce the missing--tag CLI flag.

Docs and consistency

  • UpdateREADME.md formarkdownlint compliance and other minor tweaks.
  • Add missing argument docs to_async_main docstring.
  • Re-order global variables inconfig.py for consistency.
  • Swap the order ofinclude_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 toexpected intest_parse_query_without_host.
  • Runpre-commit hooks.

TODO

  • The environment variable names such asGITINGEST_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").

Comment on lines 9 to 40
def _get_env_var(key: str, default: int | str, cast_func: Callable[[str], int | str] | None = None) -> int | str:
"""Get environment variable with ``GITINGEST_`` prefix and optional type casting.

Parameters
----------
key : str
The name of the environment variable.
default : int | str
The default value to return if the environment variable is not set.
cast_func : Callable[[str], int | str] | None
The function to cast the environment variable to the desired type.

Returns
-------
int | str
The value of the environment variable, cast to the desired type if provided.

"""
env_key = f"GITINGEST_{key}"
value = os.environ.get(env_key)

if value is None:
return default

if cast_func:
try:
return cast_func(value)
except (ValueError, TypeError):
print(f"Warning: Invalid value for {env_key}: {value}. Using default: {default}")
return default

return value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I like the approach but I think in the end havingint | str as a return type might cause some unnecessary headaches in some situations.

Perhaps just returningstr and casting whenever calling the function is fine?

@Cheelax
Copy link
Author

@filipchristiansen it looks to me that the naming is good in the readme. Every var I am looking for should be prefixed by :

   env_key = f"GITINGEST_{key}"    value = os.environ.get(env_key)
filipchristiansen reacted with thumbs up emoji

Cheelaxand others added2 commitsJuly 7, 2025 15:05
- Introduced a new helper function `_get_int_env_var` to retrieve environment variables as integers with default fallback and error handling.- Updated `config.py` and `server_config.py` to use the new helper function for better clarity and maintainability.- Simplified `_get_env_var` in `config_utils.py` to focus solely on retrieving string values without type casting.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@NicolasIRAGNENicolasIRAGNENicolasIRAGNE left review comments

@filipchristiansenfilipchristiansenfilipchristiansen left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: config parameters as environment variables
3 participants
@Cheelax@filipchristiansen@NicolasIRAGNE

[8]ページ先頭

©2009-2025 Movatter.jp