You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
To enable it with Database backed Session Map, also install PostgreSQL service in the same namespace as Selenium Grid. You can set the following values:
The action failed due to multiple test failures in the Selenium tests. The specific issues encountered were:
The test test_grid_is_up failed with a TypeError because it attempted to subscript a NoneType object.
Several Selenium tests, including test_parallel_autoscaling, test_play_video, test_with_frames, test_title, and others, failed with a WebDriverException indicating a "503 Service Temporarily Unavailable" error from the server.
These errors suggest that the Selenium Grid or related services were not available or properly configured during the test execution.
Sensitive information exposure: Database credentials (username/password) are exposed in multiple files:
docker-compose-v3-full-grid-external-datastore.yml has hardcoded PostgreSQL credentials
charts/selenium-grid/values.yaml contains default database credentials
These should be moved to secrets management or environment variables.
⚡ Recommended focus areas for review
Version Lock The PostgreSQL JDBC driver version is hardcoded. Consider making it configurable or using a version range to allow flexibility in updates.
Error Handling The script lacks error handling for external datastore connection failures. Should add checks and appropriate error messages.
Security Risk Default database credentials are hardcoded in the compose file. Should be moved to environment variables or secrets.
if [ "${SE_SESSIONS_MAP_EXTERNAL_DATASTORE}" = "true" ]; then
+ if [ -z "${SE_SESSIONS_MAP_EXTERNAL_IMPLEMENTATION}" ]; then+ echo "Error: SE_SESSIONS_MAP_EXTERNAL_IMPLEMENTATION is required when external datastore is enabled"+ exit 1+ fi
if [[ -n "${SE_SESSIONS_MAP_EXTERNAL_SCHEME}" ]]; then
echo "scheme = \"${SE_SESSIONS_MAP_EXTERNAL_SCHEME}\"" >>"$FILENAME"
fi
Apply this suggestion
Suggestion importance[1-10]: 8
Why: The suggestion adds critical validation for required environment variables, preventing silent failures that could be hard to diagnose in production.
8
Add file existence check before reading configuration file to prevent script failure
Add error handling to verify that CONFIG_FILE exists before attempting to read it with cat. The current code assumes the file exists which could cause the script to fail.
Why: The suggestion adds important error handling to prevent script failures when the config file is missing, which is a common source of runtime errors in shell scripts.
7
Improve string concatenation logic to handle empty variables correctly
Fix the string concatenation in EXTRA_LIBS assignment to prevent potential empty string concatenation issues when EXTERNAL_JARS is empty.
Why: The suggestion improves the robustness of the string concatenation by properly handling empty variables, preventing potential issues with malformed paths.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Fixes #2472
Following https://www.selenium.dev/documentation/grid/advanced_features/external_datastore/ - now can easily to config and deploy on Docker or Kubernetes (via Helm configs)
To enable it with Database backed Session Map, also install PostgreSQL service in the same namespace as Selenium Grid. You can set the following values:
To enable it with Redis backed Session Map, also install Redis service in the same namespace as Selenium Grid. You can set the following values:
Motivation and Context
Types of changes
Checklist