Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Update list CLI options #1678

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
diemol merged 2 commits into SeleniumHQ:trunk from NDViet:trunk
Apr 19, 2024
Merged

Update list CLI options #1678

diemol merged 2 commits into SeleniumHQ:trunk from NDViet:trunk
Apr 19, 2024

Conversation

@VietND96
Copy link
Member

@VietND96 VietND96 commented Apr 19, 2024 •
edited by qodo-merge-pro bot
Loading

User description

Thanks for contributing to the Selenium site and documentation!
A PR well described will help maintainers to review and merge it quickly

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

Description

Motivation and Context

Add new CLI option mentioned in PR SeleniumHQ/selenium#13804
Add 2 more CLI options are available in latest version but not mentioned yet

Types of changes

  • Change to the site (I have double-checked the Netlify deployment, and my changes look good)
  • Code example added (and I also added the example to all translated languages)
  • Improved translation
  • Added new translation (and I also added a notice to each document missing translation)

Checklist

  • I have read the contributing document.
  • I have used hugo to render the site/docs locally and I am sure it works.

Type

enhancement, documentation

Description

  • Added new CLI option --newsession-threadpool-size across multiple language documents to allow configuration of the thread pool size for session creation.
  • Introduced --docker-host-config-keys CLI option to specify which Docker host configuration keys should be passed to browser containers.
  • Implemented --disable-ui CLI option to allow disabling the Grid UI.

Changes walkthrough

Relevant files
Enhancement
cli_options.en.md
Add new CLI options for session management, Docker, and UI control

website_and_docs/content/documentation/grid/configuration/cli_options.en.md

  • Added new CLI option --newsession-threadpool-size for configuring
    thread pool size.
  • Added new CLI option --docker-host-config-keys to specify Docker host
    configuration keys.
  • Added new CLI option --disable-ui to disable the Grid UI.
    • +3/-0
    cli_options.ja.md
    Add new CLI options for session management, Docker, and UI control

    website_and_docs/content/documentation/grid/configuration/cli_options.ja.md

  • Added new CLI option --newsession-threadpool-size for configuring
    thread pool size.
  • Added new CLI option --docker-host-config-keys to specify Docker host
    configuration keys.
  • Added new CLI option --disable-ui to disable the Grid UI.
    • +3/-0
    cli_options.pt-br.md
    Add new CLI options for session management, Docker, and UI control

    website_and_docs/content/documentation/grid/configuration/cli_options.pt-br.md

  • Added new CLI option --newsession-threadpool-size for configuring
    thread pool size.
  • Added new CLI option --docker-host-config-keys to specify Docker host
    configuration keys.
  • Added new CLI option --disable-ui to disable the Grid UI.
    • +3/-0
    cli_options.zh-cn.md
    Add new CLI options for session management, Docker, and UI control

    website_and_docs/content/documentation/grid/configuration/cli_options.zh-cn.md

  • Added new CLI option --newsession-threadpool-size for configuring
    thread pool size.
  • Added new CLI option --docker-host-config-keys to specify Docker host
    configuration keys.
  • Added new CLI option --disable-ui to disable the Grid UI.
    • +3/-0

    ✨ PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    netlify bot commented Apr 19, 2024 •
    edited
    Loading

    ✅ Deploy Preview for selenium-dev ready!

    Name Link
    🔨 Latest commit a995500
    🔍 Latest deploy log https://app.netlify.com/sites/selenium-dev/deploys/66222288c7948100082754ea
    😎 Deploy Preview https://deploy-preview-1678--selenium-dev.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @qodo-merge-pro qodo-merge-pro bot added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 19, 2024
    Copy link
    Contributor

    PR Description updated to latest commit (a9a46c6)

    Copy link
    Contributor

    PR Review

    ⏱️ Estimated effort to review [1-5]

    2, because the changes are straightforward and involve adding new CLI options to documentation files across multiple languages. The modifications are consistent and well-documented within the context of the existing structure.

    🧪 Relevant tests

    No

    🔍 Possible issues

    Possible Inconsistency: The description for --newsession-threadpool-size mentions that the default value is "no. of available processors * 3", but it's not clear if this calculation is automatically handled by the software or if it needs manual configuration. Clarification in the documentation might be needed to avoid user confusion.

    🔒 Security concerns

    No
    ✨ Review tool usage guide:

    Overview:
    The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

    The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

    • When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:
    /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

    [pr_reviewer]
some_config1=...
some_config2=...

    See the review usage page for a comprehensive guide on using this tool.
    Copy link
    Contributor

    PR Code Suggestions

    CategorySuggestions
    Enhancement
    Add a detailed explanation or example for configuring the thread pool size.

    Consider adding a more detailed explanation or example for the
    --newsession-threadpool-size option to help users understand how to choose an optimal
    thread pool size based on their specific hardware and workload conditions.

    website_and_docs/content/documentation/grid/configuration/cli_options.en.md [165] 

    -| `--newsession-threadpool-size` | int | `24` | The Distributor uses a fixed-sized thread pool to create new sessions as it consumes new session requests from the queue. This allows configuring the size of the thread pool. The default value is no. of available processors * 3. Note: If the no. of threads is way greater than the available processors it will not always increase the performance. A high number of threads causes more context switching which is an expensive operation. |
+| `--newsession-threadpool-size` | int | `24` | The Distributor uses a fixed-sized thread pool to create new sessions as it consumes new session requests from the queue. This allows configuring the size of the thread pool based on specific hardware and workload conditions. The default value is no. of available processors * 3. Note: If the no. of threads is way greater than the available processors it will not always increase the performance. A high number of threads causes more context switching which is an expensive operation. Example: For a system with 4 processors, a pool size of 12 might be optimal. |
    Provide practical examples for using docker host configuration keys.

    It would be beneficial to clarify the documentation for --docker-host-config-keys by
    providing examples of how each key can be used in practice, enhancing the user's
    understanding and application of these settings.

    website_and_docs/content/documentation/grid/configuration/cli_options.en.md [178] 

    -| `--docker-host-config-keys` | string[] | `Dns DnsOptions DnsSearch ExtraHosts Binds` | Specify which docker host configuration keys should be passed to browser containers. Keys name can be found in the Docker API [documentation](https://docs.docker.com/engine/api/v1.41/#tag/Container/operation/ContainerCreate), or by running `docker inspect` the node-docker container. |
+| `--docker-host-config-keys` | string[] | `Dns DnsOptions DnsSearch ExtraHosts Binds` | Specify which docker host configuration keys should be passed to browser containers. For example, `DnsOptions` can be used to configure DNS options for containers. More details and examples can be found in the Docker API [documentation](https://docs.docker.com/engine/api/v1.41/#tag/Container/operation/ContainerCreate), or by running `docker inspect` on the node-docker container. |
    Clarity
    Clarify the downsides of excessive thread counts in the thread pool size documentation.

    To avoid potential confusion, consider rephrasing the note in --newsession-threadpool-size
    to explicitly state the potential downsides of having a thread count significantly higher
    than the number of processors, such as increased latency or higher resource consumption.

    website_and_docs/content/documentation/grid/configuration/cli_options.en.md [165] 

    -| `--newsession-threadpool-size` | int | `24` | The Distributor uses a fixed-sized thread pool to create new sessions as it consumes new session requests from the queue. This allows configuring the size of the thread pool. The default value is no. of available processors * 3. Note: If the no. of threads is way greater than the available processors it will not always increase the performance. A high number of threads causes more context switching which is an expensive operation. |
+| `--newsession-threadpool-size` | int | `24` | The Distributor uses a fixed-sized thread pool to create new sessions as it consumes new session requests from the queue. This allows configuring the size of the thread pool. The default value is no. of available processors * 3. Note: Excessively high thread counts compared to processor availability can lead to increased latency and resource consumption due to frequent context switching. |
    Improve the presentation of the --newsession-threadpool-size option for better readability and understanding.

    To enhance clarity and user understanding, consider breaking the lengthy explanation in
    the --newsession-threadpool-size option into bullet points or a list format, highlighting
    key points such as default values, performance implications, and operational costs.

    website_and_docs/content/documentation/grid/configuration/cli_options.en.md [165] 

    -| `--newsession-threadpool-size` | int | `24` | The Distributor uses a fixed-sized thread pool to create new sessions as it consumes new session requests from the queue. This allows configuring the size of the thread pool. The default value is no. of available processors * 3. Note: If the no. of threads is way greater than the available processors it will not always increase the performance. A high number of threads causes more context switching which is an expensive operation. |
+| `--newsession-threadpool-size` | int | `24` | The Distributor uses a fixed-sized thread pool to create new sessions. Key points:
+- **Default Size**: No. of available processors * 3.
+- **Performance**: Excessive threads may not increase performance and can lead to costly context switching.
+- **Configuration**: Allows customization based on specific needs. |
    Security
    Add a security warning for the configuration of Docker host keys.

    For the --docker-host-config-keys option, consider adding a warning or note about the
    security implications of improperly configuring Docker host keys, which could potentially
    expose sensitive data or compromise container security.

    website_and_docs/content/documentation/grid/configuration/cli_options.en.md [178] 

    -| `--docker-host-config-keys` | string[] | `Dns DnsOptions DnsSearch ExtraHosts Binds` | Specify which docker host configuration keys should be passed to browser containers. Keys name can be found in the Docker API [documentation](https://docs.docker.com/engine/api/v1.41/#tag/Container/operation/ContainerCreate), or by running `docker inspect` the node-docker container. |
+| `--docker-host-config-keys` | string[] | `Dns DnsOptions DnsSearch ExtraHosts Binds` | Specify which docker host configuration keys should be passed to browser containers. Be cautious as improper configuration can expose sensitive data or compromise container security. More details in the Docker API [documentation](https://docs.docker.com/engine/api/v1.41/#tag/Container/operation/ContainerCreate), or by running `docker inspect` on the node-docker container. |
    ✨ Improve tool usage guide:

    Overview:
    The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

    • When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:
    /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

    [pr_code_suggestions]
some_config1=...
some_config2=...

    See the improve usage page for a comprehensive guide on using this tool.

    Copy link
    Member

    @diemol diemol left a comment

    Choose a reason for hiding this comment

    The reason will be displayed to describe this comment to others. Learn more.

    Thank you, @VietND96!

    @diemol diemol merged commit b78f192 into SeleniumHQ:trunk Apr 19, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Reviewers

    @diemol diemol diemol approved these changes

    Assignees

    No one assigned

    Labels

    documentation Improvements or additions to documentation enhancement New feature or request Review effort [1-5]: 2

    Projects

    None yet

    Milestone

    No milestone

    Development

    Successfully merging this pull request may close these issues.

    2 participants

    AltStyle によって変換されたページ (->オリジナル) /