Skip to content
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

Refactor/config #46

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Refactor/config #46

wants to merge 3 commits into from

Conversation

1martin1
Copy link
Collaborator

No description provided.

@1martin1 1martin1 requested a review from angrymuskrat January 27, 2025 13:57
Comment on lines +9 to +20
redis_host (str): The hostname of the Redis server. Defaults to "localhost".
redis_port (int): The port number of the Redis server. Defaults to 6379.
redis_prefix (str): Prefix for keys used in Redis. Defaults to "llm-api".
rabbit_host (str): The hostname of the RabbitMQ server. Defaults to "localhost".
rabbit_port (int): The port number of the RabbitMQ server. Defaults to 5672.
rabbit_login (str): The username for RabbitMQ authentication. Defaults to "admin".
rabbit_password (str): The password for RabbitMQ authentication. Defaults to "admin".
queue_name (str): The name of the RabbitMQ queue to use. Defaults to "llm-api-queue".
model_path (str): Path to the model being used. Defaults to None.
token_len (int): The maximum length of tokens for processing by the model. Defaults to None.
tensor_parallel_size (int): The size of tensor parallelism for distributed processing. Defaults to None.
gpu_memory_utilisation (float): The percentage of GPU memory utilization for the model. Defaults to None.
Copy link
Contributor

Choose a reason for hiding this comment

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

Не принципиально, но стоит ли дублировать тут type hint-ы, если они есть ниже?

Comment on lines +25 to +31
redis_host: str = "localhost",
redis_port: int = 6379,
redis_prefix: str = "llm-api",
rabbit_host: str = "localhost",
rabbit_port: int = 5672,
rabbit_login: str = "admin",
rabbit_password: str = "admin",
Copy link
Contributor

Choose a reason for hiding this comment

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

Думаю лучше убрать такие дефолтные значения, особенно если они реальные.

Copy link
Collaborator

Choose a reason for hiding this comment

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

в целом это дефолтный порты и пароли от редиса и rabbit

Copy link
Contributor

Choose a reason for hiding this comment

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

Если так, то ок. Но все равно бы не дублировал их в двух местах

Comment on lines +76 to +89
return Config(
env_vars.get("REDIS_HOST", "localhost"),
int(env_vars.get("REDIS_PORT", "6379")),
env_vars.get("REDIS_PREFIX", "llm-api"),
env_vars.get("RABBIT_MQ_HOST", "localhost"),
int(env_vars.get("RABBIT_MQ_PORT", "5672")),
env_vars.get("RABBIT_MQ_LOGIN", "admin"),
env_vars.get("RABBIT_MQ_PASSWORD", "admin"),
env_vars.get("QUEUE_NAME", "llm-api-queue"),
env_vars.get("MODEL_PATH"),
int(env_vars.get("TOKENS_LEN", "16384")),
int(env_vars.get("TENSOR_PARALLEL_SIZE", "2")),
float(env_vars.get("GPU_MEMORY_UTILISATION", "0.9")),
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Тут аналогично - если такой сетап нужен для тестов, то лучше создать отдельный env-файл с нужными значениями.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants