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

Describe workflow intefrace #644

Merged
merged 21 commits into from
Sep 18, 2024
Merged

Conversation

EmilyGavrilenko
Copy link
Contributor

@EmilyGavrilenko EmilyGavrilenko commented Sep 14, 2024

Description

Adding capability to describe workflow interface - both inputs and outputs regarding types of data:

The following workflow:

{
    "version": "1.0",
    "inputs": [
        {"type": "WorkflowImage", "name": "image"},
        {"type": "WorkflowParameter", "name": "model_id"},
    ],
    "steps": [
        {
            "type": "ObjectDetectionModel",
            "name": "general_detection",
            "image": "$inputs.image",
            "model_id": "$inputs.model_id",
            "class_filter": ["dog"],
        },
        {
            "type": "Crop",
            "name": "cropping",
            "image": "$inputs.image",
            "predictions": "$steps.general_detection.predictions",
        },
        {
            "type": "ClassificationModel",
            "name": "breds_classification",
            "image": "$steps.cropping.crops",
            "model_id": "dog-breed-xpaq6/1",
        },
    ],
    "outputs": [
        {
            "type": "JsonField",
            "name": "detections",
            "selector": "$steps.general_detection.predictions",
        },
        {
            "type": "JsonField",
            "name": "crops",
            "selector": "$steps.cropping.crops",
        },
        {
            "type": "JsonField",
            "name": "classification",
            "selector": "$steps.breds_classification.*",
        },
    ],
}

will return:

{
    "inputs": {
        "image": ["image"],
        "model_id": ["roboflow_model_id"],
    },
    "outputs": {
        "detections": ["object_detection_prediction"],
         "crops": ["image"],
         "classification": {
            "inference_id": ["string"],
            "predictions": ["classification_prediction"],
         },
    },
    "typing_hints": {
        "image": "dict",
        "roboflow_model_id": "str",
        "object_detection_prediction": "dict",
        "string": "str",
        "classification_prediction": "dict",
    },
    "kinds_schemas": {
        "image": {},
        "object_detection_prediction": {"dict": "with OpenAPI 3.0 schema of result"},
        "classification_prediction": {"dict": "with OpenAPI 3.0 schema of result"}
    }
}

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

How has this change been tested, please provide a testcase or example of how you tested the change?

  • CI still 🟢
  • new tests
  • e2e tests

Any specific deployment considerations

For example, documentation changes, usability, usage/costs, secrets, etc.

Docs

  • Docs updated? What were the changes:

Copy link
Collaborator

@PawelPeczek-Roboflow PawelPeczek-Roboflow left a comment

Choose a reason for hiding this comment

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

the same can be achieved having schemas of blocks on the UI end - why secondary request to backend to run post-processing of schemas needed?

@EmilyGavrilenko
Copy link
Contributor Author

EmilyGavrilenko commented Sep 16, 2024

the same can be achieved having schemas of blocks on the UI end - why secondary request to backend to run post-processing of schemas needed?

A customer requested an API endpoint to call to get the schema for the workflow response (slack thread). Will be used for integration with their downstream pipelines

@PawelPeczek-Roboflow
Copy link
Collaborator

ok then - let's mark it to be released in 0.19.0

@PawelPeczek-Roboflow
Copy link
Collaborator

taking that and preparing for rel

@@ -956,6 +960,41 @@ async def infer_lmm(

if not DISABLE_WORKFLOW_ENDPOINTS:

@app.post(
Copy link
Collaborator

Choose a reason for hiding this comment

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

that needs to be added into hosted API routing

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@EmilyGavrilenko is the customer requesting this running on hosted API or dedicates/local?

Copy link
Collaborator

Choose a reason for hiding this comment

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

that's hosted most likely given Nick comment
I posted this comment not to forget

Copy link
Collaborator

Choose a reason for hiding this comment

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

regardless - this is potentially useful feature for preview in UI

Copy link
Contributor

Choose a reason for hiding this comment

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

@PawelPeczek-Roboflow PawelPeczek-Roboflow changed the title Describe workflow output Describe workflow intefrace Sep 17, 2024
@PawelPeczek-Roboflow
Copy link
Collaborator

Added typing_hints and kinds_schemas. Not in the way I like that - would require re-iteration to ease maintenance, but works for now and is added aside of anything else, not requesting drastic changes.

kinds_schemas provide OpenAPI 3.0 schemas for entities, but obviously not for all, simple type hints for kinds are delivered undertyping_hints key

hansent
hansent previously approved these changes Sep 18, 2024
Copy link
Contributor

@hansent hansent left a comment

Choose a reason for hiding this comment

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

lgtm tested locally

we only need this for explicit spec? i.e. not able to get it for name workflow via workflow url?

@PawelPeczek-Roboflow
Copy link
Collaborator

we only need this for explicit spec? i.e. not able to get it for name workflow via workflow url? - not sure if I understand

@hansent
Copy link
Contributor

hansent commented Sep 18, 2024

we only need this for explicit spec? i.e. not able to get it for name workflow via workflow url? - not sure if I understand

like I have to post the workflow spec, but I can't post hte workflow url and have it looked up via API like I can for running it?

@PawelPeczek-Roboflow
Copy link
Collaborator

still do not understand - I feel like both are possible

Copy link
Contributor

@hansent hansent left a comment

Choose a reason for hiding this comment

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

reapproving after fixing block names

@PawelPeczek-Roboflow PawelPeczek-Roboflow merged commit 13332f8 into main Sep 18, 2024
58 checks passed
@PawelPeczek-Roboflow PawelPeczek-Roboflow deleted the describe-workflow-output branch September 18, 2024 18:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants