JOJ/JOJ3-config-generator
27
1
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

dev #10

Merged
李衍志523370910113 merged 238 commits from dev into master 2025-03-05 16:20:39 +08:00
Conversation 16 Commits 238 Files Changed 42 +19 -24
3 changed files with 19 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 5094507112 - Show all commits

0
joj3_config_generator/lib/__init__.py → joj3_config_generator/processers/__init__.py
View File

0
joj3_config_generator/lib/repo.py → joj3_config_generator/processers/repo.py
View File

43
joj3_config_generator/lib/task.py → joj3_config_generator/processers/task.py
View File

@ -1,16 +1,7 @@
import shlex
jon-lee marked this conversation as resolved
张泊明518370910136 commented 2025-03-03 11:03:41 +08:00
Review
Copy Link

Path should not be relative to JOJ3_CONFIG_ROOT in this file, should be relative to task.toml dir

Path should not be relative to `JOJ3_CONFIG_ROOT` in this file, should be relative to `task.toml` dir
李衍志523370910113 commented 2025-03-03 11:11:32 +08:00
Review
Copy Link

I reckon you said things is relative to JOJ3_CONFIG_ROOT in JTC before. we have a task.type in task.toml to mend the path

I reckon you said things is relative to `JOJ3_CONFIG_ROOT` in JTC before. we have a `task.type` in `task.toml` to mend the path
张泊明518370910136 commented 2025-03-03 11:15:21 +08:00
Review
Copy Link

config.path is relative to JOJ3_CONFIG_ROOT.

`config.path` is relative to `JOJ3_CONFIG_ROOT`.
李衍志523370910113 commented 2025-03-03 13:32:29 +08:00
Review
Copy Link

could you explain further? I m not quite sure my understanding is clear.

could you explain further? I m not quite sure my understanding is clear.
张泊明518370910136 commented 2025-03-04 03:05:54 +08:00
Review
Copy Link

In joj3_config_generator/models/task.py, Config.path is relative to JOJ3_CONFIG_ROOT, so task.toml will located at JOJ3_CONFIG_ROOT / task_conf.path in JTC.

In `joj3_config_generator/models/task.py`, `Config.path` is relative to `JOJ3_CONFIG_ROOT`, so `task.toml` will located at `JOJ3_CONFIG_ROOT / task_conf.path` in JTC.
from typing import Tuple
jon-lee marked this conversation as resolved
张泊明518370910136 commented 2025-03-03 11:27:04 +08:00
Review
Copy Link

Some with_.update is still using raw dict, not model with model_dump.

Some `with_.update` is still using raw dict, not model with `model_dump`.
import rtoml
from joj3_config_generator.models import (
ExecutorConfig,
ExecutorWithConfig,
ParserConfig,
)
from joj3_config_generator.models.result import Cmd, CmdFile, OptionalCmd
from joj3_config_generator.models.result import Stage as ResultStage
from joj3_config_generator.models.task import Stage as TaskStage
from joj3_config_generator.models import joj1, repo, result, task
def remove_nulls(d: result.Config) -> result.Config:
@ -23,9 +14,9 @@ def remove_nulls(d: result.Config) -> result.Config:
def get_conf_stage(
jon-lee marked this conversation as resolved Outdated
李衍志523370910113 commented 2025-03-03 11:12:45 +08:00
Outdated
Review
Copy Link

BTW, is this outdated?

BTW, is this outdated?
张泊明518370910136 commented 2025-03-03 11:17:54 +08:00
Outdated
Review
Copy Link

Never heard about this rule.

Never heard about this rule.
李衍志523370910113 commented 2025-03-03 11:19:19 +08:00
Outdated
Review
Copy Link

@manuel what would be the current intended rule for group?

@manuel what would be the current intended rule for `group`?
李衍志523370910113 commented 2025-03-03 16:18:13 +08:00
Outdated
Review
Copy Link

seems current strategy is fine, resolved.

seems current strategy is fine, resolved.
task_stage: TaskStage, executor_with_config: ExecutorWithConfig
) -> ResultStage:
conf_stage = ResultStage(
task_stage: task.Stage, executor_with_config: result.ExecutorWith
) -> result.StageDetail:
conf_stage = result.StageDetail(
name=task_stage.name if task_stage.name is not None else "",
# TODO: we may have cq in future
group=(
@ -33,12 +24,12 @@ def get_conf_stage(
if (task_stage.name is not None) and ("judge" in task_stage.name)
else None
),
executor=ExecutorConfig(
executor=result.Executor(
name="sandbox",
with_=executor_with_config,
),
parsers=(
[ParserConfig(name=parser, with_={}) for parser in task_stage.parsers]
[result.Parser(name=parser, with_={}) for parser in task_stage.parsers]
if task_stage.parsers is not None
else []
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-03 11:54:43 +08:00
Outdated
Review
Copy Link

should loop through conf_stage.parsers here and update the with field according to the parser name.

should loop through `conf_stage.parsers` here and update the `with` field according to the parser name.
李衍志523370910113 commented 2025-03-03 13:36:23 +08:00
Outdated
Review
Copy Link

I think its already implemented in each of the fix_parsers functions

I think its already implemented in each of the `fix_parsers` functions
张泊明518370910136 commented 2025-03-03 13:43:12 +08:00
Outdated
Review
Copy Link

No, do not find the parser in the fix_xxx function. Instead, iterate through the parsers here and decide how to fill in the with.

No, do not find the parser in the `fix_xxx` function. Instead, iterate through the parsers here and decide how to fill in the `with`.
李衍志523370910113 commented 2025-03-03 16:17:53 +08:00
Outdated
Review
Copy Link

resolved.

resolved.
张泊明518370910136 commented 2025-03-04 03:02:45 +08:00
Outdated
Review
Copy Link

Use a dict to store parser name, field, function to process.

    process_dict: Dict[
        str, Tuple[Callable[[result.ParserConfig, BaseModel], None], BaseModel]
    ] = {
        "clangtidy": (fix_keyword, task_stage.clangtidy),
        "keyword": (fix_keyword, task_stage.keyword),
        "diff": (fix_diff, task_stage.diff),
    }
    for i, parser in enumerate(task_stage.parsers):
        if parser in process_dict:
            func, parser_model = process_dict[parser]
            func(conf_stage.parsers[i], parser_model)
Use a dict to store parser name, field, function to process. ``` process_dict: Dict[ str, Tuple[Callable[[result.ParserConfig, BaseModel], None], BaseModel] ] = { "clangtidy": (fix_keyword, task_stage.clangtidy), "keyword": (fix_keyword, task_stage.keyword), "diff": (fix_diff, task_stage.diff), } for i, parser in enumerate(task_stage.parsers): if parser in process_dict: func, parser_model = process_dict[parser] func(conf_stage.parsers[i], parser_model) ```
李衍志523370910113 commented 2025-03-04 14:46:40 +08:00
Outdated
Review
Copy Link

resolved.

resolved.
),
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-05 03:51:41 +08:00
Outdated
Review
Copy Link

Do we need to support both kinds of names?

Do we need to support both kinds of names?
李衍志523370910113 commented 2025-03-05 08:42:25 +08:00
Outdated
Review
Copy Link

probably yes, since it is easy for new ta to type it wrong

probably yes, since it is easy for new ta to type it wrong
张泊明518370910136 commented 2025-03-05 09:00:28 +08:00
Outdated
Review
Copy Link

parsers name should be a str enum, force them to use the correct names

parsers name should be a str enum, force them to use the correct names
李衍志523370910113 commented 2025-03-05 09:09:46 +08:00
Outdated
Review
Copy Link

ok, then removed.

ok, then removed.
@ -47,8 +38,8 @@ def get_conf_stage(
def get_executorWithConfig(
task_stage: TaskStage, cached: list[str]
) -> Tuple[ExecutorWithConfig, list[str]]:
task_stage: task.Stage, cached: list[str]
) -> Tuple[result.ExecutorWith, list[str]]:
file_import = (
task_stage.files.import_
if hasattr(task_stage, "files")
@ -73,7 +64,7 @@ def get_executorWithConfig(
else []
),
copy_in={
file: CmdFile(src=f"/home/tt/.config/joj/{file}")
file: result.CmdFile(src=f"/home/tt/.config/joj/{file}")
for file in copy_in_files
},
copy_in_cached={file: file for file in copy_in_files},
@ -93,7 +84,7 @@ def get_executorWithConfig(
if task_stage.limit is not None and task_stage.limit.mem is not None
else 4 * 1_024 * 1_024
),
stderr=CmdFile(
stderr=result.CmdFile(
name="stderr",
max=(
task_stage.limit.stderr * 1_000_000_000
@ -102,7 +93,7 @@ def get_executorWithConfig(
else 4 * 1_024 * 1_024
),
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-05 03:58:08 +08:00
Outdated
Review
Copy Link

The reason for the suffix in keyword_parser_?

The reason for the suffix in `keyword_parser_`?
李衍志523370910113 commented 2025-03-05 08:42:46 +08:00
Outdated
Review
Copy Link

just forgot to remove, sorry

just forgot to remove, sorry
),
stdout=CmdFile(
stdout=result.CmdFile(
name="stdout",
max=(
task_stage.limit.stdout * 1_000_000_000
@ -154,7 +145,9 @@ def fix_keyword(
return conf_stage
def fix_result_detail(task_stage: TaskStage, conf_stage: ResultStage) -> ResultStage:
def fix_result_detail(
task_stage: task.Stage, conf_stage: result.StageDetail
) -> result.StageDetail:
if (task_stage.parsers is not None) and ("result-detail" in task_stage.parsers):
result_detail_parser = next(
p for p in conf_stage.parsers if p.name == "result-detail"
@ -212,7 +205,9 @@ def fix_dummy(
return conf_stage
def fix_diff(task_stage: TaskStage, conf_stage: ResultStage) -> ResultStage:
def fix_diff(
task_stage: task.Stage, conf_stage: result.StageDetail
) -> result.StageDetail:
if task_stage.parsers is not None and "diff" in task_stage.parsers:
jon-lee marked this conversation as resolved Outdated
张泊明518370910136 commented 2025-03-02 02:28:53 +08:00
Outdated
Review
Copy Link

Create models for these dicts, then update them with the dict from model_dump

Create models for these dicts, then update them with the dict from `model_dump`
李衍志523370910113 commented 2025-03-02 11:38:27 +08:00
Outdated
Review
Copy Link

@bomingzh I don't think we can change it. This is to make proper alias so that we can get the content of result-status.

@bomingzh I don't think we can change it. This is to make proper alias so that we can get the content of `result-status`.
张泊明518370910136 commented 2025-03-02 12:09:26 +08:00
Outdated
Review
Copy Link
class DummyConfig(BaseModel):
    score: int
    comment: str

Then dummy_parser_.with_.update(dummy_config(...).model_dump()).

``` class DummyConfig(BaseModel): score: int comment: str ``` Then `dummy_parser_.with_.update(dummy_config(...).model_dump())`.
李衍志523370910113 commented 2025-03-02 17:38:42 +08:00
Outdated
Review
Copy Link

fixed

fixed
diff_parser = next((p for p in conf_stage.parsers if p.name == "diff"), None)
skip = task_stage.skip or []
@ -245,8 +240,8 @@ def fix_diff(task_stage: TaskStage, conf_stage: ResultStage) -> ResultStage:
)
stage_cases.append(
OptionalCmd(
stdin=CmdFile(
result.OptionalCmd(
stdin=result.CmdFile(
src=f"/home/tt/.config/joj/{conf_stage.name}/{case}.in"
),
cpu_limit=cpu_limit,