Skip to content

Conversation

@provokateurin
Copy link
Member

No description provided.

Signed-off-by: provokateurin <kate@provokateurin.de>
Copy link
Contributor

@andrey18106 andrey18106 left a comment

Choose a reason for hiding this comment

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

Didn't test, but LGTM

Copy link
Member

@marcelklehr marcelklehr left a comment

Choose a reason for hiding this comment

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

Conceptually, I would say the new task type should be independent from the provider. E.g. one app could register multiple providers for the same custom task type. Also, one app could want to register only a task type for a task to be provided by other apps.

@bigcat88
Copy link
Member

Conceptually, I would say the new task type should be independent from the provider. E.g. one app could register multiple providers for the same custom task type.

Will this implementation be enough at this stage?
This does not require creating a new table, and is simpler from a API point of view, IMHO.

@bigcat88
Copy link
Member

bigcat88 commented Jul 13, 2024

import contextlib
from os import environ
from pathlib import Path

import nc_py_api

environ["APP_ID"] = "nc_py_api"
environ["APP_VERSION"] = "1.0.0"
environ["APP_SECRET"] = "12345"


if __name__ == "__main__":
    nc = nc_py_api.NextcloudApp(nextcloud_url="http://nextcloud.local")
    nc.set_user("admin")

    nc.providers.task_processing.register(
        "visionatrix",
        "SDXL Lighting",
        "visionatrix:sdxl_lighting",
        {
            "id": "visionatrix:sdxl_lighting",
            "name": "SDXL Lighting",
            "description": "Fast Image Generation",
            "input_shape": {
                "prompt": {
                    "name": "prompt",
                    "description": "Prompt",
                    "type": 1,  # Text
                }
            },
            "output_shape": {
                "image": {
                    "name": "image",
                    "description": "Image",
                    "type": 2,  # Image
                }
            }
        }
    )
    s1 = nc.ocs(
        "POST",
        "/ocs/v2.php/taskprocessing/schedule",
        json={
            "input": {"prompt": "Star"},
            "type": "visionatrix:sdxl_lighting",
            "appId": "nc_py_api",
        }
    )
    print(s1)

    my_task = nc.providers.task_processing.next_task(["visionatrix"], ["visionatrix:sdxl_lighting"])
    print(my_task)

Description of idea(that is in this PR): if one ExApp provides multiple custom_task_types for each custom task_type it calls "Register Provider" (as anyway "Optional Shapes" are binded to Provider struct)

I am still in process of reviewing and learning how to use our new API..

Copy link
Member

@bigcat88 bigcat88 left a comment

Choose a reason for hiding this comment

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

PR is fine for me, but I can't make it work with the Assistant app.

No further actions is needed(until someone want to help find to which repo the problem belongs), current idea is to take a much deeper look at this at Monday/Tuesday and find the reason.

Copy link
Member

@bigcat88 bigcat88 left a comment

Choose a reason for hiding this comment

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

Ok, it was a bug in the Assistant app - it was fixed 20 minutes ago thanks to Julien for fast reacting.

bigcat88 pushed a commit to cloud-py-api/nc_py_api that referenced this pull request Jul 13, 2024
Counter part for nextcloud/app_api#324

---------

Signed-off-by: provokateurin <kate@provokateurin.de>
@provokateurin provokateurin merged commit 636d7b8 into main Jul 14, 2024
@provokateurin provokateurin deleted the feat/task-processing/custom-task-types branch July 14, 2024 07:07
@marcelklehr
Copy link
Member

Will this implementation be enough at this stage?

I'm uncertain on this. think the goal of ExApps was to have stable APIs. Having this one in a half baked state does not contribute to that goal. On the other hand it may be possible to extend this implementation in the future. But then I don't see when we would do that. IMO it would be good to have this implemented properly, but I defer to @julien-nc

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.

5 participants