Bump 1.2.2#96
Conversation
… kill process in finish script
Signed-off-by: Hyungyu Kim <kimhg@robotis.com>
…feature-zenoh-updates
Signed-off-by: Hyungyu Kim <kimhg@robotis.com>
…5, bh5) and improve error messaging for robot type validation.
…move branch specification from cyclo_manager git clone command.
…to ensure packages are up-to-date before installation.
…on to avoid BuildKit UndefinedVar warning and ensure apt-get upgrade is executed before package installation.
…plify container management by consolidating commands for starting and stopping the container.
…to streamline configuration and ensure proper newline at the end of container.sh file.
…o streamline package installation process.
… for arm64 to streamline package management.
1.2.2 unified s6 service
![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 updates the AI worker environment to version 1.2.2, primarily replacing the talos_system_manager with cyclo_manager and unifying ROS 2 services into ai_worker_bringup and avatar_bringup. The Docker management script has been simplified by removing noVNC-oriented workflows and ARM-specific host checks. Feedback focuses on improving the robustness of the Dockerfiles and service scripts, specifically by avoiding interactive shells (bash -i) for background services to prevent logging issues, ensuring PYTHONPATH is appended rather than overwritten, and pinning git clones to specific branches to ensure build reproducibility.
| # which allows s6 to properly signal it and its children | ||
| # Note: stdout/stderr are automatically piped to ${SERVICE_NAME}-log via producer-for/consumer-for | ||
| exec bash -c "${ROS2_CMD}" | ||
| exec bash -i -c "${ROS2_CMD}" |
There was a problem hiding this comment.
Using an interactive shell (bash -i) for a background service is generally discouraged. It can cause issues with job control, lead to 'Inappropriate ioctl for device' warnings in logs, and makes the service behavior dependent on the user's .bashrc. Since the ROS environment is explicitly sourced in this script (lines 39-40), an interactive shell is not required.
| exec bash -i -c "${ROS2_CMD}" | |
| exec bash -c "${ROS2_CMD}" |
| COPY --from=librealsense /usr/src/librealsense/config/99-realsense-libusb.rules /etc/udev/rules.d/ | ||
| COPY --from=librealsense /usr/src/librealsense/config/99-realsense-d4xx-mipi-dfu.rules /etc/udev/rules.d/ | ||
| ENV PYTHONPATH=$PYTHONPATH:/usr/local/lib | ||
| ENV PYTHONPATH=/usr/local/lib |
There was a problem hiding this comment.
Overwriting PYTHONPATH without including the existing value can lead to issues if the base image or previous layers have already set it. It is safer to append or prepend to the existing variable.
ENV PYTHONPATH=${PYTHONPATH}:/usr/local/lib
| RUN git clone https://github.com/ROBOTIS-GIT/talos_system_manager.git /opt/talos_system_manager | ||
|
|
||
| ENV PYTHONPATH=/opt/talos_system_manager:${PYTHONPATH} | ||
| RUN git clone https://github.com/ROBOTIS-GIT/cyclo_manager.git /opt/cyclo_manager |
There was a problem hiding this comment.
Cloning a repository without specifying a branch or tag can lead to non-deterministic builds if the default branch changes. Consider pinning to a specific branch (e.g., jazzy) or a commit hash, consistent with other clones in this file.
RUN git clone -b jazzy https://github.com/ROBOTIS-GIT/cyclo_manager.git /opt/cyclo_manager
| COPY --from=librealsense-builder /usr/src/librealsense/config/99-realsense-libusb.rules /etc/udev/rules.d/ | ||
| COPY --from=librealsense-builder /usr/src/librealsense/config/99-realsense-d4xx-mipi-dfu.rules /etc/udev/rules.d/ | ||
| ENV PYTHONPATH=$PYTHONPATH:/usr/local/lib | ||
| ENV PYTHONPATH=/usr/local/lib |
There was a problem hiding this comment.
Overwriting PYTHONPATH without including the existing value can lead to issues if the base image or previous layers have already set it. It is safer to append or prepend to the existing variable.
ENV PYTHONPATH=${PYTHONPATH}:/usr/local/lib
| chmod 777 /var/run/agent | ||
|
|
||
| RUN git clone https://github.com/ROBOTIS-GIT/talos_system_manager.git /opt/talos_system_manager | ||
| RUN git clone https://github.com/ROBOTIS-GIT/cyclo_manager.git /opt/cyclo_manager |
There was a problem hiding this comment.
Cloning a repository without specifying a branch or tag can lead to non-deterministic builds if the default branch changes. Consider pinning to a specific branch (e.g., jazzy) or a commit hash, consistent with other clones in this file.
RUN git clone -b jazzy https://github.com/ROBOTIS-GIT/cyclo_manager.git /opt/cyclo_manager
| if [ -f "${COMMON_SCRIPT}" ]; then | ||
| export SERVICE_NAME="${SERVICE_NAME}" | ||
| export ROS2_COMMAND="${ROS2_COMMAND}" | ||
| exec /command/with-contenv env SERVICE_NAME="${SERVICE_NAME}" ROS2_COMMAND="${ROS2_COMMAND}" bash -i -c "${COMMON_SCRIPT}" |
There was a problem hiding this comment.
Using bash -i -c to execute a script that already has a shebang and is intended for a non-interactive environment is unnecessary and can cause side effects. It is better to execute the script directly.
exec /command/with-contenv env SERVICE_NAME="${SERVICE_NAME}" ROS2_COMMAND="${ROS2_COMMAND}" bash "${COMMON_SCRIPT}"
| # Use env to explicitly pass variables to the script | ||
| # The with-contenv wrapper will preserve these variables | ||
| exec /command/with-contenv env SERVICE_NAME="${SERVICE_NAME}" ROS2_COMMAND="${ROS2_COMMAND}" bash "${COMMON_SCRIPT}" | ||
| exec /command/with-contenv env SERVICE_NAME="${SERVICE_NAME}" ROS2_COMMAND="${ROS2_COMMAND}" bash -i -c "${COMMON_SCRIPT}" |
There was a problem hiding this comment.
Using bash -i -c to execute a script that already has a shebang and is intended for a non-interactive environment is unnecessary and can cause side effects. It is better to execute the script directly.
exec /command/with-contenv env SERVICE_NAME="${SERVICE_NAME}" ROS2_COMMAND="${ROS2_COMMAND}" bash "${COMMON_SCRIPT}"
2 participants
No description provided.