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.

Sign up for GitHub

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

Jump to bottom

Changes related to the next MeiliSearch release (v0.25.0) #273

Merged
merged 14 commits into from Jan 12, 2022
Merged

Changes related to the next MeiliSearch release (v0.25.0) #273

merged 14 commits into from Jan 12, 2022

Conversation

meili-bot
Copy link
Contributor

Related to this issue: meilisearch/integration-guides#157

This PR:

  • gathers the changes related to the next MeiliSearch release (v0.25.0) so that this package is ready when the official release is out.
  • should pass the tests against the latest pre-release of MeiliSearch.
  • might eventually contain test failures until the MeiliSearch v0.25.0 is out.

⚠️ This PR should NOT be merged until the next release of MeiliSearch (v0.25.0) is out.

This PR is auto-generated for the pre-release week purpose.

@curquiza curquiza added the breaking-change The related changes are breaking for the users label Dec 13, 2021
@curquiza
Changes related to the new task API (#276)
8893955 
* First verison of the new task API

* Create task.rb

* Fix linter error

* Add tests for client.wait_for_task

* Fix linter errors
@curquiza
Update tests according to the typo fix on MeiliSearch side
f52addc
@curquiza
Remove get_or_create_index method (#277)
e6acfa1
@curquiza
Remove delete_index_if_exists method (#278)
d0f7fc9
@curquiza
Add API keys methods (#279)
1519c96 
* Add API keys methods

* Fix linter errors
@curquiza
Copy link
Member

curquiza commented Jan 4, 2022

bors try

meili-bors bot added a commit that referenced this pull request Jan 4, 2022
@meili-bors
Try #273:
925c8f4
@meili-bors
Copy link
Contributor

meili-bors bot commented Jan 4, 2022

try

Build failed:

brunoocasali
brunoocasali reviewed Jan 6, 2022
View reviewed changes
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

Hey, @curquiza thanks for your awesome work (as always) on this! 💟

I left some comments, ideas, suggestions, feel free to reach me out with any doubt :)
Oh, I didn’t comment on every item I found, but most of the comments regarding specs could be applied on other spec cases :)

lib/meilisearch.rb Show resolved Hide resolved
lib/meilisearch/http_request.rb Show resolved Hide resolved
lib/meilisearch/index.rb Outdated Show resolved Hide resolved
lib/meilisearch/task.rb Show resolved Hide resolved
spec/meilisearch/client/indexes_spec.rb Show resolved Hide resolved
lib/meilisearch/task.rb Outdated Show resolved Hide resolved
lib/meilisearch/client.rb Outdated Show resolved Hide resolved
spec/meilisearch/client/keys_spec.rb Outdated Show resolved Hide resolved
spec/meilisearch/client/keys_spec.rb Outdated Show resolved Hide resolved
spec/meilisearch/client/keys_spec.rb
expect(response).to have_key('hits')
end
new_client = MeiliSearch::Client.new(URL, public_key['key'])
response = new_client.index(uid).search('test')
Copy link
Member

Choose a reason for hiding this comment

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

If we just make the "search" without adding documents we could make an assertion by just ensuring there were no meilisearch_api_errors. That way we could simplify a bit this spec 🦘

Copy link
Member

Choose a reason for hiding this comment

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

We need to create an index to perform the search, so I can replace add_documents! with create_index!, but it does not simplify the spec :(

Copy link
Member

Choose a reason for hiding this comment

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

I was thinking about something like this:

let(:public_client) { MeiliSearch::Client.new(URL, public_key['key']) }

it 'succeeds to search when using public key' do
  uid = random_uid

  expect {
    public_client.index(uid).search('test')
  }.not_to raise_meilisearch_api_error_with(...)
end
  • public_client could be reused
  • we only check if no errors are produced since we don’t care about the search result

(btw, I did this here on the comment, probably there are some typo)

Feel free to disagree as well haha 🍔

Copy link
Member

Choose a reason for hiding this comment

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

If the index does not exist, public_client.index(uid).search('test') will return a 404 since the index does not exist. You cannot search in a non-existing index

This was referenced Jan 10, 2022
Closed
Merged
@curquiza
Add code samples for v0.25.0 (#286)
4bdf2d1 
* Add code samples for v0.25.0

* Update .code-samples.meilisearch.yaml
alallema
alallema reviewed Jan 11, 2022
View reviewed changes
Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

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

I just came to copy over your README changes and saw a typo.

.code-samples.meilisearch.yaml Outdated Show resolved Hide resolved
.code-samples.meilisearch.yaml Outdated Show resolved Hide resolved
alallema
alallema reviewed Jan 11, 2022
View reviewed changes
README.md Outdated Show resolved Hide resolved
curquiza and others added 4 commits January 12, 2022 11:47
@curquiza @alallema
Update README.md
5c42479 
Co-authored-by: Amélie <alallema@users.noreply.github.com>
@curquiza @alallema
Update .code-samples.meilisearch.yaml
b6c8d4f 
Co-authored-by: Amélie <alallema@users.noreply.github.com>
@curquiza @alallema
Update .code-samples.meilisearch.yaml
d0f45ff 
Co-authored-by: Amélie <alallema@users.noreply.github.com>
@curquiza
Apply changes after the review (#285)
982dc58 
* Apply changes after the review

* Fix linter errors
@curquiza curquiza marked this pull request as ready for review January 12, 2022 12:41
@curquiza
Copy link
Member

bors try

meili-bors bot added a commit that referenced this pull request Jan 12, 2022
@meili-bors
Try #273:
ca690e2
@meili-bors
Copy link
Contributor

meili-bors bot commented Jan 12, 2022

try

Build succeeded:

alallema
alallema approved these changes Jan 12, 2022
View reviewed changes
Copy link
Contributor

@alallema alallema left a comment

Choose a reason for hiding this comment

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

LGTM! 💪

brunoocasali
brunoocasali approved these changes Jan 12, 2022
View reviewed changes
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

Awesome work @curquiza 🎉 🌮

@brunoocasali
Copy link
Member

bors merge

@meili-bors
Copy link
Contributor

meili-bors bot commented Jan 12, 2022

Build succeeded:

@meili-bors meili-bors bot merged commit 9290118 into main Jan 12, 2022
@meili-bors meili-bors bot deleted the bump-meilisearch-v0.25.0 branch January 12, 2022 13:43
@curquiza curquiza mentioned this pull request Jan 12, 2022
Merged
meili-bors bot added a commit that referenced this pull request Jan 12, 2022
@meili-bors @curquiza
Merge #289
5f282ef 
289: Update version for the next release (v0.18.0) r=brunoocasali a=curquiza

Why is it breaking?
- #276
- #277
- #278
- #273

Co-authored-by: Clémentine Urquizar - curqui <clementine@meilisearch.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@curquiza curquiza curquiza left review comments

@brunoocasali brunoocasali brunoocasali approved these changes

@alallema alallema alallema approved these changes

Assignees
No one assigned
Labels
breaking-change The related changes are breaking for the users
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@meili-bot @curquiza @brunoocasali @alallema