Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implemented createSnapshot method and created entry with create_snaps… #885

Merged
merged 6 commits into from
Dec 19, 2023

Conversation

pbelokon
Copy link
Contributor

Pull Request

Related issue

Fixes #884

What does this PR do?

This pr Implements the new createSnapshot method in the meilisearch\client.py, also creates a new entry with the key "create_snapshot_1:" in the .code-samples.meilisearch.yml

PR checklist

Please check if your PR fulfills the following requirements:

  • Does this PR fix an existing issue, or have you listed the changes applied in the PR description (and why they are needed)?
  • Have you read the contributing guidelines?
  • Have you made sure that the title is accurate and descriptive of the changes?

@pbelokon
Copy link
Contributor Author

Not quite sure where I can update the code-samples, can some one guide me there ?

Copy link

codecov bot commented Nov 22, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fdd848a) 100.00% compared to head (5dd994d) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #885   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           13        13           
  Lines          748       752    +4     
=========================================
+ Hits           748       752    +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sanders41 sanders41 added the enhancement New feature or request label Nov 22, 2023
Copy link
Collaborator

@sanders41 sanders41 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the documentation the .code-samples.meilisearch.yaml file does that so you have already done it.

We also need to add a test for the snapshot creation, you can model it off the dump creation tests. Just note that your test will probably fail until #887 is merged.

.code-samples.meilisearch.yaml Outdated Show resolved Hide resolved
meilisearch/client.py Outdated Show resolved Hide resolved
meilisearch/client.py Outdated Show resolved Hide resolved
meilisearch/client.py Outdated Show resolved Hide resolved
meilisearch/client.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@sanders41 sanders41 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expected the integration tests to fail while we wait on the other PR to merge. The test looks good so I expect it will pass after the other PR is brought in here.

For the failing black and pylint tests running black meilisearch tests should fix both of these.

@sanders41
Copy link
Collaborator

I'll wait to merge this until we have all the tests passing, but I'll be surprised if we need any more changes here. Everything looks good to me, we just need the other bug fix to merge.

@pbelokon
Copy link
Contributor Author

pbelokon commented Nov 24, 2023

I'll wait to merge this until we have all the tests passing, but I'll be surprised if we need any more changes here. Everything looks good to me, we just need the other bug fix to merge.

Thank you of such a great collaboration! I will wait for the updates

@curquiza
Copy link
Member

Hello everyone
The other PR (#887) has been merged
You can fix the git conflict and merge the PR when you are ready 😄

Thank you for your work here ❤️

Copy link
Collaborator

@sanders41 sanders41 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pbelokon, after getting that bug fix merged everything is passing.

@sanders41
Copy link
Collaborator

bors merge

Copy link
Contributor

meili-bors bot commented Dec 19, 2023

@pbelokon pbelokon closed this Dec 19, 2023
@meili-bors meili-bors bot merged commit 083d6b9 into meilisearch:main Dec 19, 2023
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[v1.5] Add a new method for creating snapshots
3 participants