Add dumps methods

[eskombro]

As stated in meilisearch/integration-guides#44, SDKs should implement the dump feature, by adding 2 new routes, testing and adding code samples

[curquiza]

Awesome

[curquiza]

From my POV, you are testing MeiliSearch here. The SDK should not test the dump is created, in particular in test_dump_status_route that only returns the status of a dump.
Cf the end of the conversation 🐌

[eskombro]

Indeed!

But at the same time it checks that with a dump status change on MS, client recieves the new information. Doesn't hurt to have the check for free :)

If you think this is absolutely out of scope, I'm not against removing it tho!

[curquiza]

This check is not "for free". It adds a small complexity in test readability. A small one, but if you add more and more like this one, it's not small at all 🙂
Now MeiliSearch is greatly doing its own tests, we should try now to stay in our scope to keep our SDKs as clean as possible. A lot of our old tests should not be in our SDKs anymore because they are also testing MeiliSearch, but before we find time to clean them, I suggest we don't add any other out-of-scope tests.
Also, @bidoubiwa, what do you think? Maybe both of you think this is in the scope.
Cf the end of the conversation 🐌

[eskombro]

I understand your POV and mostly agree with it. But also, I would still prefer to wait until the dump is created (while loop) because if we add a new test below (as I just did in a new commit) we can introduce a conflict (because you are not allowed to create another dump if one is being created). So it would be better to wait until the dump is done to quit the test. In this case, I would remove just the last line of this block, which is why I said it is 'free' to check the 'done' status, WDYT?

[eskombro]

I did abstract this waiting loop on a wait_for_dump_creation method just used in tests, to facilitate reusability and readability.

It now looks like:

    ...
    assert dump_status['status'] == 'processing'
    wait_for_dump_creation(dump_status['uid'])
    assert dump_status['status'] == 'done'

If you want me to remove the last line, react with 👎 and I'll do ;)

[eskombro]

It is needed because if we exit the test right away, and a new test tries to create a new dump, MeiliSearch will throw an error, since you can't create two dumps at the same time

[eskombro]

In the other hand, I opted for removing the last check for the 'done' status as you suggested

[curquiza]

It is needed because if we exit the test right away, and a new test tries to create a new dump, MeiliSearch will throw an error, since you can't create two dumps at the same time

I was slow to understand! sorryyyyy! 🐌 I've just understood that's only for the next tests! 😅

[curquiza]

But if it's for the next tests, why the test test_dump_creation does not have any wait_for_pending_dump and does not seem to need it?

[eskombro]

Because at the time the create_dump test is executed, there are no documents, MeiliSearch index is completely empty, and therefore the dump creation is quite fast, almost instant. But you are right, in theory, it should also wait for its creation. On the other hand, before the get_dump_status test, MeiliSearch creates an index and populate it before creating a dump, and so it takes some time.

[eskombro]

Removed the redundant asserts ;)

I also added a test for it to fail with a MeiliSearchApiError in case the uid doesn't exist @curquiza

[curquiza]

Could you add a teardown_class to delete all the indexes, or at least the index your create?
The setup_class is here to make the test context clean
The teardown_class is to clean after the test is done to let a clean MeiliSearch after running the tests. It's for maintainers convenience only: if you run only this test or if it's the last test to be processed.

[bidoubiwa]

thanks a lot for the PR! looks really good to me :)

[curquiza]

LPTM (Looks PERFECT to me)