Fix: Follow override image gen service URL#352
Conversation
julien-nc
force-pushed
the
fix/noid/image-retrieval-auth
branch
from
e469e15 to
6579eaa
Compare
lukasdotcom
left a comment
lukasdotcom
left a comment
There was a problem hiding this comment.
Looks good other than one small thing. Nice catch on getIsImageRetrievalAuthenticated.
| public function getImageRequestOptions(?string $userId): array { | ||
| $timeout = $this->openAiSettingsService->getRequestTimeout(); | ||
| $requestOptions = [ | ||
| 'timeout' => $timeout, |
There was a problem hiding this comment.
Timeout should always have some value. When ImageRetrieval isn't authenticated timeout will not be defined.
There was a problem hiding this comment.
Check the last commit.
…are using OpenAI (fixes the default toggle value) and to get the credentials when retrieving the images Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
…om a URL Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
julien-nc
force-pushed
the
fix/noid/image-retrieval-auth
branch
from
6579eaa to
d4ed807
Compare
julien-nc
force-pushed
the
fix/noid/image-retrieval-auth
branch
3 times, most recently
from
f682d33 to
d28bace
Compare
…ing them which can break scoping if there actually is an update (the pkg is reinstalled and not scoped anymore) Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
julien-nc
force-pushed
the
fix/noid/image-retrieval-auth
branch
from
d28bace to
5501ca1
Compare
|
@marcelklehr It took me a while to figure out the psalm issue in this PR. With no change related with composer pkgs, psalm started to fail. It did not find the getid3 classes anymore. They were not scoped in the CI but everything was fine locally. This was actually because the steps we run in the Wdyt about the fix? (last commit) It prevents upgrading pkgs as a side effect of running |
| "grep -r 'OCA\\\\OpenAi\\\\Vendor' ./vendor || vendor/bin/php-scoper add-prefix --prefix='OCA\\OpenAi\\Vendor' --output-dir=\".\" --working-dir=\"./vendor/\" -f --config=\"../scoper.inc.php\"", | ||
| "composer dump-autoload", | ||
| "composer update --no-scripts" | ||
| "composer install --no-scripts" |
There was a problem hiding this comment.
👍 We should also do this in integration_replicate, I think
3 participants
Remaining issue for later: The user-defined credentials will be ignored if an admin overrides the image gen service URL.