- Issue created by @a.dmitriiev
- π©πͺGermany a.dmitriiev
a.dmitriiev β changed the visibility of the branch 3521601-pass-search-api to hidden.
- π¬π§United Kingdom MrDaleSmith
MR is in draft and has failing tests, so knocking back to needs work.
- πΊπΈUnited States SocialNicheGuru
So with this MR is ai 1.1.x needed? Wasn't this the cause of that other branch?
- π©πͺGermany a.dmitriiev
Test failure:
Fatal error: Declaration of Drupal\field_validation\Plugin\Validation\Constraint\UlidConstraint::validatedBy() must be compatible with Symfony\Component\Validator\Constraint::validatedBy(): string in /builds/issue/ai-3521601/web/modules/contrib/field_validation/src/Plugin/Validation/Constraint/UlidConstraint.php on line 21
This is not relevant to the MR and moreover it is from another module.
- π¬π§United Kingdom scott_euser
Added some comments to the MR. I still think (per slack) that this should never go to 1.1.x though as getClient() should just get client and not cause side effects. And since 1.1.x has a mechanism to get the server, this is really just temporary throw away code for 1.0.x
- π©πͺGermany a.dmitriiev
I have fixed the issues mentioned in the MR comments.
Regarding "throw away code" I disagree. You mention that there is now a way to get the context, but it is only available for method
vectorSearch
, how would it be retrieved in any other method of the class if needed? - π¬π§United Kingdom scott_euser
how would it be retrieved in any other method of the class if needed?
My point isn't that its not necessarily not needed, but that if it is needed elsewhere (is it??), adding a side effect to getClient() does not seem like the right way to do it, so more that this approach is temporary code to solve specific problem in 1.0.x. Have not looked deeply, but maybe some way in AiVdbProviderPluginManager::createInstance() to construct the provider e.g. in AiVdbProviderClientBase...
- First commit to issue fork.
- πͺπΈSpain gxleano CΓ‘ceres
Moving changes from
1.0.x
to1.1.x
in order to move forward this topic. - πͺπΈSpain gxleano CΓ‘ceres
gxleano β changed the visibility of the branch 3521601-server-context to hidden.