Skip to content

feat: add option to force start rag-api-server#148

Merged
DarumaDocker merged 2 commits intomainfrom
force-rag
Jan 6, 2025
Merged

feat: add option to force start rag-api-server#148
DarumaDocker merged 2 commits intomainfrom
force-rag

Conversation

@DarumaDocker
Copy link
Contributor

No description provided.

Copy link
Contributor

juntao commented Jan 6, 2025

Hello, I am a code review agent on flows.network. Here are my reviews of changed source code files in this PR.


gaianet

Potential issues

  1. Issue: The grep command used in check_config_options() and other places is case-sensitive and may miss fields with different casing.

    • Explanation: Using grep -q '"address":' will only match "address": exactly, but it won’t match "Address":, "ADDRESS":, etc.
  2. Issue: The ulimit -n 10000 command is applied to both Darwin and Linux, which might not be appropriate for all environments.

    • Explanation: Setting the file descriptor limit without checking current limits or providing an option to customize it can lead to unexpected behavior on systems with different requirements.
  3. Issue: The nohup commands are used extensively but do not redirect both stdout and stderr properly, which can lead to incomplete logging.

    • Explanation: While most nohup commands use > $log_dir/some.log 2>&1, there are a few instances like nohup supervise $gaianet_base_dir > $log_dir/start-vector.log 2>&1 & that might not capture all error logs if supervise writes to stderr in unexpected ways.

Summary of changes

    • Added force_rag parameter to the start() function to allow forcing the start of rag-api-server.
  • Modified condition to start Qdrant instance to include the case when force_rag is true.
  • Introduced a new command-line option --force-rag to enable forced starting of rag-api-server.

gaianet Outdated
check_base_dir
fi
;;
--rag)
Copy link
Contributor

Choose a reason for hiding this comment

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

How about use --force-rag instead of --rag? Thanks!

@DarumaDocker DarumaDocker merged commit 5c2ae96 into main Jan 6, 2025
2 checks passed
@DarumaDocker DarumaDocker deleted the force-rag branch January 6, 2025 10:08
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