merge: from main to 3.0 branch #34062
Conversation
…ory (#33956) * refactor: improve install script for customizable installation directory * fix: update installation script for correct version mode and tool names * fix: correct quoting and comments in installation script for improved clarity * fix: remove commented debug output for cleaner installation script * refactor: update installation paths and add customizable installation directory support for tdgpt * feat: add help option to installation script and improve directory handling * fix: remove unnecessary sudo from virtual environment creation * fix: update anode creation IP address in test case * fix: add missing sys and os imports for path configuration * feat: add support for additional virtual environments and model services * refactor: support dynamic download model. * fix: fix errors. * feat: add support for additional model environments and update start-model script * fix: comment out shesd algorithm query in anomaly window test * feat: unify model service start and stop scripts for improved usability * feat: update uninstall script to support dynamic virtual environment removal * fix: improve formatting and clarity in model service start/stop script documentation * fix: add punctuation and improve usage instructions in model service documentation * fix: improve formatting and clarity in model service startup and shutdown script documentation * fix: correct link path in management documentation * feat: add stop-model script to installation and enhance model service startup logging * fix: improve comments and logging messages for clarity in installation and configuration scripts * fix(uninstall): update clean_venv function to prompt for manual deletion of Python virtual environments * fix(install): modify install_main_path to only delete non-data/log/cfg files * fix(test): update anode creation and expected row counts in TDgpt tests * fix(install): update service file handling to use a temporary file during installation * fix(install): update service configuration path in install_service_on_systemd function * Refactor remove.sh script for improved usability and structure - Added command-line options for silent mode and custom install directory. - Enhanced installation directory detection logic. - Simplified service management commands by removing unnecessary sudo usage. - Improved handling of configuration, data, and log file removal. - Updated service management for macOS to handle user mode correctly. - Cleaned up redundant code and improved readability. Fix start-model.sh to ensure proper log file path handling - Adjusted log file path construction to prevent double slashes in the log directory. * fix(install): remove redundant assignment of service_config in install_service_on_systemd function * fix(install): update installation script for client and server modes, improve OS compatibility * fix(install): update version mode to edge, improve error messages, and enhance path safety checks * fix(install): change default version type from client to server in installation script * fix(install): streamline installation script by removing unnecessary echo statements and improving argument handling in start-model script * fix(install): enhance path handling in installation scripts and improve safety checks --------- Co-authored-by: Haojun Liao <hjliao@taosdata.com>
…nd msg to trigger if error happens in reader (#34031)
guanshengliang
requested review from
a team,
dapan1121,
feici02,
hzcheng and
zitsen
as code owners
Summary of ChangesHello @guanshengliang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates significant updates from the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a unified model management system for TDgpt, replacing individual start/stop scripts with start-model.sh and stop-model.sh. Documentation has been updated to reflect these new scripts and their usage, including a new section detailing their functionality and linking to it. The install.sh and remove.sh scripts have undergone significant refactoring to improve privilege handling, support non-root installations, and enhance directory management. Specifically, the csudo variable has been removed, and logic has been added to dynamically determine installation paths and user modes (root/non-root), affecting how binaries, libraries, and service files are linked and configured. The install.sh now includes options for custom installation directories and silent mode, and updates user environment variables like PATH and LD_LIBRARY_PATH for non-root users. The remove.sh script now safely identifies installation paths and includes a validate_safe_path function to prevent accidental deletion of critical system directories. In the core codebase, heartbeat handling for TMQ connections has been unified with query connections, and a 'type' column has been added to the connectionsSchema system table to distinguish connection types. Logging in mmInt.c and mndMain.c has been enhanced to provide more detailed information during MNode operations, and the syncWriteCfgFile function now includes a 'reason' parameter for better logging of configuration changes. Stream processing in vnodeStream.c received additional debug logging and a fix to ensure responses are sent for all stream reader messages. A bug in join queries was addressed by modifying planUtil.c to correctly adjust data requirements for join operations, and a new test case was added to test_join.py to validate this fix. The tdgpt configuration (taosanode.ini) was updated to use /var/lib/taos/taosanode/ as the default base directory for virtual environments and models, and new virtual environment paths were added for timesfm, moirai, chronos, and momentfm. The taosanoded.service file was updated to reflect the new virtual environment path and use uwsgi directly for starting/stopping. The tdgpt installation script (tools/tdgpt/script/install.sh) was updated to support custom installation directories, install additional virtual environments for various models (timesfm, moirai, chronos, moment), and correctly configure paths in service files and the taosanode.ini. The tdgpt release script (tools/tdgpt/script/release.sh) was modified to pack all models if specified. The tdtsfm-server.py was updated to dynamically load model weights based on configuration. Review comments highlight several issues: the install.sh script still attempts to write to system-wide directories without proper privilege checks for non-root users in install_jemalloc and install_lib, and administrative commands in deb_erase, rpm_erase, finished_install_info, updateProduct, clean_service_on_sysvinit, and install_service_on_sysvinit are called without sudo. Similarly, pip3 install commands in install_extra_venvs lack sudo for root-owned virtual environment directories. The remove.sh script also calls administrative commands without sudo and duplicates the ini_get function, which should be sourced from ini_utils.sh.
| if [ -d ${jemalloc_dir} ]; then | ||
| ${csudo}/usr/bin/install -c -d /usr/local/bin | ||
| /usr/bin/install -c -d /usr/local/bin | ||
|
|
||
| if [ -f ${jemalloc_dir}/bin/jemalloc-config ]; then | ||
| ${csudo}/usr/bin/install -c -m 755 ${jemalloc_dir}/bin/jemalloc-config /usr/local/bin | ||
| /usr/bin/install -c -m 755 ${jemalloc_dir}/bin/jemalloc-config /usr/local/bin | ||
| fi | ||
| if [ -f ${jemalloc_dir}/bin/jemalloc.sh ]; then | ||
| ${csudo}/usr/bin/install -c -m 755 ${jemalloc_dir}/bin/jemalloc.sh /usr/local/bin | ||
| /usr/bin/install -c -m 755 ${jemalloc_dir}/bin/jemalloc.sh /usr/local/bin | ||
| fi | ||
| if [ -f ${jemalloc_dir}/bin/jeprof ]; then | ||
| ${csudo}/usr/bin/install -c -m 755 ${jemalloc_dir}/bin/jeprof /usr/local/bin | ||
| /usr/bin/install -c -m 755 ${jemalloc_dir}/bin/jeprof /usr/local/bin | ||
| fi | ||
| if [ -f ${jemalloc_dir}/include/jemalloc/jemalloc.h ]; then | ||
| ${csudo}/usr/bin/install -c -d /usr/local/include/jemalloc | ||
| ${csudo}/usr/bin/install -c -m 644 ${jemalloc_dir}/include/jemalloc/jemalloc.h /usr/local/include/jemalloc | ||
| /usr/bin/install -c -d /usr/local/include/jemalloc | ||
| /usr/bin/install -c -m 644 ${jemalloc_dir}/include/jemalloc/jemalloc.h /usr/local/include/jemalloc | ||
| fi | ||
| if [ -f ${jemalloc_dir}/lib/libjemalloc.so.2 ]; then | ||
| ${csudo}/usr/bin/install -c -d /usr/local/lib | ||
| ${csudo}/usr/bin/install -c -m 755 ${jemalloc_dir}/lib/libjemalloc.so.2 /usr/local/lib | ||
| ${csudo}ln -sf libjemalloc.so.2 /usr/local/lib/libjemalloc.so | ||
| ${csudo}/usr/bin/install -c -d /usr/local/lib | ||
| /usr/bin/install -c -d /usr/local/lib | ||
| /usr/bin/install -c -m 755 ${jemalloc_dir}/lib/libjemalloc.so.2 /usr/local/lib | ||
| ln -sf libjemalloc.so.2 /usr/local/lib/libjemalloc.so | ||
| /usr/bin/install -c -d /usr/local/lib | ||
| # if [ -f ${jemalloc_dir}/lib/libjemalloc.a ]; then | ||
| # ${csudo}/usr/bin/install -c -m 755 ${jemalloc_dir}/lib/libjemalloc.a /usr/local/lib | ||
| # /usr/bin/install -c -m 755 ${jemalloc_dir}/lib/libjemalloc.a /usr/local/lib | ||
| # fi | ||
| # if [ -f ${jemalloc_dir}/lib/libjemalloc_pic.a ]; then | ||
| # ${csudo}/usr/bin/install -c -m 755 ${jemalloc_dir}/lib/libjemalloc_pic.a /usr/local/lib | ||
| # /usr/bin/install -c -m 755 ${jemalloc_dir}/lib/libjemalloc_pic.a /usr/local/lib | ||
| # fi | ||
| if [ -f ${jemalloc_dir}/lib/pkgconfig/jemalloc.pc ]; then | ||
| ${csudo}/usr/bin/install -c -d /usr/local/lib/pkgconfig | ||
| ${csudo}/usr/bin/install -c -m 644 ${jemalloc_dir}/lib/pkgconfig/jemalloc.pc /usr/local/lib/pkgconfig | ||
| /usr/bin/install -c -d /usr/local/lib/pkgconfig | ||
| /usr/bin/install -c -m 644 ${jemalloc_dir}/lib/pkgconfig/jemalloc.pc /usr/local/lib/pkgconfig | ||
| fi | ||
| fi | ||
| if [ -f ${jemalloc_dir}/share/doc/jemalloc/jemalloc.html ]; then | ||
| ${csudo}/usr/bin/install -c -d /usr/local/share/doc/jemalloc | ||
| ${csudo}/usr/bin/install -c -m 644 ${jemalloc_dir}/share/doc/jemalloc/jemalloc.html /usr/local/share/doc/jemalloc | ||
| /usr/bin/install -c -d /usr/local/share/doc/jemalloc | ||
| /usr/bin/install -c -m 644 ${jemalloc_dir}/share/doc/jemalloc/jemalloc.html /usr/local/share/doc/jemalloc | ||
| fi | ||
| if [ -f ${jemalloc_dir}/share/man/man3/jemalloc.3 ]; then | ||
| ${csudo}/usr/bin/install -c -d /usr/local/share/man/man3 | ||
| ${csudo}/usr/bin/install -c -m 644 ${jemalloc_dir}/share/man/man3/jemalloc.3 /usr/local/share/man/man3 | ||
| /usr/bin/install -c -d /usr/local/share/man/man3 | ||
| /usr/bin/install -c -m 644 ${jemalloc_dir}/share/man/man3/jemalloc.3 /usr/local/share/man/man3 | ||
| fi | ||
|
|
||
| if [ -d /etc/ld.so.conf.d ]; then | ||
| echo "/usr/local/lib" | ${csudo}tee /etc/ld.so.conf.d/jemalloc.conf >/dev/null || echo -e "failed to write /etc/ld.so.conf.d/jemalloc.conf" | ||
| ${csudo}ldconfig | ||
| else | ||
| echo "/etc/ld.so.conf.d not found!" | ||
| if [ "$osType" != "Darwin" ]; then | ||
| if [ -d /etc/ld.so.conf.d ]; then | ||
| echo "/usr/local/lib" | tee /etc/ld.so.conf.d/jemalloc.conf >/dev/null || echo -e "failed to write /etc/ld.so.conf.d/jemalloc.conf" | ||
| ldconfig | ||
| else | ||
| echo "/etc/ld.so.conf.d not found!" | ||
| fi | ||
| fi | ||
| fi | ||
| } |
There was a problem hiding this comment.
The csudo command has been removed from this function, but it still attempts to write to system-wide directories like /usr/local/bin and /etc/ld.so.conf.d. This will cause the installation to fail for non-root users, which this script refactoring is intended to support.
To fix this, the function should:
- Use the path variables defined in
setup_env(e.g.,$bin_link_dir,$lib_link_dir) instead of hardcoded paths. - Check
user_modeand only attempt operations requiring root privileges (like modifying/etc/ld.so.conf.d) when running as root.
| if echo $osinfo | grep -qwi "ubuntu"; then | ||
| # echo "this is ubuntu system" | ||
| ${csudo}dpkg --force-all -P tdengine >/dev/null 2>&1 || : | ||
| dpkg --force-all -P tdengine >/dev/null 2>&1 || : | ||
| elif echo $osinfo | grep -qwi "debian"; then | ||
| # echo "this is debian system" | ||
| ${csudo}dpkg --force-all -P tdengine >/dev/null 2>&1 || : | ||
| dpkg --force-all -P tdengine >/dev/null 2>&1 || : | ||
| elif echo $osinfo | grep -qwi "centos"; then | ||
| # echo "this is centos system" | ||
| ${csudo}rpm -e --noscripts tdengine >/dev/null 2>&1 || : | ||
| rpm -e --noscripts tdengine >/dev/null 2>&1 || : | ||
| fi |
There was a problem hiding this comment.
The script attempts to call dpkg and rpm without sudo. These are administrative commands that require root privileges and will fail for non-root users. The script should check for root privileges before executing these commands.
| function install_extra_venvs() { | ||
| echo -e "${GREEN}Creating timesfm venv at ${timesfm_venv_dir}${NC}" | ||
| if [ -d "${timesfm_venv_dir}" ]; then | ||
| echo "Removing existing timesfm venv..." | ||
| rm -rf "${timesfm_venv_dir}" | ||
| fi | ||
| "python3.${python_minor_ver}" -m venv "${timesfm_venv_dir}" | ||
| echo "Activating timesfm venv and installing dependencies..." | ||
| source "${timesfm_venv_dir}/bin/activate" | ||
| "${timesfm_venv_dir}/bin/pip3" install torch==2.3.1+cpu jax timesfm flask==3.0.3 \ | ||
| -f https://download.pytorch.org/whl/torch_stable.html | ||
| deactivate | ||
|
|
||
| echo -e "${GREEN}Creating moirai venv at ${moirai_venv_dir}${NC}" | ||
| if [ -d "${moirai_venv_dir}" ]; then | ||
| echo "Removing existing moirai venv..." | ||
| rm -rf "${moirai_venv_dir}" | ||
| fi | ||
| "python3.${python_minor_ver}" -m venv "${moirai_venv_dir}" | ||
| echo "Activating moirai venv and installing dependencies..." | ||
| source "${moirai_venv_dir}/bin/activate" | ||
| "${moirai_venv_dir}/bin/pip3" install torch==2.3.1+cpu uni2ts flask \ | ||
| -f https://download.pytorch.org/whl/torch_stable.html | ||
| deactivate | ||
|
|
||
| echo -e "${GREEN}Creating chronos venv at ${chronos_venv_dir}${NC}" | ||
| if [ -d "${chronos_venv_dir}" ]; then | ||
| echo "Removing existing chronos venv..." | ||
| rm -rf "${chronos_venv_dir}" | ||
| fi | ||
| "python3.${python_minor_ver}" -m venv "${chronos_venv_dir}" | ||
| echo "Activating chronos venv and installing dependencies..." | ||
| source "${chronos_venv_dir}/bin/activate" | ||
| "${chronos_venv_dir}/bin/pip3" install --upgrade torch==2.3.1+cpu chronos-forecasting flask \ | ||
| -f https://download.pytorch.org/whl/torch_stable.html | ||
| deactivate | ||
|
|
||
| echo -e "${GREEN}Creating momentfm venv at ${momentfm_venv_dir}${NC}" | ||
| if [ -d "${momentfm_venv_dir}" ]; then | ||
| echo "Removing existing momentfm venv..." | ||
| rm -rf "${momentfm_venv_dir}" | ||
| fi | ||
| "python3.${python_minor_ver}" -m venv "${momentfm_venv_dir}" | ||
| echo "Activating momentfm venv and installing dependencies..." | ||
| source "${momentfm_venv_dir}/bin/activate" | ||
| "${momentfm_venv_dir}/bin/pip3" install --upgrade torch==2.3.1+cpu transformers==4.33.3 numpy==1.25.2 \ | ||
| matplotlib pandas==1.5 scikit-learn flask==3.0.3 momentfm \ | ||
| -f https://download.pytorch.org/whl/torch_stable.html | ||
| deactivate | ||
|
|
||
| echo -e "${GREEN}All extra venvs created and dependencies installed.${NC}" | ||
| } |
There was a problem hiding this comment.
In install_extra_venvs, pip3 install is called without sudo (or ${csudo}). However, the virtual environment directories are created under a root-owned path (/var/lib/taos/taosanode by default). The installation of packages will fail due to permission errors if the script is not run as root. This is inconsistent with install_anode_venv which correctly uses ${csudo} for pip3 install. Please add ${csudo} to the pip3 install commands in this function for consistency and correctness.
function install_extra_venvs() {
echo -e "${GREEN}Creating timesfm venv at ${timesfm_venv_dir}${NC}"
if [ -d "${timesfm_venv_dir}" ]; then
echo "Removing existing timesfm venv..."
rm -rf "${timesfm_venv_dir}"
fi
"python3.${python_minor_ver}" -m venv "${timesfm_venv_dir}"
echo "Activating timesfm venv and installing dependencies..."
source "${timesfm_venv_dir}/bin/activate"
${csudo}"${timesfm_venv_dir}/bin/pip3" install torch==2.3.1+cpu jax timesfm flask==3.0.3 \
-f https://download.pytorch.org/whl/torch_stable.html
deactivate
echo -e "${GREEN}Creating moirai venv at ${moirai_venv_dir}${NC}"
if [ -d "${moirai_venv_dir}" ]; then
echo "Removing existing moirai venv..."
rm -rf "${moirai_venv_dir}"
fi
"python3.${python_minor_ver}" -m venv "${moirai_venv_dir}"
echo "Activating moirai venv and installing dependencies..."
source "${moirai_venv_dir}/bin/activate"
${csudo}"${moirai_venv_dir}/bin/pip3" install torch==2.3.1+cpu uni2ts flask \
-f https://download.pytorch.org/whl/torch_stable.html
deactivate
echo -e "${GREEN}Creating chronos venv at ${chronos_venv_dir}${NC}"
if [ -d "${chronos_venv_dir}" ]; then
echo "Removing existing chronos venv..."
rm -rf "${chronos_venv_dir}"
fi
"python3.${python_minor_ver}" -m venv "${chronos_venv_dir}"
echo "Activating chronos venv and installing dependencies..."
source "${chronos_venv_dir}/bin/activate"
${csudo}"${chronos_venv_dir}/bin/pip3" install --upgrade torch==2.3.1+cpu chronos-forecasting flask \
-f https://download.pytorch.org/whl/torch_stable.html
deactivate
echo -e "${GREEN}Creating momentfm venv at ${momentfm_venv_dir}${NC}"
if [ -d "${momentfm_venv_dir}" ]; then
echo "Removing existing momentfm venv..."
rm -rf "${momentfm_venv_dir}"
fi
"python3.${python_minor_ver}" -m venv "${momentfm_venv_dir}"
echo "Activating momentfm venv and installing dependencies..."
source "${momentfm_venv_dir}/bin/activate"
${csudo}"${momentfm_venv_dir}/bin/pip3" install --upgrade torch==2.3.1+cpu transformers==4.33.3 numpy==1.25.2 \
matplotlib pandas==1.5 scikit-learn flask==3.0.3 momentfm \
-f https://download.pytorch.org/whl/torch_stable.html
deactivate
echo -e "${GREEN}All extra venvs created and dependencies installed.${NC}"
}| function clean_service_on_sysvinit() { | ||
| if ps aux | grep -v grep | grep $1 &>/dev/null; then | ||
| ${csudo}service $1 stop || : | ||
| service $1 stop || : | ||
| fi | ||
|
|
||
| if ((${initd_mod} == 1)); then | ||
| if [ -e ${service_config_dir}/$1 ]; then | ||
| ${csudo}chkconfig --del $1 || : | ||
| chkconfig --del $1 || : | ||
| fi | ||
| elif ((${initd_mod} == 2)); then | ||
| if [ -e ${service_config_dir}/$1 ]; then | ||
| ${csudo}insserv -r $1 || : | ||
| insserv -r $1 || : | ||
| fi | ||
| elif ((${initd_mod} == 3)); then | ||
| if [ -e ${service_config_dir}/$1 ]; then | ||
| ${csudo}update-rc.d -f $1 remove || : | ||
| update-rc.d -f $1 remove || : | ||
| fi | ||
| fi | ||
|
|
||
| ${csudo}rm -f ${service_config_dir}/$1 || : | ||
| rm -f ${service_config_dir}/$1 || : | ||
|
|
||
| if $(which init &>/dev/null); then | ||
| ${csudo}init q || : | ||
| init q || : | ||
| fi | ||
| } |
There was a problem hiding this comment.
The clean_service_on_sysvinit function calls administrative commands like service, chkconfig, insserv, and update-rc.d without sudo. These commands require root privileges and will fail for non-root users on systems using SysVInit. The script should check for root privileges before executing these commands.
| if ((${os_type} == 1)); then | ||
| ${csudo}cp ${script_dir}/init.d/${serverName}.deb ${service_config_dir}/${serverName} && ${csudo}chmod a+x ${service_config_dir}/${serverName} | ||
| cp ${script_dir}/init.d/${serverName}.deb ${service_config_dir}/${serverName} && chmod a+x ${service_config_dir}/${serverName} | ||
| elif ((${os_type} == 2)); then | ||
| ${csudo}cp ${script_dir}/init.d/${serverName}.rpm ${service_config_dir}/${serverName} && ${csudo}chmod a+x ${service_config_dir}/${serverName} | ||
| cp ${script_dir}/init.d/${serverName}.rpm ${service_config_dir}/${serverName} && chmod a+x ${service_config_dir}/${serverName} | ||
| fi | ||
|
|
||
| if ((${initd_mod} == 1)); then | ||
| ${csudo}chkconfig --add $1 || : | ||
| ${csudo}chkconfig --level 2345 $1 on || : | ||
| chkconfig --add $1 || : | ||
| chkconfig --level 2345 $1 on || : | ||
| elif ((${initd_mod} == 2)); then | ||
| ${csudo}insserv $1} || : | ||
| ${csudo}insserv -d $1 || : | ||
| insserv $1} || : | ||
| insserv -d $1 || : | ||
| elif ((${initd_mod} == 3)); then | ||
| ${csudo}update-rc.d $1 defaults || : | ||
| update-rc.d $1 defaults || : | ||
| fi |
There was a problem hiding this comment.
The install_service_on_sysvinit function calls administrative commands like chkconfig, insserv, and update-rc.d without sudo. These commands require root privileges and will fail for non-root users on systems using SysVInit. The script should check for root privileges before executing these commands.
| clean_service_on_sysvinit_of() { | ||
| _service=$1 | ||
| if pidof ${_service} &>/dev/null; then | ||
| echo "${_service} is running, stopping it..." | ||
| ${csudo}service ${_service} stop || : | ||
| service ${_service} stop || : | ||
| fi | ||
| if ((${initd_mod} == 1)); then | ||
| if [ -e ${service_config_dir}/${_service} ]; then | ||
| ${csudo}chkconfig --del ${_service} || : | ||
| chkconfig --del ${_service} || : | ||
| fi | ||
| elif ((${initd_mod} == 2)); then | ||
| if [ -e ${service_config_dir}/${_service} ]; then | ||
| ${csudo}insserv -r ${_service} || : | ||
| insserv -r ${_service} || : | ||
| fi | ||
| elif ((${initd_mod} == 3)); then | ||
| if [ -e ${service_config_dir}/${_service} ]; then | ||
| ${csudo}update-rc.d -f ${_service} remove || : | ||
| update-rc.d -f ${_service} remove || : | ||
| fi | ||
| fi | ||
|
|
||
| ${csudo}rm -f ${service_config_dir}/${_service} || : | ||
| rm -f ${service_config_dir}/${_service} || : | ||
|
|
||
| if $(which init &>/dev/null); then | ||
| ${csudo}init q || : | ||
| init q || : | ||
| fi | ||
| } |
There was a problem hiding this comment.
The clean_service_on_sysvinit_of function calls administrative commands like service, chkconfig, insserv, and update-rc.d without sudo. These commands require root privileges and will fail when the script is run by a non-root user. The script should check for root privileges before attempting these operations.
| ini_get() { | ||
| local section="$1" key="$2" default="${3-}" file="$CONFIG_FILE" | ||
| local in_section=0 line k v | ||
| while IFS= read -r line || [ -n "$line" ]; do | ||
| line="${line%%\#*}"; line="${line%%;*}" | ||
| line="$(printf '%s' "$line" | sed -e 's/^[[:space:]]\+//' -e 's/[[:space:]]\+$//')" | ||
| [ -z "$line" ] && continue | ||
| if [[ "$line" =~ ^\[(.+)\]$ ]]; then | ||
| [[ "${BASH_REMATCH[1]}" == "$section" ]] && in_section=1 || in_section=0 | ||
| continue | ||
| fi | ||
| if [[ $in_section -eq 1 && "$line" =~ ^([^=]+)=[[:space:]]*(.*)$ ]]; then | ||
| k="$(printf '%s' "${BASH_REMATCH[1]}" | sed 's/[[:space:]]//g')" | ||
| v="${BASH_REMATCH[2]}" | ||
| v="${v%\"}"; v="${v#\"}"; v="${v%\'}"; v="${v#\'}" | ||
| if [[ "$k" == "$key" ]]; then | ||
| printf '%s\n' "$v" | ||
| return 0 | ||
| fi | ||
| fi | ||
| done < "$file" | ||
| printf '%s\n' "$default" | ||
| } |
There was a problem hiding this comment.
The ini_get function is duplicated from the new ini_utils.sh script. To avoid code duplication and improve maintainability, this script should source ini_utils.sh and use the function from there. You could add source "$(dirname "$(readlink -f "$0")")/ini_utils.sh" at the top of the script.
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.