clp-package: Deduplicate and clean up code in CLI job-launcher scripts. #473
Conversation
| @@ -241,6 +257,43 @@ def generate_container_config(clp_config: CLPConfig, clp_home: pathlib.Path): | |||
| return container_clp_config, docker_mounts | |||
|
|
|||
|
|
|||
| def dump_container_config( | |||
| clp_config: CLPConfig, container_clp_config: CLPConfig, container_name: str | |||
There was a problem hiding this comment.
Can we swap container_clp_config and clp_config?
There was a problem hiding this comment.
technically, we don't need the clp_config. the only thing needed by this function is clp_config.logs_directory which can be an argument called host_log_directory.
I didn't write in this way because then clp_config.logs_directory need to be duplicated in all callers.
me know what you think
There was a problem hiding this comment.
Since clp_config.logs_directory and container_clp_config.logs_directory are linked, I figure it's safer to pass in container_clp_config and clp_config to lessen the risk that the caller passes in two random paths.
components/clp-package-utils/clp_package_utils/scripts/compress.py
Outdated
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Outdated
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Outdated
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Outdated
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Show resolved
Hide resolved
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Show resolved
Hide resolved
Co-authored-by: kirkrodrigues <[email protected]>
components/clp-package-utils/clp_package_utils/scripts/native/decompress.py
Outdated
Show resolved
Hide resolved
| @@ -249,9 +322,29 @@ def validate_config_key_existence(config, key): | |||
| return value | |||
|
|
|||
|
|
|||
| def validate_and_load_config_file( | |||
| def validate_and_load_config_for_job_submission_script( | |||
There was a problem hiding this comment.
can't think of a good name for this method. I don't want to call it validate_and_load_config because the usage of this method is only limited to the caller script under package_utils/scripts. I also feel it's ok to revert this change.
There was a problem hiding this comment.
Yeah, maybe it's better to revert cause I'm also noticing that the different instances of this code validate different directories.
haiqi96
changed the title
Description
This PR factors out the common code used in job submission scripts in clp_package_utils. It also move some query job related functions into a separate util file to prepare an upcoming change that supports ir extraction job in the decompression script.
Validation performed
Launched the package, verified that compression, decompression and search still work from command line