-
Notifications
You must be signed in to change notification settings - Fork 147
Fix issues #544 and #364: Add --extra-docker-config-file and --docker… #604
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
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -278,6 +278,15 @@ def _migrate_csharp_csproj(project_dir: Path) -> None: | |||||||||
| default="{}", | ||||||||||
| help="Extra docker configuration as a JSON string. " | ||||||||||
| "For more information https://docker-py.readthedocs.io/en/stable/containers.html") | ||||||||||
| @option("--extra-docker-config-file", | ||||||||||
| type=PathParameter(exists=True, file_okay=True, dir_okay=False), | ||||||||||
| help="Path to a JSON file with extra docker configuration. " | ||||||||||
| "This is recommended over --extra-docker-config on Windows to avoid shell quote issues.") | ||||||||||
| @option("--docker-timeout", | ||||||||||
| type=int, | ||||||||||
| help="Docker client timeout in seconds (default: 60). " | ||||||||||
| "Increase this for slow connections or large image pulls. " | ||||||||||
| "Can also be set via DOCKER_CLIENT_TIMEOUT environment variable.") | ||||||||||
| @option("--no-update", | ||||||||||
| is_flag=True, | ||||||||||
| default=False, | ||||||||||
|
|
@@ -298,6 +307,8 @@ def backtest(project: Path, | |||||||||
| addon_module: Optional[List[str]], | ||||||||||
| extra_config: Optional[Tuple[str, str]], | ||||||||||
| extra_docker_config: Optional[str], | ||||||||||
| extra_docker_config_file: Optional[Path], | ||||||||||
| docker_timeout: Optional[int], | ||||||||||
| no_update: bool, | ||||||||||
| parameter: List[Tuple[str, str]], | ||||||||||
| **kwargs) -> None: | ||||||||||
|
|
@@ -316,9 +327,13 @@ def backtest(project: Path, | |||||||||
| Alternatively you can set the default engine image for all commands using `lean config set engine-image <image>`. | ||||||||||
| """ | ||||||||||
| from datetime import datetime | ||||||||||
| from json import loads | ||||||||||
| from lean.components.util.json_parser import load_json_from_file_or_string | ||||||||||
|
|
||||||||||
| logger = container.logger | ||||||||||
|
|
||||||||||
| # Set Docker timeout if specified | ||||||||||
| if docker_timeout is not None: | ||||||||||
| container.docker_manager._timeout = docker_timeout | ||||||||||
|
||||||||||
| container.docker_manager._timeout = docker_timeout | |
| container.docker_manager.set_timeout(docker_timeout) |
Outdated
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Missing blank line after the timeout setting block for consistency. Other commands (research.py, optimize.py, deploy.py) all have a blank line after this block. Add a blank line after line 336 for consistency.
| container.docker_manager._timeout = docker_timeout | |
| container.docker_manager._timeout = docker_timeout |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -110,6 +110,15 @@ def _get_history_provider_name(data_provider_live_names: [str]) -> [str]: | |||||
| default="{}", | ||||||
| help="Extra docker configuration as a JSON string. " | ||||||
| "For more information https://docker-py.readthedocs.io/en/stable/containers.html") | ||||||
| @option("--extra-docker-config-file", | ||||||
| type=PathParameter(exists=True, file_okay=True, dir_okay=False), | ||||||
| help="Path to a JSON file with extra docker configuration. " | ||||||
| "This is recommended over --extra-docker-config on Windows to avoid shell quote issues.") | ||||||
| @option("--docker-timeout", | ||||||
| type=int, | ||||||
| help="Timeout in seconds for Docker operations (default: 60). " | ||||||
| "Increase this for slow connections or large image pulls. " | ||||||
| "Can also be set via DOCKER_CLIENT_TIMEOUT environment variable.") | ||||||
| @option("--no-update", | ||||||
| is_flag=True, | ||||||
| default=False, | ||||||
|
|
@@ -131,6 +140,8 @@ def deploy(project: Path, | |||||
| addon_module: Optional[List[str]], | ||||||
| extra_config: Optional[Tuple[str, str]], | ||||||
| extra_docker_config: Optional[str], | ||||||
| extra_docker_config_file: Optional[Path], | ||||||
| docker_timeout: Optional[int], | ||||||
| no_update: bool, | ||||||
| **kwargs) -> None: | ||||||
| """Start live trading a project locally using Docker. | ||||||
|
|
@@ -155,9 +166,13 @@ def deploy(project: Path, | |||||
| """ | ||||||
| from copy import copy | ||||||
| from datetime import datetime | ||||||
| from json import loads | ||||||
| from lean.components.util.json_parser import load_json_from_file_or_string | ||||||
|
|
||||||
| logger = container.logger | ||||||
|
|
||||||
| # Set Docker timeout if specified | ||||||
| if docker_timeout is not None: | ||||||
| container.docker_manager._timeout = docker_timeout | ||||||
|
||||||
| container.docker_manager._timeout = docker_timeout | |
| container.docker_manager.set_timeout(docker_timeout) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -139,6 +139,15 @@ def get_filename_timestamp(path: Path) -> datetime: | |||||
| default="{}", | ||||||
| help="Extra docker configuration as a JSON string. " | ||||||
| "For more information https://docker-py.readthedocs.io/en/stable/containers.html") | ||||||
| @option("--extra-docker-config-file", | ||||||
| type=PathParameter(exists=True, file_okay=True, dir_okay=False), | ||||||
| help="Path to a JSON file with extra docker configuration. " | ||||||
| "This is recommended over --extra-docker-config on Windows to avoid shell quote issues.") | ||||||
| @option("--docker-timeout", | ||||||
| type=int, | ||||||
| help="Timeout in seconds for Docker operations (default: 60). " | ||||||
| "Increase this for slow connections or large image pulls. " | ||||||
| "Can also be set via DOCKER_CLIENT_TIMEOUT environment variable.") | ||||||
| @option("--no-update", | ||||||
| is_flag=True, | ||||||
| default=False, | ||||||
|
|
@@ -164,6 +173,8 @@ def optimize(project: Path, | |||||
| addon_module: Optional[List[str]], | ||||||
| extra_config: Optional[Tuple[str, str]], | ||||||
| extra_docker_config: Optional[str], | ||||||
| extra_docker_config_file: Optional[Path], | ||||||
| docker_timeout: Optional[int], | ||||||
| no_update: bool, | ||||||
| **kwargs) -> None: | ||||||
| """Optimize a project's parameters locally using Docker. | ||||||
|
|
@@ -207,8 +218,13 @@ def optimize(project: Path, | |||||
| from docker.types import Mount | ||||||
| from re import findall, search | ||||||
| from os import cpu_count | ||||||
| from lean.components.util.json_parser import load_json_from_file_or_string | ||||||
| from math import floor | ||||||
|
|
||||||
| # Set Docker timeout if specified | ||||||
| if docker_timeout is not None: | ||||||
| container.docker_manager._timeout = docker_timeout | ||||||
|
||||||
| container.docker_manager._timeout = docker_timeout | |
| container.docker_manager.set_timeout(docker_timeout) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -71,6 +71,15 @@ def _check_docker_output(chunk: str, port: int) -> None: | |||||
| default="{}", | ||||||
| help="Extra docker configuration as a JSON string. " | ||||||
| "For more information https://docker-py.readthedocs.io/en/stable/containers.html") | ||||||
| @option("--extra-docker-config-file", | ||||||
| type=PathParameter(exists=True, file_okay=True, dir_okay=False), | ||||||
| help="Path to a JSON file with extra docker configuration. " | ||||||
| "This is recommended over --extra-docker-config on Windows to avoid shell quote issues.") | ||||||
| @option("--docker-timeout", | ||||||
| type=int, | ||||||
| help="Timeout in seconds for Docker operations (default: 60). " | ||||||
| "Increase this for slow connections or large image pulls. " | ||||||
| "Can also be set via DOCKER_CLIENT_TIMEOUT environment variable.") | ||||||
| @option("--no-update", | ||||||
| is_flag=True, | ||||||
| default=False, | ||||||
|
|
@@ -86,6 +95,8 @@ def research(project: Path, | |||||
| update: bool, | ||||||
| extra_config: Optional[Tuple[str, str]], | ||||||
| extra_docker_config: Optional[str], | ||||||
| extra_docker_config_file: Optional[Path], | ||||||
| docker_timeout: Optional[int], | ||||||
| no_update: bool, | ||||||
| **kwargs) -> None: | ||||||
| """Run a Jupyter Lab environment locally using Docker. | ||||||
|
|
@@ -96,9 +107,13 @@ def research(project: Path, | |||||
| """ | ||||||
| from docker.types import Mount | ||||||
| from docker.errors import APIError | ||||||
| from json import loads | ||||||
| from lean.components.util.json_parser import load_json_from_file_or_string | ||||||
|
|
||||||
| logger = container.logger | ||||||
|
|
||||||
| # Set Docker timeout if specified | ||||||
| if docker_timeout is not None: | ||||||
| container.docker_manager._timeout = docker_timeout | ||||||
|
||||||
| container.docker_manager._timeout = docker_timeout | |
| container.docker_manager.set_timeout(docker_timeout) |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -27,16 +27,18 @@ | |||||||||||
| class DockerManager: | ||||||||||||
| """The DockerManager contains methods to manage and run Docker images.""" | ||||||||||||
|
|
||||||||||||
| def __init__(self, logger: Logger, temp_manager: TempManager, platform_manager: PlatformManager) -> None: | ||||||||||||
| def __init__(self, logger: Logger, temp_manager: TempManager, platform_manager: PlatformManager, timeout: int = 60) -> None: | ||||||||||||
| """Creates a new DockerManager instance. | ||||||||||||
|
|
||||||||||||
| :param logger: the logger to use when printing messages | ||||||||||||
| :param temp_manager: the TempManager instance used when creating temporary directories | ||||||||||||
| :param platform_manager: the PlatformManager used when checking which operating system is in use | ||||||||||||
| :param timeout: the timeout in seconds for Docker client operations (default: 60) | ||||||||||||
| """ | ||||||||||||
| self._logger = logger | ||||||||||||
| self._temp_manager = temp_manager | ||||||||||||
| self._platform_manager = platform_manager | ||||||||||||
| self._timeout = timeout | ||||||||||||
|
|
||||||||||||
| def get_image_labels(self, image: str) -> str: | ||||||||||||
| docker_image = self._get_docker_client().images.get(image) | ||||||||||||
|
|
@@ -570,7 +572,12 @@ def _get_docker_client(self): | |||||||||||
|
|
||||||||||||
| try: | ||||||||||||
| from docker import from_env | ||||||||||||
| docker_client = from_env() | ||||||||||||
| from os import environ | ||||||||||||
|
|
||||||||||||
| # Check for environment variable override | ||||||||||||
| timeout = int(environ.get("DOCKER_CLIENT_TIMEOUT", self._timeout)) | ||||||||||||
|
||||||||||||
| timeout = int(environ.get("DOCKER_CLIENT_TIMEOUT", self._timeout)) | |
| try: | |
| timeout = int(environ.get("DOCKER_CLIENT_TIMEOUT", self._timeout)) | |
| except ValueError: | |
| timeout = self._timeout # Fall back to instance timeout on invalid value |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
| @@ -0,0 +1,94 @@ | ||||
| # QUANTCONNECT.COM - Democratizing Finance, Empowering Individuals. | ||||
| # Lean CLI v1.0. Copyright 2021 QuantConnect Corporation. | ||||
| # | ||||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||||
| # you may not use this file except in compliance with the License. | ||||
| # You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||||
| # | ||||
| # Unless required by applicable law or agreed to in writing, software | ||||
| # distributed under the License is distributed on an "AS IS" BASIS, | ||||
| # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||||
| # See the License for the specific language governing permissions and | ||||
| # limitations under the License. | ||||
|
|
||||
| from pathlib import Path | ||||
| from typing import Dict, Any, Optional | ||||
| from json import loads, JSONDecodeError | ||||
|
|
||||
|
|
||||
| def parse_json_safely(json_string: str) -> Dict[str, Any]: | ||||
| """ | ||||
| Attempts to parse a JSON string with multiple fallback strategies. | ||||
| This function is designed to handle JSON strings that may have been | ||||
| mangled by Windows shells (PowerShell/CMD) which strip or escape quotes. | ||||
| :param json_string: The JSON string to parse | ||||
| :return: Parsed dictionary | ||||
| :raises ValueError: If all parsing attempts fail | ||||
| """ | ||||
| if not json_string or json_string.strip() == "": | ||||
| return {} | ||||
|
|
||||
| # Try standard JSON parsing first | ||||
| try: | ||||
| return loads(json_string) | ||||
| except JSONDecodeError as e: | ||||
| original_error = str(e) | ||||
|
|
||||
| # Try fixing common Windows shell issues | ||||
| # 1. Try adding back quotes that may have been stripped | ||||
| attempts = [ | ||||
| json_string, | ||||
| json_string.replace("'", '"'), # Single quotes to double quotes | ||||
| '{"' + json_string.strip('{}').replace(':', '":').replace(',', ',"') + '}', # Add missing quotes | ||||
|
||||
| '{"' + json_string.strip('{}').replace(':', '":').replace(',', ',"') + '}', # Add missing quotes |
Outdated
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docstring states "raises ValueError: If both parameters are None or if parsing fails", but the function returns an empty dict {} when both parameters are None (line 94) instead of raising ValueError. This is inconsistent with the documented behavior.
Either update the implementation to raise an error:
if json_string is None and json_file is None:
raise ValueError("Either json_string or json_file must be provided")
return {}Or update the docstring to accurately reflect the current behavior:
:return: Parsed dictionary, or empty dict if both parameters are None
Outdated
Copilot
AI
Dec 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function silently prioritizes json_file over json_string when both are provided, which could confuse users. Consider adding validation to prevent both parameters from being specified simultaneously, or at least logging a warning.
if json_file is not None and json_string is not None:
raise ValueError("Cannot specify both json_string and json_file. Please use only one.")Alternatively, update the docstring to clearly document this precedence behavior.
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -102,7 +102,10 @@ def initialize(self, | |||||||||||
|
|
||||||||||||
| self.docker_manager = docker_manager | ||||||||||||
| if not self.docker_manager: | ||||||||||||
| self.docker_manager = DockerManager(self.logger, self.temp_manager, self.platform_manager) | ||||||||||||
| from os import environ | ||||||||||||
| # Get timeout from environment variable, default to 60 seconds | ||||||||||||
| timeout = int(environ.get("DOCKER_CLIENT_TIMEOUT", 60)) | ||||||||||||
|
||||||||||||
| timeout = int(environ.get("DOCKER_CLIENT_TIMEOUT", 60)) | |
| try: | |
| timeout = int(environ.get("DOCKER_CLIENT_TIMEOUT", 60)) | |
| except ValueError: | |
| timeout = 60 # Fall back to default on invalid value |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The help text for
--docker-timeoutis inconsistent across commands. This command uses "Docker client timeout in seconds" while other commands (research.py, optimize.py, deploy.py) use "Timeout in seconds for Docker operations".For consistency and clarity, use the same wording across all commands: