Skip to content

TASK-7524 - Add new Variant output format: JSON_SPARSE #109

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

Open
wants to merge 2 commits into
base: release-6.x.x
Choose a base branch
from

Conversation

j-coll
Copy link
Member

@j-coll j-coll commented May 9, 2025

No description provided.

@imedina
Copy link
Member

imedina commented May 9, 2025

@j-coll j-coll requested a review from Copilot May 9, 2025 09:06
@j-coll j-coll changed the title TASK-7524 TASK-7524 - Add new Variant output format: JSON_SPARSE May 9, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR tackles TASK-7524 by adding a new test for the Command class, deprecating SingleProcess, and refactoring the Command class to use asynchronous stream reading via Futures.

  • Added CommandTest.java to validate execution and output handling of Command
  • Marked SingleProcess as deprecated
  • Updated Command.java to use an ExecutorService for stream reading and added a getStdin() method

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
commons-lib/src/test/java/org/opencb/commons/exec/CommandTest.java Added tests to verify command execution and output correctness
commons-lib/src/main/java/org/opencb/commons/exec/SingleProcess.java Deprecated the SingleProcess class
commons-lib/src/main/java/org/opencb/commons/exec/RunnableProcess.java Initialized status to WAITING for improved state management
commons-lib/src/main/java/org/opencb/commons/exec/NamedThreadFactory.java No functional changes
commons-lib/src/main/java/org/opencb/commons/exec/Command.java Refactored to use asynchronous stream reading with Futures and added a new getStdin() accessor
Comments suppressed due to low confidence (1)

commons-lib/src/main/java/org/opencb/commons/exec/Command.java:208

  • The log formatting appears incorrect; consider updating it to: logger.trace(outputName + " - last bytesRead = {}", bytesRead);
logger.trace(outputName + " - last bytesRead = {})", bytesRead);

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.

3 participants