Skip to content

feat(diracx-api): add set_job_status#777

Closed
Loxeris wants to merge 1 commit into
DIRACGrid:mainfrom
Loxeris:job-status-api
Closed

feat(diracx-api): add set_job_status#777
Loxeris wants to merge 1 commit into
DIRACGrid:mainfrom
Loxeris:job-status-api

Conversation

@Loxeris
Copy link
Copy Markdown
Member

@Loxeris Loxeris commented Feb 9, 2026

Adds the set_job_status function to set the status, minor status and application status of a job.

Needed for issue DIRACGrid/dirac-cwl#83

Copy link
Copy Markdown
Contributor

@aldbr aldbr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly based on the comments I gave in your other PR in dirac-cwl.

"""Set the status of a job.

:param job_id: Target Job ID
:type job_id: str
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type is not necessary as you already specify the the type hint within the signature of the function.


@with_client
async def set_job_status(
job_id: str,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think job_id is expected to be an int

Suggested change
job_id: str,
job_id: int,

)
result = await client.jobs.set_job_statuses(body, force=force)
if result.success:
logger.debug("Job statuses set successfully")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it should be singular here.

Suggested change
logger.debug("Job statuses set successfully")
logger.debug("Job status set successfully")

@DIRACGridBot DIRACGridBot marked this pull request as draft February 10, 2026 09:05
@chaen
Copy link
Copy Markdown
Contributor

chaen commented Feb 10, 2026

why not just calling the client directly ? There does not seem to be any added logic here ?

@chaen
Copy link
Copy Markdown
Contributor

chaen commented Feb 11, 2026

@aldbr maybe we should be more explicit in docs/devs/explanations/components/api.md ?

@aldbr
Copy link
Copy Markdown
Contributor

aldbr commented Feb 11, 2026

why not just calling the client directly ? There does not seem to be any added logic here ?

There is actually a very small piece of logic to create and add the timestamp to the request here https://github.com/DIRACGrid/diracx/pull/777/changes#diff-2d32214b7bab7d5a88d21308ad744235df705a7ff238ac879292c6b2a4a364c3R166-R167

In DIRAC, we have:

  • a client that can set a single job status in one call (we don't have this in diracx, which is fine but then to reproduce that we need a small function to create the timestamp as done here). It doesn't seem to be called though.
  • a JobReport object that adds up different status update before sending them in one call. This is used by the JobAgent and the JobWrapper.

Here I see 2 options for the new job wrapper:

  • this diracx-api function to report the status of a given job each time there is an update. It's simple but can create a lot more calls to the services than the JobReport approach though.
  • we recreate a JobReport object, that should likely be located on the client side since the JobWrapper does not have a direct access to the DBs and only relies on client dependencies.

Do you have any opinion?

@aldbr
Copy link
Copy Markdown
Contributor

aldbr commented Feb 17, 2026

Note: I am closing this PR, we are going to work on the JobReport design and implementation within dirac-cwl as it's the only place it's going to be used for now.

@aldbr aldbr closed this Feb 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants