-
Notifications
You must be signed in to change notification settings - Fork 10
feat: Add deployment scripts for source code startup and multi-instance support #37
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -33,6 +33,7 @@ | |||||
| ) | ||||||
|
|
||||||
| from peewee import OperationalError | ||||||
| from werkzeug.exceptions import NotFound | ||||||
|
||||||
| from werkzeug.exceptions import NotFound |
Copilot
AI
Jan 9, 2026
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.
The logging.exception() call here will log an exception that wasn't caught - it's not inside an exception handler. Use logging.error() instead of logging.exception() when you're not in an exception context. logging.exception() should only be used within an except block to include the traceback.
| logging.exception(f"Data error: {message}") | |
| logging.error(f"Data error: {message}") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,13 +20,14 @@ | |
| from logging.handlers import RotatingFileHandler | ||
| from common.file_utils import get_project_base_directory | ||
|
|
||
| initialized_root_logger = False | ||
| _initialized_loggers = set() | ||
|
|
||
| def init_root_logger(logfile_basename: str, log_format: str = "%(asctime)-15s %(levelname)-8s %(process)d %(message)s"): | ||
| global initialized_root_logger | ||
| if initialized_root_logger: | ||
| global _initialized_loggers | ||
| # Allow re-initialization for different log file names (e.g., multi-instance servers) | ||
| if logfile_basename in _initialized_loggers: | ||
| return | ||
| initialized_root_logger = True | ||
| _initialized_loggers.add(logfile_basename) | ||
|
Comment on lines
+26
to
+30
|
||
|
|
||
| logger = logging.getLogger() | ||
| logger.handlers.clear() | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,15 +29,35 @@ function usage() { | |||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| ENABLE_WEBSERVER=1 # Default to enable web server | ||||||||||||||||||||||||||||||
| ENABLE_TASKEXECUTOR=1 # Default to enable task executor | ||||||||||||||||||||||||||||||
| ENABLE_DATASYNC=1 | ||||||||||||||||||||||||||||||
| ENABLE_MCP_SERVER=0 | ||||||||||||||||||||||||||||||
| ENABLE_ADMIN_SERVER=0 # Default close admin server | ||||||||||||||||||||||||||||||
| INIT_SUPERUSER_ARGS="" # Default to not initialize superuser | ||||||||||||||||||||||||||||||
| CONSUMER_NO_BEG=0 | ||||||||||||||||||||||||||||||
| CONSUMER_NO_END=0 | ||||||||||||||||||||||||||||||
| WORKERS=1 | ||||||||||||||||||||||||||||||
| ENABLE_WEBSERVER=${ENABLE_WEBSERVER:-1} # Default to enable web server | ||||||||||||||||||||||||||||||
| ENABLE_TASKEXECUTOR=${ENABLE_TASKEXECUTOR:-1} # Default to enable task executor | ||||||||||||||||||||||||||||||
| ENABLE_DATASYNC=${ENABLE_DATASYNC:-1} | ||||||||||||||||||||||||||||||
| ENABLE_MCP_SERVER=${ENABLE_MCP_SERVER:-0} | ||||||||||||||||||||||||||||||
| ENABLE_ADMIN_SERVER=${ENABLE_ADMIN_SERVER:-0} # Default close admin server | ||||||||||||||||||||||||||||||
| ENABLE_POWERRAG_SERVER=${ENABLE_POWERRAG_SERVER:-1} # Default close PowerRAG server | ||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||
| ENABLE_POWERRAG_SERVER=${ENABLE_POWERRAG_SERVER:-1} # Default close PowerRAG server | |
| ENABLE_POWERRAG_SERVER=${ENABLE_POWERRAG_SERVER:-0} # Default close PowerRAG server |
Copilot
AI
Jan 9, 2026
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.
The render_service_conf function uses eval "echo \"$line\"" to expand environment variables from service_conf.yaml.template, which introduces a shell code injection risk. If an attacker can influence any of the environment variables used in the template (e.g., via orchestrator/UI that lets them set REDIS_PASSWORD, RAGFLOW_HOST, etc.), they can embed command substitutions like $(...) so that when eval re-parses the expanded line, arbitrary commands execute inside the container with the entrypoint's privileges. Replace the eval-based templating with a non-evaluating mechanism (e.g., envsubst or a simple variable substitution script that does not invoke the shell parser on untrusted data) so that environment values are treated as data, not executable shell code.
| rm -f "${out_file}" | |
| while IFS= read -r line || [[ -n "$line" ]]; do | |
| # shellcheck disable=SC2034 | |
| SVR_HTTP_PORT="${ragflow_port}" ADMIN_SVR_HTTP_PORT="${admin_port}" \ | |
| eval "echo \"$line\"" >> "${out_file}" | |
| done < "${TEMPLATE_FILE}" | |
| if ! command -v envsubst >/dev/null 2>&1; then | |
| echo "ERROR: envsubst is required to render service_conf from template." >&2 | |
| return 1 | |
| fi | |
| rm -f "${out_file}" | |
| SVR_HTTP_PORT="${ragflow_port}" ADMIN_SVR_HTTP_PORT="${admin_port}" \ | |
| envsubst < "${TEMPLATE_FILE}" > "${out_file}" |
Copilot
AI
Jan 9, 2026
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.
Inconsistent configuration file naming between deploy.sh and entrypoint.sh. In deploy.sh (line 647), the main instance uses GLOBAL_SERVICE_CONF (defaults to "service_conf.yaml"), while in entrypoint.sh (lines 282, 319), it uses "local.service_conf.yaml". This inconsistency could cause configuration issues when switching between deployment modes. Consider using a consistent naming scheme or documenting this intentional difference.
| conf_name="local.service_conf.yaml" | |
| conf_name="service_conf.yaml" |
Copilot
AI
Jan 9, 2026
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.
The variable INIT_SUPERUSER_ARGS is used on line 303 but is not defined anywhere in the visible changes. If this variable was previously defined and is still needed, it should be initialized with a default value (e.g., INIT_SUPERUSER_ARGS="${INIT_SUPERUSER_ARGS:-}") to prevent potential undefined variable errors when running with "set -u".
Copilot
AI
Jan 9, 2026
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.
Missing start logic for PowerRAG server. ENABLE_POWERRAG_SERVER is defined and set to 1 by default on line 37, and the start_powerrag_server() function is defined on lines 264-270, but the visible component startup section (lines 342-350) doesn't include a check to start the PowerRAG server. Based on the pattern for MCP server and admin server, there should be a corresponding if block checking ENABLE_POWERRAG_SERVER and calling start_powerrag_server.
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.
Import statement moved after init_root_logger() call but before the imports it depends on. Lines 30-31 import signal and sys, which come after the init_root_logger() call on line 29. While this works, it creates an unusual import order. If init_root_logger() internally depends on these modules or if there are circular dependencies, this could cause issues. Consider keeping all imports together at the top of the file, or add a comment explaining why logging must be initialized before other imports.