-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Add EGA-Cryptor #51108
base: master
Are you sure you want to change the base?
Add EGA-Cryptor #51108
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces three new files for the Changes
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (9)
recipes/ega-cryptor/build.sh (2)
1-3
: Good setup, consider consolidating error handling.The script starts with proper error handling and debugging options, which is excellent. However, you might consider consolidating the error handling options for brevity.
Consider combining lines 2 and 3 into a single line:
set -euxo pipefail
This achieves the same effect more concisely.
5-8
: Directory setup looks good, consider version-specific symlinks.The directory structure is set up correctly using environment variables, which is a good practice for versioning. However, the symbolic link created on line 7 might cause issues if multiple versions of the package are installed.
Consider using a version-specific symlink to avoid potential conflicts:
ln -s $PREFIX/share/$PKG_NAME-$PKG_VERSION-$PKG_BUILDNUM $PREFIX/share/$PKG_NAME-$PKG_VERSIONThis allows multiple versions to coexist without overwriting each other's symlinks.
recipes/ega-cryptor/meta.yaml (4)
13-15
: Consider versioning the source URL.The source section correctly provides both the URL and SHA256 checksum. However, the URL doesn't include the version number, which might cause issues with future updates.
Consider updating the URL to include the version number if possible. For example:
source: url: https://ega-archive.org/assets/files/EgaCryptor-{{ version }}.zipThis change would make it easier to update the package in the future and ensure that the correct version is always downloaded.
17-20
: Specify a Python version range.The runtime requirements correctly include OpenJDK 8 or higher. However, the Python requirement doesn't specify a version range.
Consider specifying a Python version range to ensure compatibility. For example:
requirements: run: - openjdk >=8 - python >=3.6This change would help users understand which Python versions are supported by the package.
22-24
: LGTM: Basic test is in place. Consider adding more comprehensive tests.The test section includes a basic command to check if the application runs and can display its help output. This is a good start.
If possible, consider adding more comprehensive tests to verify the functionality of the application. For example:
- Test file encryption
- Verify MD5 checksum generation
- Check if the output directory structure is created correctly
These additional tests would provide more confidence in the package's functionality.
16-16
: Remove trailing spaces.There are trailing spaces on lines 16 and 33. While these don't affect functionality, it's good practice to remove them for consistency and cleanliness.
Please remove the trailing spaces from lines 16 and 33.
Also applies to: 33-33
🧰 Tools
🪛 yamllint
[error] 16-16: trailing spaces
(trailing-spaces)
recipes/ega-cryptor/ega-cryptor.py (3)
68-68
: Correct typographical error in comment.The word "explictly" is misspelled. It should be "explicitly".
Apply this diff to correct the typo:
# it is important to explictly check for equality with None + # it is important to explicitly check for equality with None
83-83
: Fix typographical error in docstring.There's a typo in the docstring: "If the exec_dir dies not exist," should be "If the exec_dir does not exist,".
Apply this diff to correct the typo:
If the exec_dir dies not exist, + If the exec_dir does not exist,
96-98
: Check the return value of the subprocess call.Capturing and checking the return value of the subprocess call can provide more controlled exit status and allow for custom error handling if needed.
Consider modifying the code to handle the return value:
java_args = [java] + mem_opts + prop_opts + [jar_arg] + [jar_path] + pass_args - sys.exit(subprocess.call(java_args)) + return_code = subprocess.call(java_args) + if return_code != 0: + sys.stderr.write(f"Java process exited with return code {return_code}\n") + sys.exit(return_code)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- recipes/ega-cryptor/build.sh (1 hunks)
- recipes/ega-cryptor/ega-cryptor.py (1 hunks)
- recipes/ega-cryptor/meta.yaml (1 hunks)
🧰 Additional context used
🪛 Ruff
recipes/ega-cryptor/ega-cryptor.py
52-55: Combine
if
branches using logicalor
operatorCombine
if
branches(SIM114)
89-92: Use ternary operator
jar_arg = '-cp' if pass_args != [] and pass_args[0].startswith('eu') else '-jar'
instead ofif
-else
-blockReplace
if
-else
-block withjar_arg = '-cp' if pass_args != [] and pass_args[0].startswith('eu') else '-jar'
(SIM108)
🪛 yamllint
recipes/ega-cryptor/meta.yaml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
[error] 33-33: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
recipes/ega-cryptor/build.sh (2)
12-13
: Final setup looks good.The creation of a symbolic link in the bin directory and setting the appropriate permissions for the executable are both correct and follow best practices.
1-14
: Overall, the build script is functional but has room for improvement.The script successfully sets up the ega-cryptor package, following the general structure expected for a Bioconda build script. However, there are several areas where it could be optimized:
- Consolidating error handling options for brevity.
- Using version-specific symlinks to avoid potential conflicts with multiple installations.
- Being more selective in file copying to avoid including unnecessary files.
- Clarifying the purpose of the Python script (
ega-cryptor.py
).Addressing these points will enhance the script's robustness and clarity. Despite these suggestions, the script is functional and achieves its primary goal of setting up the package correctly.
recipes/ega-cryptor/meta.yaml (1)
1-5
: LGTM: Package information is well-defined.The package name and version are correctly specified. Using a variable for the version is a good practice for maintainability.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
elif arg.startswith('--exec_dir='): | ||
exec_dir = arg.split('=')[1].strip('"').strip("'") | ||
if not os.path.exists(exec_dir): | ||
shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None) |
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.
Add error handling for shutil.copytree
.
When copying files with shutil.copytree
, it's good practice to handle potential exceptions that may occur, such as FileExistsError
or OSError
, to prevent the script from crashing unexpectedly.
Apply this diff to add exception handling:
if not os.path.exists(exec_dir):
- shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None)
+ try:
+ shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None)
+ except Exception as e:
+ sys.stderr.write(f"Error copying files to exec_dir: {e}\n")
+ sys.exit(1)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None) | |
if not os.path.exists(exec_dir): | |
try: | |
shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None) | |
except Exception as e: | |
sys.stderr.write(f"Error copying files to exec_dir: {e}\n") | |
sys.exit(1) |
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.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (2)
recipes/ega-cryptor/meta.yaml (2)
25-27
: Consider adding more comprehensive tests.The current test command
ega-cryptor -h
is a good basic smoke test. However, to ensure the package functions correctly, consider adding more comprehensive tests, such as:
- Checking if all expected files are present in the installation directory.
- Verifying the version of the installed ega-cryptor.
- Testing basic functionality, like encrypting a small test file.
Example of additional test commands:
test: commands: - "ega-cryptor -h" - "ega-cryptor --version" - "test -f $PREFIX/bin/ega-cryptor" - "echo 'test' > test.txt && ega-cryptor -i test.txt -o test.encrypted.txt"These additional tests will provide more confidence in the package's functionality.
29-38
: Consider adding attribution for the description.The about section is now complete with the correct license (Apache-2.0) specified. However, the description appears to be copied directly from the project's website. While comprehensive, it's important to ensure you have permission to use this text or to provide proper attribution.
Consider either:
- Adding an attribution line if you have permission to use the text, e.g., "Description from the EGA-Cryptor website."
- Rewriting the description in your own words while maintaining the key information.
This will help avoid any potential copyright issues and align with best practices for package metadata.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/ega-cryptor/ega-cryptor.py (1 hunks)
- recipes/ega-cryptor/meta.yaml (1 hunks)
🧰 Additional context used
🪛 Ruff
recipes/ega-cryptor/ega-cryptor.py
100-100: SyntaxError: unindent does not match any outer indentation level
101-101: SyntaxError: Unexpected indentation
🪛 yamllint
recipes/ega-cryptor/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (12)
recipes/ega-cryptor/meta.yaml (4)
1-5
: LGTM: Package and version are correctly defined.The package name and version are properly set using Jinja2 templating, which is a good practice for maintainability.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
14-16
: LGTM: Source URL and checksum are properly defined.The source URL and SHA256 checksum are correctly specified, which is good practice for security and integrity verification.
18-23
: LGTM: Requirements are appropriately specified.The build and run requirements are correctly defined:
unzip
is needed for the build process to extract the source.openjdk >=8
andpython >=3.6
are appropriate runtime dependencies for this Java-based application.The use of version constraints for runtime dependencies is a good practice.
7-12
:⚠️ Potential issueRevise the
run_exports
section.The build section is mostly correct, but there's still an issue with the
run_exports
section:
- The
pin_subpackage
function is typically used in therequirements
section, not inrun_exports
.- The syntax for version pinning in
run_exports
is incorrect.Consider replacing the
run_exports
section with:run_exports: - {{ pin_compatible('ega-cryptor', max_pin='x.x.x') }}This will ensure that the package is compatible with other packages built against this version of ega-cryptor.
recipes/ega-cryptor/ega-cryptor.py (8)
9-18
: LGTM: Imports and global variables are well-defined.The import statements are appropriate for the script's functionality, and the global variables are correctly defined. The default JVM memory options seem reasonable for general use.
23-25
: LGTM:real_dirname
function is well-implemented.The
real_dirname
function correctly resolves symlinks and returns the canonical directory path. The docstring accurately describes the function's purpose.
28-36
: LGTM:java_executable
function is robust and well-implemented.The
java_executable
function correctly handles both cases whenJAVA_HOME
is set and when it's not. The logic for checking the executable's accessibility is sound, providing a reliable way to determine the Java interpreter to use.
39-74
: LGTM:jvm_opts
function is well-implemented overall.The
jvm_opts
function correctly processes command-line arguments to construct lists of memory options, property options, and passthrough arguments. The logic for handling default JVM memory options when not explicitly set is sound.
77-101
: LGTM:main
function is well-implemented overall.The
main
function correctly orchestrates the execution of the Java application. It properly uses the helper functions to construct the Java command and executes it usingsubprocess.call
. The error handling for non-zero return codes is a good practice.🧰 Tools
🪛 Ruff
100-100: SyntaxError: unindent does not match any outer indentation level
101-101: SyntaxError: Unexpected indentation
103-104
: LGTM: Script execution block is correct.The script execution block correctly calls the
main
function when the script is run directly, following the standard Python idiom.
1-104
: Overall, the script is well-implemented and fulfills its purpose.This Python wrapper script for executing Java applications in Conda environments is well-structured and follows good practices. It correctly handles JVM options, Java executable discovery, and command construction. The separation of concerns into distinct functions makes the code readable and maintainable.
A few minor improvements have been suggested in the review comments, including:
- Combining consecutive
if
statements in thejvm_opts
function.- Adding error handling for
shutil.copytree
.- Simplifying an
if
-else
block using a ternary operator in themain
function.- Fixing indentation issues in the
main
function.Implementing these suggestions will further enhance the script's quality and robustness.
🧰 Tools
🪛 Ruff
100-100: SyntaxError: unindent does not match any outer indentation level
101-101: SyntaxError: Unexpected indentation
61-61
:⚠️ Potential issueAdd error handling for
shutil.copytree
.As suggested in the previous review, it's good practice to handle potential exceptions that may occur when using
shutil.copytree
. This will prevent the script from crashing unexpectedly.Apply this diff to add exception handling:
if not os.path.exists(exec_dir): - shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None) + try: + shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None) + except Exception as e: + sys.stderr.write(f"Error copying files to exec_dir: {e}\n") + sys.exit(1)Likely invalid or redundant comment.
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
recipes/ega-cryptor/meta.yaml (2)
24-26
: LGTM: Basic test command is defined.The test command
ega-cryptor -h
provides a basic smoke test to ensure the application can be executed and display its help information.Consider adding more comprehensive tests if possible, such as:
- Checking for specific expected output from the help command.
- Testing basic encryption/decryption functionality with sample data.
28-38
: Description source and formatting.The about section is well-defined with the correct home URL, summary, and license (Apache-2.0). The description is comprehensive, which is good.
- Consider rephrasing the description to avoid direct copying from the website, or ensure you have permission to use it.
- The last line about the description source could be moved to a comment:
description: | The EGACryptor v.2.0.0 is a JAVA-based application which enables submitters to produce EGA compliant encrypted files along with files for the encrypted and unencrypted md5sum for each file to be submitted. The application will generate an output folder that will by default mirror the directory structure containing the original files. This output folder can subsequently be uploaded to the EGA FTP staging area via an FTP or Aspera client. # Description adapted from the EGA-Cryptor website.This keeps the description clean while still attributing the source.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- recipes/ega-cryptor/ega-cryptor.py (1 hunks)
- recipes/ega-cryptor/meta.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- recipes/ega-cryptor/ega-cryptor.py
🧰 Additional context used
🪛 yamllint
recipes/ega-cryptor/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (4)
recipes/ega-cryptor/meta.yaml (4)
1-5
: LGTM: Package and version are correctly defined.The package name and version are properly set using Jinja2 templating, which is a good practice for maintainability.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
13-15
: LGTM: Source URL and checksum are correctly defined.The source URL and SHA256 checksum are properly specified, which ensures the integrity of the downloaded file.
17-22
: LGTM: Requirements are appropriately defined.The build and run requirements are correctly specified:
unzip
is needed to extract the source zip file.openjdk >=8
andpython >=3.6
are appropriate for a Java-based application with Python support.The version constraints are reasonable and allow for compatibility with newer versions.
1-1
: Note on yamllint warning.The yamllint tool reports a syntax error for the '%' character at the beginning of the file. This is a false positive, as the '%' is part of the Jinja2 templating syntax, which is valid and commonly used in Bioconda recipes.
No action is needed to address this warning.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
recipes/ega-cryptor/meta.yaml (3)
13-15
: LGTM with suggestion: Consider using a versioned URL.The source section is well-defined with a correct URL and SHA256 checksum. However, for better reproducibility and version tracking, consider using a versioned URL if available.
For example:
url: https://ega-archive.org/assets/files/EgaCryptor-{{ version }}.zipThis change would make it easier to update the package in the future and ensure that the correct version is always downloaded.
24-26
: LGTM with suggestions: Consider adding more comprehensive tests.The current test command is a good basic smoke test to ensure the package is installed correctly. However, to improve the robustness of the package, consider adding more comprehensive tests. For example:
- Check if specific expected output is present in the help message.
- Test basic functionality by encrypting a small test file.
- Verify that MD5 checksums are generated correctly.
Example of a more comprehensive test section:
test: commands: - ega-cryptor -h | grep "EGA-Cryptor" - echo "test" > test.txt - ega-cryptor -i test.txt -o encrypted_test.txt - test -f encrypted_test.txt - test -f encrypted_test.txt.md5 - test -f test.txt.md5These additional tests would provide more confidence in the package's functionality.
28-37
: LGTM: About section is comprehensive and improved.Great job on improving the about section:
- The home URL is correct.
- The summary is concise and informative.
- The license has been updated to Apache-2.0, addressing the previous "unknown" license issue.
- The description is comprehensive and accurately describes the package's functionality.
It's good that you've indicated the source of the description with the comment. To further improve, consider adding a reference to the Apache-2.0 license text or a link to it.
recipes/ega-cryptor/ega-cryptor.py (3)
28-29
: Add a docstring to the java_executable function.For consistency with other functions and to improve code documentation, consider adding a detailed docstring to the
java_executable
function.Example implementation:
def java_executable(): """ Determine the path to the Java executable. This function first checks if JAVA_HOME is set and contains a valid Java executable. If not, it falls back to using 'java' and relies on the system PATH. Returns: str: The path to the Java executable. """
63-70
: Clarify comment about shell script behavior.The comment explaining the reproduction of shell script behavior could be clearer. Consider rephrasing it to make the logic more explicit.
Suggested rephrasing:
# Reproduce the behavior of the original shell script: # If no JVM memory options were set on the command line, # and the _JAVA_OPTIONS environment variable is not set, # use the default memory options. if not mem_opts and os.environ.get('_JAVA_OPTIONS') is None: mem_opts = default_jvm_mem_optsThis makes the logic more explicit and easier to understand.
77-83
: Remove or update outdated comment.The comment mentioning PeptideShaker seems out of place and might be outdated. If it's not relevant to EGA-Cryptor, consider removing it or updating it to reflect the current application.
If the comment is still relevant, consider updating it to:
""" EGA-Cryptor may update files relative to the path of the jar file. In a multiuser setting, the option --exec_dir="exec_dir" can be used as the location for the EGA-Cryptor distribution. If the exec_dir does not exist, we copy the jar file, lib, and resources to the exec_dir directory. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- recipes/ega-cryptor/build.sh (1 hunks)
- recipes/ega-cryptor/ega-cryptor.py (1 hunks)
- recipes/ega-cryptor/meta.yaml (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- recipes/ega-cryptor/build.sh
🧰 Additional context used
🪛 yamllint
recipes/ega-cryptor/meta.yaml
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
🔇 Additional comments (7)
recipes/ega-cryptor/meta.yaml (4)
1-5
: LGTM: Package and version section is well-defined.The use of Jinja2 templating for the version is a good practice, and the package name and version are correctly defined.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
7-11
: LGTM: Build section is well-defined and improved.The build section is correctly configured:
noarch: generic
is appropriate for a Java-based application.- The build number is correctly set to 0 for a new package.
- The
run_exports
section has been improved as per previous suggestions, usingpin_compatible
which is the correct approach.Great job on implementing the suggested changes!
17-22
: LGTM: Requirements are well-defined.The requirements section is correctly configured:
- The build requirement
unzip
is appropriate for extracting the zip file.- The run requirements
openjdk >=8
andpython >=3.6
are suitable for a Java-based application with Python support.- The version constraints are reasonable and allow for flexibility.
1-1
: Note: Ignore yamllint syntax error for Jinja2 templating.The yamllint tool reports a syntax error for the '%' character at the beginning of the file. This is a false positive. The '%' character is part of the Jinja2 templating syntax, which is correctly used in conda-forge and bioconda recipes. No action is required to address this warning.
🧰 Tools
🪛 yamllint
[error] 1-1: syntax error: found character '%' that cannot start any token
(syntax)
recipes/ega-cryptor/ega-cryptor.py (3)
23-26
: LGTM: Well-implemented directory resolution function.The
real_dirname
function correctly resolves symlinks and returns the canonical directory path. The implementation is concise and uses appropriateos.path
functions.
98-99
: LGTM: Correct main execution block.The main execution block is correctly implemented, following the standard Python idiom for executing the main function when the script is run directly.
1-99
: Overall: Well-structured wrapper script with minor improvement opportunities.This Python script serves as a wrapper for executing the EGA-Cryptor Java application within a Conda environment. It effectively handles JVM options, determines the correct Java executable, and executes the Java command. The overall structure and functionality of the script are sound and it appears to fulfill its purpose well.
There are a few areas where the script could be improved:
- Making the jar file name configurable
- Adding missing docstrings
- Enhancing robustness of --exec_dir handling
- Clarifying comments and removing outdated information
- Improving error handling
These improvements would enhance the script's flexibility, maintainability, and robustness. Overall, the script is well-implemented and, with these minor enhancements, would be even more effective in its role.
from os import access | ||
from os import getenv | ||
from os import X_OK | ||
jar_file = 'ega-cryptor-2.0.0.jar' |
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.
🛠️ Refactor suggestion
Consider making the jar file name configurable.
The jar file name is currently hardcoded. To improve flexibility and ease future updates, consider making this configurable. You could use an environment variable or a command-line argument.
Example implementation:
import os
jar_file = os.environ.get('EGA_CRYPTOR_JAR', 'ega-cryptor-2.0.0.jar')
This allows users to override the jar file name if needed, while maintaining the current default.
exec_dir = arg.split('=')[1].strip('"').strip("'") | ||
if not os.path.exists(exec_dir): | ||
shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None) |
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.
Improve robustness of --exec_dir handling.
The current implementation of --exec_dir
handling could be more robust. Consider adding error handling and validation for the directory path.
Example implementation:
elif arg.startswith('--exec_dir='):
exec_dir = arg.split('=')[1].strip('"').strip("'")
if not os.path.isabs(exec_dir):
sys.stderr.write(f"Error: --exec_dir must be an absolute path\n")
sys.exit(1)
if not os.path.exists(exec_dir):
try:
shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None)
except OSError as e:
sys.stderr.write(f"Error creating exec_dir: {e}\n")
sys.exit(1)
This implementation ensures that the path is absolute and handles potential errors during directory creation.
def jvm_opts(argv): | ||
"""Construct list of Java arguments based on our argument list. | ||
|
||
The argument list passed in argv must not include the script name. | ||
The return value is a 3-tuple lists of strings of the form: | ||
(memory_options, prop_options, passthrough_options) | ||
""" | ||
mem_opts = [] | ||
prop_opts = [] | ||
pass_args = [] | ||
exec_dir = None | ||
|
||
for arg in argv: | ||
if arg.startswith('-D') or arg.startswith('-XX'): | ||
prop_opts.append(arg) | ||
elif arg.startswith('-Xm'): | ||
mem_opts.append(arg) | ||
elif arg.startswith('--exec_dir='): | ||
exec_dir = arg.split('=')[1].strip('"').strip("'") | ||
if not os.path.exists(exec_dir): | ||
shutil.copytree(real_dirname(sys.argv[0]), exec_dir, symlinks=False, ignore=None) | ||
else: | ||
pass_args.append(arg) | ||
|
||
# In the original shell script the test coded below read: | ||
# if [ "$jvm_mem_opts" == "" ] && [ -z ${_JAVA_OPTIONS+x} ] | ||
# To reproduce the behaviour of the above shell code fragment | ||
# it is important to explicitly check for equality with None | ||
# in the second condition, so a null envar value counts as True! | ||
|
||
if mem_opts == [] and getenv('_JAVA_OPTIONS') is None: | ||
mem_opts = default_jvm_mem_opts | ||
|
||
return (mem_opts, prop_opts, pass_args, exec_dir) |
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.
🛠️ Refactor suggestion
Consider refactoring jvm_opts function.
The jvm_opts
function is quite long and handles multiple responsibilities. Consider splitting it into smaller, more focused functions to improve readability and maintainability.
You could split the function into:
parse_jvm_args
: Handle parsing of JVM-related arguments.handle_exec_dir
: Process the --exec_dir argument.apply_default_memory_opts
: Apply default memory options if necessary.
This would make the main jvm_opts
function shorter and easier to understand.
return_code = subprocess.call(java_args) | ||
if return_code != 0: | ||
sys.stderr.write(f"Java process exited with return code {return_code}\n") | ||
sys.exit(return_code) |
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.
🛠️ Refactor suggestion
Enhance error handling for subprocess call.
The current error handling for the subprocess call could be more informative. Consider capturing and logging more details about the error.
Example implementation:
try:
return_code = subprocess.call(java_args)
if return_code != 0:
sys.stderr.write(f"Java process exited with return code {return_code}\n")
sys.exit(return_code)
except Exception as e:
sys.stderr.write(f"Error executing Java process: {e}\n")
sys.exit(1)
This will provide more context if there's an error in executing the Java process.
Add EGA-Cryptor (https://ega-archive.org/submission/data/file-preparation/egacryptor/) package to bioconda.
The EGACryptor v.2.0.0 is a JAVA-based application which enables submitters to produce EGA compliant encrypted files along with files for the encrypted and unencrypted md5sum for each file to be submitted. The application will generate an output folder that will by default mirror the directory structure containing the original files. This output folder can subsequently be uploaded to the EGA FTP staging area via an FTP or Aspera client.
Please read the guidelines for Bioconda recipes before opening a pull request (PR).
General instructions
@BiocondaBot please add label
command.@bioconda/core
in a comment.Instructions for avoiding API, ABI, and CLI breakage issues
Conda is able to record and lock (a.k.a. pin) dependency versions used at build time of other recipes.
This way, one can avoid that expectations of a downstream recipe with regards to API, ABI, or CLI are violated by later changes in the recipe.
If not already present in the meta.yaml, make sure to specify
run_exports
(see here for the rationale and comprehensive explanation).Add a
run_exports
section like this:with
...
being one of:{{ pin_subpackage("myrecipe", max_pin="x") }}
{{ pin_subpackage("myrecipe", max_pin="x.x") }}
{{ pin_subpackage("myrecipe", max_pin="x.x") }}
(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin="x.x.x") }}
(in such a case, please add a note that shortly mentions your evidence for that){{ pin_subpackage("myrecipe", max_pin=None) }}
while replacing
"myrecipe"
with eithername
if aname|lower
variable is defined in your recipe or with the lowercase name of the package in quotes.Bot commands for PR management
Please use the following BiocondaBot commands:
Everyone has access to the following BiocondaBot commands, which can be given in a comment:
@BiocondaBot please update
@BiocondaBot please add label
please review & merge
label.@BiocondaBot please fetch artifacts
You can use this to test packages locally.
Note that the
@BiocondaBot please merge
command is now depreciated. Please just squash and merge instead.Also, the bot watches for comments from non-members that include
@bioconda/<team>
and will automatically re-post them to notify the addressed<team>
.Summary by CodeRabbit
New Features
ega-cryptor
package.ega-cryptor
package version2.0.0
with detailed metadata and runtime dependencies.Documentation
ega-cryptor
application and its functionalities in the metadata.