Skip to content

[9.0] feat: Migrating pilot client toward DiracX #8233

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: integration
Choose a base branch
from
Open

[9.0] feat: Migrating pilot client toward DiracX #8233

wants to merge 3 commits into from

Conversation

@Robin-Van-de-Merghel
Copy link
Contributor

@Robin-Van-de-Merghel Robin-Van-de-Merghel commented Jun 26, 2025 •
edited
Loading

BEGINRELEASENOTES

*PilotManager
CHANGE: Migrate pilot client toward DiracX

ENDRELEASENOTES

@Robin-Van-de-Merghel Robin-Van-de-Merghel changed the title feat: Migrating client toward DiracX feat: Migrating pilot client toward DiracX Jun 27, 2025
@fstagni fstagni changed the title feat: Migrating pilot client toward DiracX [9.0] feat: Migrating pilot client toward DiracX Jun 27, 2025
@Robin-Van-de-Merghel Robin-Van-de-Merghel mentioned this pull request Jul 1, 2025
Closed
fstagni
fstagni reviewed Jul 1, 2025
View reviewed changes
src/DIRAC/WorkloadManagementSystem/scripts/dirac_admin_add_pilot.py Outdated
gLogger.error(res["Message"])
DIRACExit(1)
res = pmc.addPilotReferences([pilotRef], VO, gridType, {pilotRef: pilotStamp})
res = pmc.addPilotReferences(pilotStamp, VO, gridType, {pilotStamp: pilotRef})
Copy link
Contributor

@fstagni fstagni Jul 1, 2025

Choose a reason for hiding this comment

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

Isn't it breaking things?

Copy link
Contributor Author

@Robin-Van-de-Merghel Robin-Van-de-Merghel Jul 2, 2025

Choose a reason for hiding this comment

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

Typo on the left : [pilotStamp]. But I reversed stamps and pilotRef to be consistent with DiracX changes.

Copy link
Contributor

@fstagni fstagni Jul 2, 2025

Choose a reason for hiding this comment

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

Please use named arguments instead of positional arguments.

@Robin-Van-de-Merghel
Copy link
Contributor Author

Robin-Van-de-Merghel commented Jul 15, 2025

Issue with GetPilotOutput: as of now, we cannot use the new client with the old client at the same time (to help migrating a client bit by bit).

So, if a user calls GetPilotOutput, it will responds with an error because my new client does not have it.

@fstagni
Copy link
Contributor

fstagni commented Jul 15, 2025

Issue with GetPilotOutput: as of now, we cannot use the new client with the old client at the same time (to help migrating a client bit by bit).

So, if a user calls GetPilotOutput, it will responds with an error because my new client does not have it.

I think it's fine. This functionality (storing the pilot output in a MySQL blob) is already borderline useful, and given that we are finally getting somewhere with the pilot logging in diracx, I am thinking it would be OK to remove it. If for the moment replying with an error is OK.

@Robin-Van-de-Merghel Robin-Van-de-Merghel mentioned this pull request Jul 17, 2025
Draft
@Robin-Van-de-Merghel Robin-Van-de-Merghel mentioned this pull request Jul 7, 2025
Open
18 tasks
@Robin-Van-de-Merghel Robin-Van-de-Merghel force-pushed the robin-migrate-client branch 6 times, most recently from 8114684 to 2410060 Compare August 5, 2025 08:08
@Robin-Van-de-Merghel Robin-Van-de-Merghel marked this pull request as ready for review August 5, 2025 09:42
This was referenced Aug 7, 2025
Open
Open
fstagni
fstagni reviewed Aug 7, 2025
View reviewed changes
src/DIRAC/Interfaces/scripts/dirac_admin_get_pilot_output.py
diracAdmin = DiracAdmin()
exitCode = 0
errorList = []
print("This command is not supported anymore with DIRAV V9.")
Copy link
Contributor

@fstagni fstagni Aug 7, 2025

Choose a reason for hiding this comment

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

Suggested change
print("This command is not supported anymore with DIRAV V9.")
print("This command is not supported anymore with DIRAC V9.")

fstagni
fstagni reviewed Aug 7, 2025
View reviewed changes
src/DIRAC/WorkloadManagementSystem/Service/PilotManagerHandler.py

@classmethod
def export_deletePilots(cls, pilotIDs):
# Used by no one. We keep it for tests.
Copy link
Contributor

@fstagni fstagni Aug 7, 2025

Choose a reason for hiding this comment

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

Nah, if it is not used just remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fstagni fstagni fstagni left review comments

@atsareg atsareg Awaiting requested review from atsareg atsareg is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Robin-Van-de-Merghel @fstagni