Test PR 4

[Alex-Burmak]

PR Type

Enhancement, Bug fix

---

Description

• Modernize Python dependency management with uv

• Update supported Python versions (3.9+ required)

• Fix LockManager file handling issue

• Add workaround for "Invalid thread ID" error

---

Changes walkthrough 📝

	Relevant files
Enhancement	15 files control.py Replace pkg_resources with packaging.version                          +1/-1      setup.py Remove setup.py in favor of pyproject.toml                              +0/-67    clickhouse.py Replace pkg_resources with packaging.version                          +1/-1      compose.py Add support for "docker compose" command                                  +7/-2      utils.py Replace pkg_resources with packaging.version                          +1/-1      test_backup_tables.py Use parenthesized context manager syntax                                  +3/-2      test_pipeline.py Use parenthesized context manager syntax                                  +4/-3      test_clickhouse_version.yml Replace Python setup with uv setup                                              +1/-4      workflow.yml Update CI workflow to use uv and newer Python                        +10/-17  Dockerfile-deb-build Add uv installation to Dockerfile                                                +6/-3      Makefile Refactor Makefile to use uv instead of pip                              +84/-95  Dockerfile Update Dockerfile to use wheel installation                            +16/-22  pyproject.toml Add pyproject.toml for modern Python packaging                      +102/-0  requirements-dev.txt Remove requirements-dev.txt in favor of pyproject.toml      +0/-21    requirements.txt Remove requirements.txt in favor of pyproject.toml              +0/-20
Bug fix	2 files lock_manager.py Fix file descriptor handling in LockManager                            +3/-5      pipelines.py Add workaround for "Invalid thread ID" error                          +13/-2
Formatting	18 files exec_pool.py Add trailing comma in function parameters                                +1/-1      pipeline_builder.py Add trailing comma in function call                                            +1/-1      zookeeper.py Add newline for consistent formatting                                        +1/-0      ch_backup.py Add newline for consistent formatting                                        +1/-0      clickhouse.py Add newline for consistent formatting                                        +1/-0      common.py Add newline for consistent formatting                                        +1/-0      s3.py Add newline for consistent formatting                                        +1/-0      zookeeper.py Add newline for consistent formatting                                        +1/-0      test_access.py Reorder imports for consistency                                                    +1/-2      test_cli.py Reorder imports for consistency                                                    +1/-2      test_clickhouse_encryption.py Reorder imports for consistency                                                    +1/-2      test_control.py Reorder imports for consistency                                                    +1/-2      test_disks.py Reorder imports for consistency                                                    +1/-1      test_rate_limiter.py Reorder imports for consistency                                                    +1/-1      test_retry_exponetial.py Reorder imports for consistency                                                    +1/-1      test_schema.py Reorder imports for consistency                                                    +1/-2      test_upload_part_observer.py Reorder imports for consistency                                                    +1/-2      backup_replicated.feature Fix indentation in feature file                                                    +2/-2
Configuration changes	7 files .codespellrc Remove .codespellrc in favor of pyproject.toml                      +0/-3      .isort.cfg Remove .isort.cfg in favor of pyproject.toml                          +0/-4      .pylintrc Update minimum Python version to 3.9                                          +1/-1      .python-version Add Python version file for version management                      +1/-0      control Update minimum Python version to 3.9                                          +1/-1      rules Update Python version detection in debian rules                    +2/-2      mypy.ini Add Python version specification for mypy                                +1/-0
Documentation	1 files AUTHORS Update and standardize author email formats                            +6/-5

---

Need help?

Type /help how to ... in the comments thread for any questions about PR-Agent usage.

Check out the documentation for more information.

[robot-ch-tools]

PR Code Suggestions ✨

Latest suggestions up to 5f57891

Category	Suggestion	Impact
Security	Update vulnerable dependency version The PyNaCl dependency is pinned to a very old version (1.2.1) from 2018 which contains known security vulnerabilities. Consider updating to a more recent version. pyproject.toml [47] -"pynacl == 1.2.1", +"pynacl >= 1.5.0", Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a security vulnerability in the pinned PyNaCl version 1.2.1 (from 2018). Updating to a more recent version would address known security issues, making this a high-priority change.	High
	Update outdated dependency constraint The PyYAML dependency is constrained to versions below 5.4, which excludes security fixes. Recent versions of PyYAML (6.x) include important security patches. pyproject.toml [49] -"pyyaml >= 5.0, < 5.4", +"pyyaml >= 6.0", Suggestion importance[1-10]: 8 __ Why: The suggestion correctly points out that the PyYAML constraint (<5.4) excludes newer versions with important security fixes. Updating to PyYAML 6.x would incorporate these security patches, significantly improving the project's security posture.	Medium
General	Fix inconsistent indentation The indentation of the ClickHouse setting is inconsistent with the rest of the file. The setting should be properly indented to maintain consistent formatting. tests/integration/features/backup_replicated.feature [867] """ -allow_experimental_database_replicated: 1 + allow_experimental_database_replicated: 1 """ Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies an indentation inconsistency in the feature file. The PR removes indentation that was present in the old code, and the suggestion recommends restoring it for consistency with the rest of the file's formatting.	Low
	Handle keyword arguments properly **The submit method signature has been changed to include kwargs as a parameter, but the implementation of the method might not be using these keyword arguments. This could lead to unexpected behavior if callers are passing keyword arguments that are being silently ignored. ch_backup/storage/async_pipeline/base_pipeline/exec_pool.py [54-58] def submit( self, func: Callable, *args: Any, callback: Optional[Callable] = None, **kwargs: Any, ) -> None: + # Make sure to pass kwargs to func when it's called Suggestion importance[1-10]: 3 __ Why: The suggestion correctly identifies that the submit method signature has been updated to include **kwargs, but there's no indication in the PR diff about how these kwargs are used. This is a valid concern about potential silent parameter ignoring.	Low

---

Previous suggestions

Suggestions up to commit 5f57891

Category	Suggestion	Impact
Possible issue	Incorrect with statement syntax The with statement in the test is missing the actual context managers to be used. The parentheses are opened but the context managers are not properly defined, which will cause a syntax error. Fix the with statement by properly connecting the context managers. tests/unit/test_backup_tables.py [109-112] # Backup table -with ( - patch("os.stat", side_effect=fake_stats), - patch("ch_backup.logic.table.Path", read_bytes=read_bytes_mock), +with patch("os.stat", side_effect=fake_stats), patch( + "ch_backup.logic.table.Path", read_bytes=read_bytes_mock ): table_backup.backup( context, Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a syntax issue in the PR. The PR introduces a new syntax with parentheses around context managers but doesn't properly close them. The suggested fix properly formats the with statement to use multiple context managers correctly, which would prevent a syntax error.	High
Security	Update vulnerable dependency The PyNaCl version is pinned to a very old version (1.2.1) from 2018 which has known security vulnerabilities. Consider updating to a more recent version. pyproject.toml [47] -"pynacl == 1.2.1", +"pynacl >= 1.5.0", Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a security vulnerability in the pinned PyNaCl version 1.2.1 (from 2018). Updating to a more recent version (>=1.5.0) would address potential security issues, making this a high-priority suggestion.	High
General	Fix incorrect indentation The indentation of the Scenario Outline line is incorrect, which could cause parsing issues in Gherkin/Behave. It should be aligned with the left margin, not indented. tests/integration/features/backup_replicated.feature [862] -+ Scenario Outline: Add new host with replicated +Scenario Outline: Add new host with replicated Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies an indentation issue in the Gherkin feature file. Proper indentation is important for Gherkin/Behave parsing, and fixing it would prevent potential test execution problems.	Medium

[robot-ch-tools]

PR Reviewer Guide 🔍

(Review updated until commit 5f57891)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review File Handling The LockManager._flock method now opens the file in "r+" mode instead of "r" mode, which is correct for writing, but there's no error handling for file operations. Consider adding try/except blocks to handle potential file access errors. self._file = open(self._process_lockfile_path, "r+", encoding="utf-8") flock(self._file, LOCK_SH) Exception Handling The workaround for "Invalid thread ID" error catches ValueError exceptions with a specific string check. This approach might mask other ValueError exceptions that should be properly handled or propagated. except ValueError as e: if "Invalid thread ID" in str(e): logging.warning( "Thread ID error during iteration exhaustion due to" " incorrect thread termination in stopit library, skipping", exc_info=True, ) else: raise Docker Compose Detection The code now tries to detect whether to use "docker-compose" or "docker compose", but doesn't handle the case where neither is available. Consider adding proper error handling if both commands are missing. docker_compose = "docker-compose" if shutil.which(docker_compose) is None: docker_compose = "docker compose"

[robot-ch-tools]

Persistent review updated to latest commit 5f57891

[robot-ch-tools]

Persistent review updated to latest commit 5f57891