Changes due to the implicit index creation #175
Conversation
curquiza
added
the
breaking-change
There was a problem hiding this comment.
This is a great work @curquiza , thank you so much!
I did a quite intense code review, sorry if there is a lot of pain in going through my review :)
I do like a lot the robust testing!
I don't actually agree with using objects in a test that are coming from (or modified by) another test, I think this is not consistent, and a test should be independet and always behave in the same way if it is called alone or in a test set. Not saying this is your fault, but maybe we can fix that or at least not introduce more entangled tests :)
There is already #170 😉 |
|
About the removal of |
I am sorry, I didn't see #170. But I'm glad, it means we saw both that this is clearly an issue. Feel free to ignore my comments about independent tests as they all belong to that issue! |
great, let me know if you don't and I'll do a quick PR :) do you agree with those changes @bidoubiwa ? What could be the purpose of this asserts: |
|
I did the changes about |
curquiza
force-pushed
the
implicit-index-creation-v2
branch
from
c37ee08 to
ed86de1
Compare
|
bors merge |
|
Build succeeded: |
176: Restructure code r=curquiza a=curquiza⚠️ Should be merged after the merge of #175 Breaking change: - Make the internal method `Index.create(...)` a `@classmethod` instead of a `@staticmethod`. Why? Put the logic related to an index in the `Index` class instead of the `Client` class. See the difference between static and class methods in Python: https://stackabuse.com/pythons-classmethod-and-staticmethod-explained/. - Remove the internal method `Index.get_indexes()`. Since it's not related to only one index, this method should not be present in the `Index` class. - Make the `update()` method returns an `Index` object instead of a `dict` because it's more convenient to manipulate object instead of dict. This is consitent with `client.index('movies').fetch_info()` and `client.create_index('movies)` and `client.index('movies')` Changes: - Check the type of the responses more accurately: `assert isinstance(response, Index)` Co-authored-by: Clementine Urquizar <[email protected]>
Related to meilisearch/integration-guides#48
Here is the list of changes you have to check:
get_index()does an HTTP call and is not the main method to use anymore.index()methodindex()methodget_or_create_indexwith the new way of usingindex()primary_keyattribute inIndexclass. This attribute is updated when an HTTP call to the index is done (creation, update, or fetch the info). The attribute is obviously not up-to-date when using theindex()method: the method does not do any HTTP call. Refer to the limitation section in the main issue.info()method intofetch_info()to make the HTTP call explicit. Now, the right usage to get the index information isclient.get_index('movies')orclient.index('movies').fetch_info().get_indexfrom theIndexclass because not used anymore.index.pychanges might be not visible if you don't click onload diff: