Skip to content
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.

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

Interactive apps #78

Merged
merged 51 commits into from
Jan 25, 2024
Merged

Interactive apps #78

merged 51 commits into from
Jan 25, 2024

Conversation

sverhoeven
Copy link
Member

@sverhoeven sverhoeven commented Sep 5, 2023

Adds endpoints for interactive applications (<30s).

Which can be run on a directory of a completed job and are configured in yaml file with JSON schema and Jinja template.

Refs haddocking/haddock3#686

image

This PR also replaces the dynamic routes for application to static ones in the openapi spec.

@sverhoeven sverhoeven mentioned this pull request Sep 5, 2023
@sverhoeven
Copy link
Member Author

Notes for self for current implementation

Pros

  • Can use job directory without having to copy stuff around
    Cons:
  • Different then rest of bonvinlab apps.
  • Trust interactiveapp to not do anything wrong in job directory.

Alternative impl

Each app will have own endpoint, /api/job/{jobid}/interactive/rescor instead of /api/job/{jobid}/interactive/{application}, with input schema exposed to openapi spec.

Pro:

  • more api gateway like
    Cons:
  • Each interactiveapp will have own openapi endpoint.
    The generated openapi client will be specific to bartender instance.
    To use bartender with haddock3-webapp (which has generated client)
    you already need to configure haddock3 as application in config.yaml
    so this is not a big deal.

@sverhoeven
Copy link
Member Author

sverhoeven commented Nov 14, 2023

Now we have 3 dynamic endpoints:

  • GET /api/job/{jobid}/interactive
  • GET /api/job/{jobid}/interactive/{application}
  • POST /api/job/{jobid}/interactive/{application}

These could be replaced with an endpoint for each interactive app. For example for rescore interactive app it could be:

  • POST /api/job/{jobid}/interactive/rescore with config.input schema as request body schema.

By calling below to lifespan

def unroll_interactive_app_routes(app: FastAPI) -> None:
    """Unroll interactive app routes.

    Replaces `/api/job/{jobid}/interactive/{application}` endpoint with
    Loop over config.interactive_applications and add a post route for each

    Args:
        app: FastAPI app
    """
    if app.openapi_schema:
        return app.openapi_schema
    app.openapi()
    interactive_applications = cast(
        InteractiveApplicationConfigurations, app.state.config.interactive_applications
    )
    existing_post_path = app.openapi_schema["paths"]["/api/job/{jobid}/interactive/{application}"][
        "post"
    ]
    for iname, config in interactive_applications.items():
        path = f"/api/job/{{jobid}}/interactive/{iname}"
        post = {
                "tags": ["interactive"],
                "operationId": f"interactive_{iname}",
                "parameters": [
                    {
                        "name": "jobid",
                        "in": "path",
                        "required": True,
                        "schema": {"type": "string"},
                    },
                    {
                        "name": "body",
                        "in": "body",
                        "required": True,
                        "schema": config.input,
                    },
                ],
                "responses": existing_post_path["responses"],
                "security": existing_post_path["security"],
            }
        if config.summary is not None:
            post["summary"] = config.summary
        if config.description is not None:
            post["description"] = config.description
        app.openapi_schema["paths"][path] = {
            "post": post
        }

Pros:

  • From consumer perspective it is easier to see which interactive apps is available. You can just look at openapi spec. You dont have to call to 2 get endpoints
  • Request body is typed in openapi. So generated clients are also typed.

Cons:

  • Each instance of bartender web service has own openapi spec. Any web app using bartender would need to generate a client specific for an instance. Making it harder to deploy in different ways.

@sverhoeven sverhoeven marked this pull request as ready for review November 15, 2023 11:04
@sverhoeven
Copy link
Member Author

When the openapi spec has endpoints named after the interactive applications then it makes sense to do the same for the applications as well.

For example with haddock3 application the current openapi looks like

  • GET /api/application -- returns list of configured application names
  • GET /api/application/{application} -- for an application returns the config filename that should be submitted and optionally which roles a user should have
  • POST /api/application/{application} -- for an application submit an archive file

When each application has endpoint it would look like

  • GET /api/application/haddock3 -- for haddock3 application returns the config filename that should be submitted and optionally which roles a user should have
  • POST /api/application/haddock3 -- submit a haddock3 archive file

upload should be typed as blob, but changes locally elsewhere cause it to become any|null.
Which is invalid client code.

To fix had to make sure routes with fileresponse have correct response content type.

TODO
- When extra application is added then generated client has any|null type for last app.
@sverhoeven
Copy link
Member Author

When the openapi spec has endpoints named after the interactive applications then it makes sense to do the same for the applications as well.

For example with haddock3 application the current openapi looks like

  • GET /api/application -- returns list of configured application names
  • GET /api/application/{application} -- for an application returns the config filename that should be submitted and optionally which roles a user should have
  • POST /api/application/{application} -- for an application submit an archive file

When each application has endpoint it would look like

  • GET /api/application/haddock3 -- for haddock3 application returns the config filename that should be submitted and optionally which roles a user should have
  • POST /api/application/haddock3 -- submit a haddock3 archive file

Implemented this, see screenshot in PR description, but without GET routes as they are not needed.

@sverhoeven sverhoeven mentioned this pull request Jan 12, 2024
@sverhoeven
Copy link
Member Author

There is enough stuff in this PR lets merge it.

@sverhoeven sverhoeven merged commit d9833fc into main Jan 25, 2024
5 checks passed
@sverhoeven sverhoeven deleted the interactive_apps branch January 25, 2024 14:15
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.

1 participant