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

feat: Add enum and default value support in task processing #359

Merged
merged 2 commits into from
Aug 12, 2024

Conversation

kyteinsky
Copy link
Collaborator

@kyteinsky kyteinsky commented Aug 8, 2024

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.

We have time to change this, since NC30 is not published yet - let's make everything simple and clear in AppAPI too.

I propose to transmit/receive everything in the structure that Anupam introduced(array $provider), without duplicating the same information in POST parameters as separate variables.

$name, $displayName, $taskType at leat should be removed as a parameters, maybe $customTaskType too.

@kyteinsky kyteinsky force-pushed the feat/enum-task-processing branch from 29069a1 to 8b72752 Compare August 9, 2024 14:05
composer.lock Outdated Show resolved Hide resolved
bigcat88 added a commit to cloud-py-api/nc_py_api that referenced this pull request Aug 11, 2024
All related PRs:
#284
nextcloud/app_api#359
nextcloud/translate2#11

---------

Signed-off-by: Anupam Kumar <[email protected]>
Signed-off-by: Alexander Piskun <[email protected]>
Co-authored-by: Alexander Piskun <[email protected]>
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.

I'd published nc_py_api

@marcelklehr will you have time to test this tommorow?

Anupam told me that he tested and it was working, from my point of view all is looking fine.

But as I understand this all is a breaking change in API and we need to publish all ExApp related to this in one day(or at least make commit to not break "Daily").

@marcelklehr
Copy link
Member

marcelklehr commented Aug 12, 2024

@marcelklehr will you have time to test this tommorow?

Yep will do

@marcelklehr
Copy link
Member

But as I understand this all is a breaking change in API and we need to publish all ExApp related to this in one day(or at least make commit to not break "Daily").

Right. Personally, I'm ok if AI on daily doesn't work for a bit, but if we can avoid it that'd be good. That would be app_api 4.0.0 then? Affected ex apps apps of mine is only llm2, I think

@marcelklehr
Copy link
Member

Tested and works with llm2

@bigcat88 bigcat88 enabled auto-merge (squash) August 12, 2024 09:23
@bigcat88 bigcat88 merged commit c054c4c into main Aug 12, 2024
29 checks passed
@bigcat88 bigcat88 deleted the feat/enum-task-processing branch August 12, 2024 09:32
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.

4 participants