Skip to content

Docker: Add missing package to Python venv #2917

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

Merged
merged 1 commit into from
Aug 3, 2025
Merged

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Aug 3, 2025

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Fix missing package in Video container

  browser_video-1  | Traceback (most recent call last):
  browser_video-1  |   File "/opt/bin/validate_endpoint.py", line 10, in <module>
  browser_video-1  |     import requests
  browser_video-1  | ModuleNotFoundError: No module named 'requests'
  browser_video-1  | Traceback (most recent call last):
  browser_video-1  |   File "/opt/bin/validate_endpoint.py", line 10, in <module>
  browser_video-1  |     import requests
  browser_video-1  | ModuleNotFoundError: No module named 'requests'

Motivation and Context

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Bug fix


Description

  • Fix missing requests package in Python virtual environment

  • Move requests installation from system pip to venv pip

  • Resolve ModuleNotFoundError in video container validation script


Diagram Walkthrough

flowchart LR
  A["System pip install"] -- "remove requests" --> B["System packages only"]
  C["Virtual env pip install"] -- "add requests" --> D["Complete venv packages"]
  B --> E["Fixed Docker container"]
  D --> E
Loading

File Walkthrough

Relevant files
Bug fix
Dockerfile
Move requests package to virtual environment                         

Base/Dockerfile

  • Remove requests package from system pip installation command
  • Add requests package to virtual environment pip installation
  • Ensure requests is available in the correct Python environment
+2/-2     

Copy link
Contributor

qodo-merge-pro bot commented Aug 3, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Package Consistency

The virtualenv package is being installed in both system pip (line 80) and virtual environment pip (line 190), which may be redundant. Consider if virtualenv is needed in the venv since it's already available system-wide.

    && python3 -m pip install --upgrade setuptools virtualenv --break-system-packages \
    && rm -rf /var/lib/apt/lists/* /var/cache/apt/* \
    && echo "source $VENV_PATH/bin/activate" >> /etc/bash.bashrc

RUN ARCH=$(if [ "$(dpkg --print-architecture)" = "arm64" ]; then echo "aarch64"; else echo "$(dpkg --print-architecture)"; fi) \
    && wget -q https://github.com/NDViet/static-curl/releases/download/${CURL_VERSION}/curl-$ARCH -O /usr/bin/curl \
    && chmod +x /usr/bin/curl \
    && curl --version

RUN --mount=type=secret,id=SEL_PASSWD \
    if [ "${TARGETARCH}" = "arm" ] && [ "${TARGETVARIANT}" = "v7" ]; then \
       export ARCH=armhf ; \
    else \
       export ARCH=$(dpkg --print-architecture) ; \
    fi \
  && sed -i 's/securerandom\.source=file:\/dev\/random/securerandom\.source=file:\/dev\/urandom/' /usr/lib/jvm/java-${JRE_VERSION}-openjdk-${ARCH}/conf/security/java.security \
#===================
# Timezone settings
# Possible alternative: https://github.com/docker/docker/issues/3359#issuecomment-32150214
#===================
  && ln -fs /usr/share/zoneinfo/${TZ} /etc/localtime && \
    dpkg-reconfigure -f noninteractive tzdata && \
    cat /etc/timezone \
#========================================
# Add normal user and group without password sudo
#========================================
  && groupadd ${SEL_GROUP} \
         --gid ${SEL_GID} \
  && useradd ${SEL_USER} \
         --create-home \
         --gid ${SEL_GID} \
         --shell /bin/bash \
         --uid ${SEL_UID} \
  && usermod -a -G sudo ${SEL_USER} \
  && echo 'ALL ALL = (ALL) NOPASSWD: ALL' >> /etc/sudoers \
  && echo "${SEL_USER}:$(cat /run/secrets/SEL_PASSWD)" | chpasswd \
#==========
# Selenium & relaxing permissions for OpenShift and other non-sudo environments
#==========
  && mkdir -p /opt/selenium /opt/selenium/assets /opt/selenium/secrets /opt/selenium/logs /var/run/supervisor /var/log/supervisor ${SEL_DOWNLOAD_DIR} \
    ${HOME}/.mozilla ${HOME}/.vnc ${HOME}/.pki/nssdb ${VIDEO_FOLDER} \
  # NSSDB initialization with an empty password
  && certutil -d sql:${HOME}/.pki/nssdb -N --empty-password \
  && touch ${CONFIG_FILE} \
  && chown -R ${SEL_USER}:${SEL_GROUP} /opt/selenium /var/run/supervisor /var/log/supervisor /etc/passwd ${HOME} ${VIDEO_FOLDER} \
  && chmod -R 775 /opt/selenium /var/run/supervisor /var/log/supervisor /etc/passwd ${HOME} ${VIDEO_FOLDER} \
  && wget --no-verbose https://github.com/${AUTHORS}/selenium/releases/download/${RELEASE}/selenium-server-${VERSION}.jar \
    -O /opt/selenium/selenium-server.jar \
  && chgrp -R 0 /opt/selenium ${HOME} ${VIDEO_FOLDER} /opt/selenium/assets /var/run/supervisor /var/log/supervisor \
  && chmod -R g=u /opt/selenium ${HOME} ${VIDEO_FOLDER} /opt/selenium/assets /var/run/supervisor /var/log/supervisor \
  && setfacl -Rm u:${SEL_USER}:rwx /opt /opt/selenium ${HOME} ${VIDEO_FOLDER} /opt/selenium/assets /var/run/supervisor /var/log/supervisor \
  && setfacl -Rm g:${SEL_GROUP}:rwx /opt /opt/selenium ${HOME} ${VIDEO_FOLDER} /opt/selenium/assets /var/run/supervisor /var/log/supervisor \
#=====
# Download observability related OpenTelemetry jars and make them available in a separate directory
# so that the container can skip downloading them everytime it comes up
#===== \
  && if [ `arch` = "aarch64" ] || [ `arch` = "x86_64" ]; then \
        curl -fL https://github.com/coursier/coursier/releases/download/v${CS_VERSION}/coursier.jar > /tmp/cs \
        && chmod +x /tmp/cs \
        && mkdir -p /external_jars \
        && chmod -R 775 /external_jars ; \
     fi \
  && if [ -f "/tmp/cs" ]; then \
        java -jar /tmp/cs fetch --classpath --cache /external_jars \
        io.opentelemetry:opentelemetry-exporter-otlp:${OPENTELEMETRY_VERSION} \
        io.grpc:grpc-netty:${GRPC_VERSION} \
        io.netty:netty-codec-http:${NETTY_VERSION} \
        io.netty:netty-handler:${NETTY_VERSION} \
        io.netty:netty-common:${NETTY_VERSION} \
        > /external_jars/.classpath.txt \
        && chmod 664 /external_jars/.classpath.txt ; \
     fi \
  && rm -fr /root/.cache/* \
  # (Note that .bashrc is only executed in interactive bash shells.)
  && echo 'if [[ $(ulimit -n) -gt 200000 ]]; then echo "WARNING: Very high value reported by \"ulimit -n\". Consider passing \"--ulimit nofile=32768\" to \"docker run\"."; fi' >> ${HOME}/.bashrc

#======================================
# Add Grid check script
#======================================
COPY --chown="${SEL_UID}:${SEL_GID}" check-grid.sh entry_point.sh configs/node/nodeGridUrl.sh configs/node/nodePreStop.sh handle_heap_dump.sh /opt/bin/
COPY --chown="${SEL_UID}:${SEL_GID}" mask /usr/local/bin/
RUN chmod +x /opt/bin/*.sh /usr/local/bin/mask

#======================================
# Add Supervisor configuration file
#======================================
COPY supervisord.conf /etc

#===================================================
# Add the default self-signed certificate to the bundle CA
#===================================================
ARG CERT_TRUST_ATTR=TCu,Cu,Tu
COPY --chown="${SEL_UID}:${SEL_GID}" certs/add-cert-helper.sh certs/add-jks-helper.sh /opt/bin/
COPY --chown="${SEL_UID}:${SEL_GID}" certs/tls.crt certs/tls.key certs/server.jks certs/server.pass /opt/selenium/secrets/

#===================================================
# Add envsubst binary
#===================================================
RUN ARCH=$(if [ "$(dpkg --print-architecture)" = "amd64" ]; then echo "x86_64"; else echo "$(dpkg --print-architecture)"; fi) \
    && curl -fsSL https://github.com/ndviet/envsubst/releases/download/v${ENVSUBST_VERSION}/envsubst-$(uname -s)-${ARCH} -o envsubst \
    && chmod +x envsubst \
    && mv envsubst /usr/local/bin \
    && ln -sf /usr/local/bin/envsubst /usr/bin/envsubst

#===================================================
# Run the following commands as non-privileged user
#===================================================
USER ${SEL_UID}:${SEL_GID}

RUN python3 -m venv $VENV_PATH \
    && $VENV_PATH/bin/python3 -m pip install --upgrade pip setuptools virtualenv psutil requests \

Copy link
Contributor

qodo-merge-pro bot commented Aug 3, 2025

PR Code Suggestions ✨

No code suggestions found for the PR.

@VietND96 VietND96 merged commit 1316d5f into trunk Aug 3, 2025
27 checks passed
@VietND96 VietND96 deleted the missing-package branch August 3, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant