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.

Sign up for GitHub

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

Jump to bottom

enh(TextProcessing): Add two new provider interfaces #41271

Merged
merged 16 commits into from
Nov 13, 2023
Merged

enh(TextProcessing): Add two new provider interfaces #41271

merged 16 commits into from
Nov 13, 2023

Conversation

marcelklehr
Copy link
Member

@marcelklehr marcelklehr commented Nov 3, 2023 •
edited
Loading

Summary

Since we added the TextProcessing API in 27.1 some shortcomings have presented themselves, which I seek to improve with this PR.

Additions in this PR:

  • allow providers to obtain current task's userId
  • allow providers to expose average task runtime
  • IManager now also exposes a runOrSchedule() method which takes into account this runtime estimation
  • the schedule OCS API endpoint now uses runOrSchedule to avoid always scheduling a background job which has a max-delay of 5min even if the provider would be done in a few seconds, like the OpenAI provider for example.

Todo

  • Check and possible extend tests

Checklist

@marcelklehr
enh(TextProcessing): Add IProvider2
181f819 
- allow providers to obtain current task's userId
- allow providers to expose average task runtime

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
come-nc
come-nc reviewed Nov 6, 2023
View reviewed changes
core/Controller/TextProcessingApiController.php Show resolved Hide resolved
core/Controller/TextProcessingApiController.php Outdated Show resolved Hide resolved
lib/private/TextProcessing/Manager.php Outdated Show resolved Hide resolved
lib/public/TextProcessing/IProvider2.php Outdated Show resolved Hide resolved
@marcelklehr
fix(TextProcessing/Manager): Throw TaskFailureException upon failure
d11b9cb 
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
fix(TextProcessing/Manager): add canuseProvider check in getPreferred…
26a659d 
…Providers

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
fix(TextProcessing/ migration): add check whether new column exists a…
1476a72 
…lready

Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
enh: update openapi.json
b01e0c3 
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
fix: psalm issues and coding style
fecf642 
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
fix: split IProvider2 into two more verbose interfaces
17e7b6c 
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr marcelklehr changed the title enh(TextProcessing): Add IProvider2 enh(TextProcessing): Add two new provider provider interfaces Nov 7, 2023
@marcelklehr
fix coding style
b8862bd 
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr
Merge branch 'master' into enh/text-processing-iprovider2
8c0c426 
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr marcelklehr changed the title enh(TextProcessing): Add two new provider provider interfaces enh(TextProcessing): Add two new provider provider provider interfaces Nov 8, 2023
@marcelklehr marcelklehr changed the title enh(TextProcessing): Add two new provider provider provider interfaces enh(TextProcessing): Add two new provider interfaces Nov 8, 2023
julien-nc
julien-nc requested changes Nov 8, 2023
View reviewed changes
Copy link
Member

@julien-nc julien-nc left a comment

Choose a reason for hiding this comment

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

LGTM. Small remarks.

lib/private/TextProcessing/Manager.php Outdated Show resolved Hide resolved
lib/private/TextProcessing/Manager.php Outdated Show resolved Hide resolved
lib/public/TextProcessing/IProviderWithUserId.php Outdated Show resolved Hide resolved
marcelklehr and others added 3 commits November 8, 2023 17:00
@marcelklehr @julien-nc
Update lib/private/TextProcessing/Manager.php
12c558f 
Co-authored-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr @julien-nc
Update lib/private/TextProcessing/Manager.php
6fbf430 
Co-authored-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr @julien-nc
Update lib/public/TextProcessing/IProviderWithUserId.php
a29ed70 
Co-authored-by: Julien Veyssier <julien-nc@posteo.net>
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@marcelklehr marcelklehr requested a review from julien-nc November 8, 2023 16:16
@marcelklehr marcelklehr mentioned this pull request Nov 9, 2023
Merged
@marcelklehr marcelklehr added this to the Nextcloud 28 milestone Nov 9, 2023
@blizzz blizzz mentioned this pull request Nov 9, 2023
Merged
@marcelklehr
fix: Make sure array starts at 0 after array filter
2031fe1 
Signed-off-by: Marcel Klehr <mklehr@gmx.net>
@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Nov 10, 2023
@blizzz blizzz mentioned this pull request Nov 10, 2023
Merged
@marcelklehr
Copy link
Member Author

Docs are here: nextcloud/documentation#11273

@marcelklehr marcelklehr mentioned this pull request Nov 10, 2023
Open
12 tasks
@nickvergessen nickvergessen merged commit 0feb55e into master Nov 13, 2023
47 of 49 checks passed
@nickvergessen nickvergessen deleted the enh/text-processing-iprovider2 branch November 13, 2023 09:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@provokateurin provokateurin provokateurin left review comments

@come-nc come-nc come-nc approved these changes

@julien-nc julien-nc julien-nc approved these changes

Assignees
No one assigned
Labels
pending documentation This pull request needs an associated documentation update
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.
6 participants
@marcelklehr @julien-nc @provokateurin @come-nc @nickvergessen @ChristophWurst