fractal-analytics-platform / fractal-server

Fractal backend
https://fractal-analytics-platform.github.io/fractal-server/
BSD 3-Clause "New" or "Revised" License
10 stars 3 forks source link

Fix conditional inclusion of SSH task-collection endpoints (in code and/or testing) #1600

Closed tcompa closed 2 weeks ago

tcompa commented 3 weeks ago

Right now, we are using an import-time check of settings to determine whether the standard or SSH task-collection endpoints should be included in the API.


router_api_v2 = APIRouter()

router_api_v2.include_router(dataset_router_v2, tags=["V2 Dataset"])
router_api_v2.include_router(job_router_v2, tags=["V2 Job"])
router_api_v2.include_router(images_routes_v2, tags=["V2 Images"])
router_api_v2.include_router(project_router_v2, tags=["V2 Project"])
router_api_v2.include_router(submit_job_router_v2, tags=["V2 Job"])

settings = Inject(get_settings)
if settings.FRACTAL_RUNNER_BACKEND == "slurm_ssh":
    router_api_v2.include_router(
        task_collection_router_v2_ssh,
        prefix="/task",
        tags=["V2 Task Collection"],
    )
else:
    router_api_v2.include_router(
        task_collection_router_v2, prefix="/task", tags=["V2 Task Collection"]
    )
router_api_v2.include_router(task_router_v2, prefix="/task", tags=["V2 Task"])
...

This can be improved, and for sure it doesn't play well with this fixture:

@pytest.fixture
async def register_routers(app, override_settings):
    from fractal_server.main import collect_routers

    collect_routers(app)

TO DO: do not perform the settings check at import time.

tcompa commented 3 weeks ago

Note that this also the reason for how we wrote test_exclude_v1_api (as part of https://github.com/fractal-analytics-platform/fractal-server). At the moment we don't have a nice way to load the app after updating settings.

tcompa commented 3 weeks ago

One option here is to have the two endpoints (ssh or non-ssh) share the exact same signature, and then switch from one to the other from within the endpoint code.