clp-package: Add support for extracting files as IR streams through the CLI. #472

haiqi96 · 2024-07-05T15:58:44Z

Description

Let decompress cmd script in package handle both decompression and ir extraction

Validation performed

Launched the package, verified that compression, decompression, ir extraction and search still work from command line

haiqi96 · 2024-07-06T03:54:58Z

components/clp-package-utils/clp_package_utils/general.py


 def generate_container_start_cmd(
-    container_name: str, container_mounts: List[CLPDockerMounts], container_image: str
+    container_name: str, container_mounts: List[Optional[DockerMount]], container_image: str


haiqi96 · 2024-07-06T04:02:10Z

components/clp-package-utils/clp_package_utils/general.py

 from strenum import KebabCaseStrEnum

 # CONSTANTS
+DECOMPRESSION_COMMAND = "x"


Was thinking about using a enum similar to JobType, but will need to manually assign a value instead of using auto.

haiqi96 · 2024-07-06T04:04:44Z

A high level concern is that what would be good terms to distinguish, (1) a general job name launched by decompress.py script" (2) a job that decompress a file, and (3) a job that extracts an IR.

I am currently using "x" as the decompression command following CLP and CLO, but internally still use decompression as job type, which is inconsistent.

Should we call both job as decompression job, and internally distinguish them as "extraction" and "ir_extraction" command?

components/clp-package-utils/clp_package_utils/scripts/decompress.py

components/clp-package-utils/clp_package_utils/scripts/native/decompress.py

junhaoliao · 2024-07-11T06:30:19Z

How do we specify target-uncompressed-size?
I tried ./decompress.sh i --target-uncompressed-size 10240 --orig-file-id daf326b3-ab77-42ec-9fcf-056b541f948a 0 and also manually inserted a msgpack record of

{
	"orig_file_id": "f6fa2faf-d686-4d54-b086-b68c3da04405",
	"msg_ix": 1,
	"target_uncompressed_size": 10240
}

which both yielded

[2024-07-11 06:28:05,643: INFO/ForkPoolWorker-7] job_orchestration.executor.query.extract_ir_task.extract_ir[78f14806-52e2-4e87-81d8-3f00fc2ac0d2]: Started IR extraction task for job 3
[2024-07-11 06:28:05,648: ERROR/ForkPoolWorker-7] Task job_orchestration.executor.query.extract_ir_task.extract_ir[78f14806-52e2-4e87-81d8-3f00fc2ac0d2] raised unexpected: TypeError('sequence item 8: expected str instance, int found')
Traceback (most recent call last):
  File "/opt/clp/lib/python3/site-packages/celery/app/trace.py", line 453, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/opt/clp/lib/python3/site-packages/celery/app/trace.py", line 736, in __protected_call__
    return self.run(*args, **kwargs)
  File "/opt/clp/lib/python3/site-packages/job_orchestration/executor/query/extract_ir_task.py", line 106, in extract_ir
    return run_query_task(
  File "/opt/clp/lib/python3/site-packages/job_orchestration/executor/query/utils.py", line 62, in run_query_task
    logger.info(f'Running: {" ".join(task_command)}')
TypeError: sequence item 8: expected str instance, int found

Co-authored-by: Junhao Liao <junhao@junhao.ca>

haiqi96 · 2024-07-11T14:55:54Z

How do we specify target-uncompressed-size? I tried ./decompress.sh i --target-uncompressed-size 10240 --orig-file-id daf326b3-ab77-42ec-9fcf-056b541f948a 0 and also manually inserted a msgpack record of

{
	"orig_file_id": "f6fa2faf-d686-4d54-b086-b68c3da04405",
	"msg_ix": 1,
	"target_uncompressed_size": 10240
}

which both yielded

[2024-07-11 06:28:05,643: INFO/ForkPoolWorker-7] job_orchestration.executor.query.extract_ir_task.extract_ir[78f14806-52e2-4e87-81d8-3f00fc2ac0d2]: Started IR extraction task for job 3
[2024-07-11 06:28:05,648: ERROR/ForkPoolWorker-7] Task job_orchestration.executor.query.extract_ir_task.extract_ir[78f14806-52e2-4e87-81d8-3f00fc2ac0d2] raised unexpected: TypeError('sequence item 8: expected str instance, int found')
Traceback (most recent call last):
  File "/opt/clp/lib/python3/site-packages/celery/app/trace.py", line 453, in trace_task
    R = retval = fun(*args, **kwargs)
  File "/opt/clp/lib/python3/site-packages/celery/app/trace.py", line 736, in __protected_call__
    return self.run(*args, **kwargs)
  File "/opt/clp/lib/python3/site-packages/job_orchestration/executor/query/extract_ir_task.py", line 106, in extract_ir
    return run_query_task(
  File "/opt/clp/lib/python3/site-packages/job_orchestration/executor/query/utils.py", line 62, in run_query_task
    logger.info(f'Running: {" ".join(task_command)}')
TypeError: sequence item 8: expected str instance, int found

Sorry, it looks like I forgot to do a type conversion.

I included it in https://github.com/y-scope/clp/pull/472/files#diff-c3c708ca5b9cee2be7634fcb5966f6fc622b85ec0ad8575491540e676414cbbaL48. if you need this change urgently, I can also make a separate PR for it,

components/clp-package-utils/clp_package_utils/scripts/decompress.py

… Make command handlers return consistently rather than throwing exceptions in some cases.

kirkrodrigues · 2024-07-18T14:36:00Z

components/clp-package-utils/clp_package_utils/scripts/native/decompress.py

+        logger.error(f"job {job_id} finished with unexpected status: {job_status}")
+
+
+async def do_extract(


We don't really need this method, right? (Since it's purely wrapping another async method.)

hmm, I tried to remove it but then it seems that I have to turn the entire call trees to async, including main. Maybe my knowledge about async is not enough to make a neat change quickly.

Given that this code is copied from search.py, can we turn this refactor into a separate PR and apply to both decompress.py and search.py?

This doesn't work (in handle_extract_ir_cmd)?

return asyncio.run( run_function_in_process( submit_and_monitor_ir_extraction_job_in_db, clp_config.database, orig_file_id, parsed_args.msg_ix, parsed_args.target_uncompressed_size, ) )

right,, this one just works. don't remeber what I wrote but not as neat as this one.

components/clp-package-utils/clp_package_utils/scripts/decompress.py

kirkrodrigues · 2024-07-18T15:08:03Z

components/clp-package-utils/clp_package_utils/general.py

 from strenum import KebabCaseStrEnum

 # CONSTANTS
+DECOMPRESSION_COMMAND = "x"


How about:

EXTRACT_FILE_CMD and EXTRACT_IR_CMD

handle_extract_file_cmd and handle_extract_ir_cmd

components/clp-package-utils/clp_package_utils/scripts/native/decompress.py

kirkrodrigues · 2024-07-18T15:13:34Z

components/clp-package-utils/clp_package_utils/scripts/native/decompress.py

+
+    group = ir_extraction_parser.add_mutually_exclusive_group(required=True)
+    group.add_argument("--orig-file-id", type=str, help="Original file's ID.")
+    group.add_argument("--path", type=str, help="Original file's path.")


How about --orig-file-path? Makes it more consistent with the other arg.

…ess.py Co-authored-by: Junhao Liao <junhao@junhao.ca>

components/clp-package-utils/clp_package_utils/general.py

…_fork into ir_extraction_script

kirkrodrigues

For the PR title, how about:

clp-package: Add support for extracting files as IR streams through the CLI.

…he CLI. (y-scope#472)

haiqi96 added 3 commits July 5, 2024 11:58

Support ir extraction in decompression script

3be1e72

Merge branch 'main' into ir_extraction_backup

531c378

Linter

e2aa05f

haiqi96 commented Jul 6, 2024

View reviewed changes

junhaoliao reviewed Jul 11, 2024

View reviewed changes

haiqi96 marked this pull request as ready for review July 11, 2024 14:48

haiqi96 and others added 2 commits July 11, 2024 10:49

Apply suggestions from code review

e403604

Co-authored-by: Junhao Liao <junhao@junhao.ca>

fixes

93e23dd

junhaoliao reviewed Jul 15, 2024

View reviewed changes

components/clp-package-utils/clp_package_utils/scripts/decompress.py Outdated Show resolved Hide resolved

haiqi96 and others added 4 commits July 15, 2024 14:57

Merge branch 'main' into ir_extraction_script

adbffc5

revert fix that has already been done.

ef47de3

Merge branch 'main' into ir_extraction_script

84f2993

Refactor some comments, strings and whitespace; Add missing fmt: off;…

4eb049a

… Make command handlers return consistently rather than throwing exceptions in some cases.

kirkrodrigues requested changes Jul 18, 2024

View reviewed changes

haiqi96 and others added 7 commits July 18, 2024 19:20

address review concerns

ed65ed3

linter

ae13f27

address code review concerns

32ea43f

Update components/clp-package-utils/clp_package_utils/scripts/decompr…

63e9693

…ess.py Co-authored-by: Junhao Liao <junhao@junhao.ca>

Bug-fix: is not None -> is None; Make other None checks consistent.

74917e3

Rename job DECMOPRESSION -> FILE_EXTRACTION.

56987a7

Rename more instances of decompression to extract.

72aeafe

kirkrodrigues requested changes Jul 19, 2024

View reviewed changes

components/clp-package-utils/clp_package_utils/general.py Show resolved Hide resolved

haiqi96 added 4 commits July 18, 2024 23:52

revert unexpected change

070aa1e

Merge branch 'ir_extraction_script' of https://github.com/haiqi96/clp…

75a5f15

…_fork into ir_extraction_script

revert unexpected change and apply linter

4e1e667

Add missing docstrings and error check

b3d3000

haiqi96 added 2 commits July 19, 2024 00:24

Add missing docstrings and error check

da1deef

fix

65e5631

kirkrodrigues approved these changes Jul 19, 2024

View reviewed changes

haiqi96 changed the title ~~Support ir extraction in decompression script~~ clp-package: Add support for extracting files as IR streams through the CLI. Jul 19, 2024

haiqi96 merged commit 1d71b6f into y-scope:main Jul 19, 2024

jackluo923 pushed a commit to jackluo923/clp that referenced this pull request Dec 4, 2024

clp-package: Add support for extracting files as IR streams through t…

236ea17

…he CLI. (y-scope#472)

haiqi96 deleted the ir_extraction_script branch December 6, 2024 20:32

		logger.error(f"job {job_id} finished with unexpected status: {job_status}")


		async def do_extract(

clp-package: Add support for extracting files as IR streams through the CLI. #472

clp-package: Add support for extracting files as IR streams through the CLI. #472

Uh oh!

Conversation

haiqi96 commented Jul 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Validation performed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

haiqi96 commented Jul 6, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

junhaoliao commented Jul 11, 2024

Uh oh!

haiqi96 commented Jul 11, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kirkrodrigues left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

haiqi96 commented Jul 5, 2024 •

edited

Loading

haiqi96 commented Jul 6, 2024 •

edited

Loading