Skip to content

chore: explore parallel library generation#12820

Draft
diegomarquezp wants to merge 10 commits intomainfrom
feat/parallel-hermetic-build
Draft

chore: explore parallel library generation#12820
diegomarquezp wants to merge 10 commits intomainfrom
feat/parallel-hermetic-build

Conversation

@diegomarquezp
Copy link
Copy Markdown
Contributor

@diegomarquezp diegomarquezp commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request enables parallel library generation for the Java SDK by implementing a ThreadPoolExecutor and a ThreadLocalStream for log management, alongside file locking and unique temporary directories to ensure thread safety. Review feedback points out that the log-capturing mechanism is insufficient for subprocesses and lacks necessary file-like attributes, potentially causing runtime failures. Additionally, improvements were suggested regarding PEP 8 compliance, reducing module coupling, and allowing configurable thread pool limits.


def library_generation_worker(config, library_path, library, repo_config):
buffer = StringIO()
has_local = hasattr(sys.stdout, "local")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The check hasattr(sys.stdout, "local") introduces a tight coupling between this module and the specific implementation of sys.stdout in generate_repo.py. This makes the function less reusable and harder to test in isolation. Consider passing a logger or a buffer object explicitly to the worker function instead of relying on global state monkey-patching.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We keep this non-invasive. If we go this way it would be a major refactor of the scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants