Closed komalg1 closed 1 month ago
Coverage Report
Tests | Skipped | Failures | Errors | Time |
---|---|---|---|---|
130 | 0 :zzz: | 0 :x: | 0 :fire: | 8.169s :stopwatch: |
The search client logic extraction is looking great - one thing I missed when we discussed this earlier, sorry - is it possible to create the clients from a central place so that the Explore and Delete pages don't have to worry about if env_helper.AZURE_SEARCH_USE_INTEGRATED_VECTORIZATION
? But not a big deal if this can't be done, just a thought.
Second - is it possible to add unit tests for the extracted search methods in the two clients, now that they're not connected with StreamLit directly anymore?
The search client logic extraction is looking great - one thing I missed when we discussed this earlier, sorry - is it possible to create the clients from a central place so that the Explore and Delete pages don't have to worry about
if env_helper.AZURE_SEARCH_USE_INTEGRATED_VECTORIZATION
? But not a big deal if this can't be done, just a thought.Second - is it possible to add unit tests for the extracted search methods in the two clients, now that they're not connected with StreamLit directly anymore?
Thanks for the review. Will take it up as a refactoring task & see if it is possible to have clients at a central place. Added unit tests
Apart from some missed test cases, looks good to me. Once the tests are added I'm happy to approve. Thanks Komal!
Purpose
Does this introduce a breaking change?
Pull Request Type
What kind of change does this Pull Request introduce?
How to Test