Feature: get and assign sandbox to job

[natthan-pigoux]

The features have been started but not finished yet.
TBF:

• : parse the query result to fit routes expectation
• : then finish "get" routes
• : write the test

[natthan-pigoux]

Hello @chrisburr, I have moved on for the "assign" and "get" routes. But the test return a 404 not found while calling these routes and I did'nt found why... Do you have an idea on this?
Thank you!

[chaen]

Hi @natthan-pigoux ,
General guideline:

You can run the test locally with the --pdb option
pytest --pdb -k test_assign_sandbox_to_job
You will then end up in a prompt that allows you to debug.

A naive guess would be that the job id does not exist

[chaen]

I think the 404 problem comes from here. the pfn is not in the query but in the body

[natthan-pigoux]

Thanks @chaen , I simply try that: c968b65
I've tried the Pydantic syntax as well, but it doesn't solve the issue... I also checked that the job and the sandbox are created in the test and it seems to be the case.

[chaen]

if you declare pfn as an str, then you need to do your test like

r = normal_user_client.patch(
        f"/api/jobs/{job_id}/sandbox/output",
        json=sandbox_pfn,
)

(although probably the Pydantic model approach is better).
But this would result in 422, not 404

try to replace

    assert r.status_code == 200

with

    assert r.status_code == 200, r.json()

so you get to see the actual error.

If after that you still can't figure out, I'll checkout your branch and try on my machine :-)

[natthan-pigoux]

Yes, I modified the sandbox_pfn declaration. I've investigated but I didn't find any obvious solution. Could be helpful if you can have a look :)

[chaen]

OK so the problem was a bit more subtle :-)
You declare your routes like this for example

@router.get("/jobs/{job_id}/sandbox")

However, the /jobs prefix is already automatically prepended when creating the job router (this comes from that, and yes, that is part of the missing docs :-) )

diracx/setup.cfg

Line 88 in 3b4761c

jobs = diracx.routers.job_manager:router

So the way you need to declare your routes is not `/jobs/{job_id}/sandbox" but just "/{job_id}/sandbox"

Effectively, your route is available as "/jobs/jobs/{job_id}/sandbox"

So changing the route definition fixes the problem you have, but you bump then into another one, which I let in your care :-)

[natthan-pigoux]

Thanks a lot @chaen!
Indeed I could see that when looking other sandbox routes... I also fixed the pfn parameter by splitting it inside the assign_sandbox route. Is that the right way to do it ?
Otherwise the tests finally passed, the PR should be ready for review.

[chaen]

@natthan-pigoux thanks ! I'll just rebase it to pick up the latest changes :-)

[chaen]

@natthan-pigoux thanks a lot for the PR. There are a few TODO's left, in particular for identity check. Do you have time or should I finish it ? :-)

[natthan-pigoux]

Hi @chaen, for the todo list, are you talking about what's left in #13 or the TODOs I left in the code (such as: f04166a#diff-c24591c47e0251c9014801ea7df92eb36ff4d01a77c47d6a48a7c37e7c682f98R219) ?
Yes I have time to finish the work :)

[chaen]

The TODO in the code :-) Once these are adressed, I merge it and if you will, you can continue with the rest of #13 :-)

[natthan-pigoux]

@chaen, one of the todo is check that user has created the job or is admin , Can I assume the router definition: router = DiracxRouter(dependencies=[has_properties(NORMAL_USER | JOB_ADMINISTRATOR)]) gives the needed rights ?
I'm not sure what is the perimeter of NORMAL_USER

[natthan-pigoux]

@chaen, I added the method to unassign sandbox and added the concept of "entity" which was not implemented.
Still, I don't know how to manage the policy on sandbox, I suppose it is the same as for jobs. But it doesn't look like it is implemented for job right now.
Should I leave it as TODO for now?

[chaen]

yes, we are working on the policy at the moment, so don't bother with it :-)
Is the rest of the PR finished ?