feat: LHCb modules migration by AcquaDiGiorgio · Pull Request #97 · DIRACGrid/dirac-cwl

AcquaDiGiorgio · 2026-02-04T15:27:36Z

First approach, needs review.

I tried to use the current Mock classes (MockDataManager), but I don't think this is the correct way. However, it should not vary much.

I open it as a draft because I still need to create 2 more tests and change the interactions with DIRAC.

AcquaDiGiorgio · 2026-02-11T15:36:34Z

There is an issue with the FailoverRequest.
The DIRAC's workflow, during the finalize phase calls sendFailoverRequest, which through the ReqClient sends the stored requests at the workflow_commons dictionary to DIRAC.
Currently, in dirac-cwl we don't have persitency between command executions, so the operations created are lost.

AcquaDiGiorgio · 2026-04-16T13:16:57Z

This PR will now tackle the migration of every single module (starting from the ones used in MCSim) by importing LHCb specific functions instead of rewritting them from the ground up

First approach, needs review

Improve UploadLogFile tests

chore: Create proper wf_commons file update

aldbr · 2026-06-04T12:53:17Z

+                logger.info("Preparing DISET request for %s", bk_file)
+
+        logger.info("Creating DISABLE_WATCHDOG_CPU_WALLCLOCK_CHECK in order to disable the Watchdog")
+        with open("DISABLE_WATCHDOG_CPU_WALLCLOCK_CHECK", "w") as f:


When you create new files like this, please make sure they are created in job_path.
In the PushJobAgent, we might have multiple job wrappers running in parallel in the future, you want to make sure they will not collide.

Suggested change

with open("DISABLE_WATCHDOG_CPU_WALLCLOCK_CHECK", "w") as f:

with open(job_path / "DISABLE_WATCHDOG_CPU_WALLCLOCK_CHECK", "w") as f:

aldbr · 2026-06-04T12:53:47Z

+class UploadOutputData(PostProcessCommand):
+    """Registers every output generated to the corresponding SE and Catalog or to the FailoverSE in case of failure."""
+
+    def _execute(self, job_path: os.PathLike, workflow_commons: WorkflowCommons, **kwargs) -> None:


Out of curiosity, why os.PathLike here and not pathlib.Path?

As far as I'm aware, the recommended way of typing path parameters is by using os.PathLike, which reminds me that that should be Union[str, os.PathLike], as str is not a subtype of os.PathLike.
On the other hand, I mostly use pathlib.Path for file operations, so maybe is not a bad idea to just use pathlib.

Ok I didn't know. From what I read, os.PathLike[str] is abstract and useful in your specific context for typing, but then you can (should?) "cast" it as a pathlib.Path to manipulate it.
Now given that we completely control this area of the code, I naively assume that having pathlib.Path is safe.
But I would happily follow your recommendation 🙂

aldbr · 2026-06-04T12:55:08Z

+
+        # Write to file
+        bfilename = f"bookkeeping_{step_commons.id}.xml"
+        with open(bfilename, "wb") as bfile:


For instance, here you also want this to be part of job_path, let's write it explicitly may be.

Suggested change

with open(bfilename, "wb") as bfile:

with open(job_path / bfilename, "wb") as bfile:

aldbr · 2026-06-04T13:00:22Z

+        except SErrorException as e:
+            logger.error("Failed to list the log directory\n%s", e)
+
+        if file_list:


What happens if an exception is raised? I assume file_list would not exist and you would get another exception?
Can you test that please?

That's true, I missed that. I will check every newly added returnValueOrRaise, as it changes the code blocks.

aldbr · 2026-06-04T14:06:11Z

+                value = returnValueOrRaise(workflow_commons.file_report.generateForwardDISET())
+                if not value:
+                    logger.info("On second attempt, files correctly reported to TransformationDB")
+                elif workflow_commons.step_status == StepStatus.Done:


Also based on my previous comment in the WorkflowAccounting: it would be interesting to see whether there is a way to get the status of the cwl execution vs a step execution.

Because if this is possible, then may be there is a way to reproduce exactly what we have in the workflow modules with the conditions like if workflowStatus and jobStatus...

aldbr · 2026-06-04T14:59:04Z

+                f"Values for StepAccounting are wrong. Here are the given data: {data_dict}"
+            ) from e
+
+        workflow_commons.dsc.addRegister(job_step)


Are you sure it works?

Why wouldn't it?
When the command execution ends, the registers are extracted from the client and stored for later use.

aldbr · 2026-06-05T07:22:43Z

+    _request = PrivateAttr(default=None)
+    _failover_request = PrivateAttr(default=None)
+    _job_report = PrivateAttr(default=None)
+    _file_report = PrivateAttr(default_factory=FileReport)
+    _data_manager = PrivateAttr(default_factory=DataManager)
+    _bk_client = PrivateAttr(default_factory=BookkeepingClient)
+    _dsc = PrivateAttr(default_factory=DataStoreClient)


I would not carry the clients, I would only add the clients in the commands where needed. Why did you decide to have all them here?

Suggested change

_request = PrivateAttr(default=None)

_failover_request = PrivateAttr(default=None)

_job_report = PrivateAttr(default=None)

_file_report = PrivateAttr(default_factory=FileReport)

_data_manager = PrivateAttr(default_factory=DataManager)

_bk_client = PrivateAttr(default_factory=BookkeepingClient)

_dsc = PrivateAttr(default_factory=DataStoreClient)

The jobReport could potentially be part of the signature of pre/post process commands though, because it's also used in the JobWrapper (we could imagine having it reporting the "application status" being the name of the pre/post process command - that would be in the base classes - and passing it to the commands so that they can influence the status of the job if needed).

My idea was to maintain the same structure and keep the code on each command as clean as possible.
However, I understand that instantiating all clients when most are not needed is unefficient.

The jobReport could potentially be part of the signature of pre/post process commands though, because it's also used in the JobWrapper

This could be a nice addition. Also, Ryan's job_wrapper creates a DataManager. We could give the possibility of setting the clients in CommandBase and if they are not present while executing the command, just instantatiate them.

aldbr · 2026-06-05T07:36:43Z

+    Failed = "Failed"
+
+
+class Step(BaseModel):


There might be a way of having one step per CWL workflow step.
Check the workflow examples we have here (this is not MCSim but it's very similar): https://gitlab.cern.ch/lhcb-dpa/analysis-productions/lbapcommon/-/blob/master/tests/example_workflows/complex_workflow_with_filtering/AnaProd_example_workflows_job_tuple%2Csplit%2Cfilter_B0.cwl?ref_type=heads

Okay, I will look into this

aldbr · 2026-06-05T07:38:43Z

Could you split that file into multiple smaller files please? One per command

AcquaDiGiorgio self-assigned this Feb 4, 2026

AcquaDiGiorgio linked an issue Feb 4, 2026 that may be closed by this pull request

LHCb Workflow: UploadLogFile command #87

Closed

4 tasks

AcquaDiGiorgio had a problem deploying to github-pages February 12, 2026 15:59 — with GitHub Actions Failure

AcquaDiGiorgio had a problem deploying to github-pages February 12, 2026 16:02 — with GitHub Actions Failure

AcquaDiGiorgio force-pushed the issue-87-LHCb-UploadLogFile branch from c3f5c70 to 93cb40c Compare February 16, 2026 11:34

aldbr linked an issue Apr 15, 2026 that may be closed by this pull request

Integrate real LHCb workflow commands (Pre/PostExecution) #67

Open

AcquaDiGiorgio changed the title ~~feat: UploadLogFile command implementation~~ feat: LHCb modules migration Apr 16, 2026

AcquaDiGiorgio added 6 commits April 27, 2026 16:16

feat: UploadLogFile command implementation

5075547

First approach, needs review

chore: improve UploadLogFile tests

7a03ef2

feat: Change UploadLogFile DataManager Mocks to real DIRAC Classes

fd12496

Improve UploadLogFile tests

chore: Update project name at imports

8317c9f

chore: setup lhcbdirac dependency to fork

91cef73

feat: Migrate BookkeepingReport command to cwl-dirac

98ccc37

AcquaDiGiorgio force-pushed the issue-87-LHCb-UploadLogFile branch from ca676da to 98ccc37 Compare April 27, 2026 14:20

AcquaDiGiorgio had a problem deploying to github-pages April 27, 2026 14:51 — with GitHub Actions Failure

chore: set lhcbdirac dependency to https instead of ssh

1da58a2

AcquaDiGiorgio force-pushed the issue-87-LHCb-UploadLogFile branch from 6511abe to 1da58a2 Compare April 27, 2026 14:51

AcquaDiGiorgio had a problem deploying to github-pages April 27, 2026 14:53 — with GitHub Actions Failure

chore: remove all DIRAC import mypy type checking

4586f84

AcquaDiGiorgio had a problem deploying to github-pages April 28, 2026 07:28 — with GitHub Actions Failure

AcquaDiGiorgio had a problem deploying to github-pages April 28, 2026 10:04 — with GitHub Actions Failure

AcquaDiGiorgio added 5 commits May 4, 2026 11:17

feat: Migrate FailoverRequest command to cwl-dirac

0ad8e0e

chore(tests): improve command fixtures

bd285c3

feat: Migrate UploadOutputData command to cwl-dirac

26de911

chore: Create proper wf_commons file update

feat: Migrate AnalyseXmlSummary command to cwl-dirac

b87c180

feat: Migrate WorkflowAccounting command to cwl-dirac

32e54b4

AcquaDiGiorgio had a problem deploying to github-pages May 6, 2026 12:20 — with GitHub Actions Failure

feat: Migrate UploadLogFile command to cwl-dirac

f02159a

AcquaDiGiorgio had a problem deploying to github-pages May 6, 2026 14:54 — with GitHub Actions Failure

chore: update pixi.lock

f4d2821

AcquaDiGiorgio had a problem deploying to github-pages May 7, 2026 07:30 — with GitHub Actions Failure

aldbr requested changes May 8, 2026

View reviewed changes

AcquaDiGiorgio added 4 commits May 11, 2026 12:36

chore: fix BookkeepingReport typo

028ca4f

chore: fix possible None values while saving workflow_commons

d9e24e9

chore: set proper commands exception catching

6907021

chore: fix job path not being taken into account

1987844

AcquaDiGiorgio had a problem deploying to github-pages May 15, 2026 13:56 — with GitHub Actions Failure

chore: change workflow commons from dict to a pydantic model

947bc8b

AcquaDiGiorgio force-pushed the issue-87-LHCb-UploadLogFile branch from c3b1414 to 947bc8b Compare May 15, 2026 14:29

AcquaDiGiorgio had a problem deploying to github-pages May 15, 2026 14:30 — with GitHub Actions Failure

chore: fix typos

396645e

AcquaDiGiorgio had a problem deploying to github-pages May 15, 2026 14:41 — with GitHub Actions Failure

chore: add logging to commands

a81648b

AcquaDiGiorgio had a problem deploying to github-pages May 18, 2026 15:01 — with GitHub Actions Failure

AcquaDiGiorgio had a problem deploying to github-pages June 4, 2026 10:06 — with GitHub Actions Failure

AcquaDiGiorgio added 3 commits June 4, 2026 12:18

chore: wrap command execute function

dcb1693

chore: use DataStoreClient private registersList attribute

2d16150

feat: Add step information for executions of multiple steps at a time

ed1c6f0

AcquaDiGiorgio force-pushed the issue-87-LHCb-UploadLogFile branch from 2e7ae71 to ed1c6f0 Compare June 4, 2026 10:19

AcquaDiGiorgio had a problem deploying to github-pages June 4, 2026 10:20 — with GitHub Actions Failure

chore: fix snake-case convention mismatch

3a8908d

AcquaDiGiorgio had a problem deploying to github-pages June 4, 2026 12:38 — with GitHub Actions Failure

aldbr requested changes Jun 5, 2026

View reviewed changes

	with open("DISABLE_WATCHDOG_CPU_WALLCLOCK_CHECK", "w") as f:
	with open(job_path / "DISABLE_WATCHDOG_CPU_WALLCLOCK_CHECK", "w") as f:

	with open(bfilename, "wb") as bfile:
	with open(job_path / bfilename, "wb") as bfile:

Conversation

AcquaDiGiorgio commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AcquaDiGiorgio commented Feb 11, 2026

Uh oh!

AcquaDiGiorgio commented Apr 16, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AcquaDiGiorgio commented Feb 4, 2026 •

edited

Loading