Skip to content

Conversation

@FlamesRunner
Copy link
Contributor

This pull request adds the following methods:

  • add_documents_in_batches(documents, batch_size = 1000, primary_key = nil)
  • update_documents_in_batches(documents, batch_size = 1000, primary_key = nil)

The tests for these methods are:

  • adds documents in a batch (as a array of documents)
  • adds document batches synchronously (as an array of documents)
  • updates documents in index in batches
  • updates documents synchronously in index in batches (as an array of documents)

They are based on their non-batch methods. There are two linter errors:

lib/meilisearch/index.rb:7:3: C: Metrics/ClassLength: Class has too many lines. [252/226]
  class Index < HTTPRequest ...
  ^^^^^^^^^^^^^^^^^^^^^^^^^
spec/meilisearch/index/documents_spec.rb:3:1: C: Metrics/BlockLength: Block has too many lines. [500/497]
RSpec.describe 'MeiliSearch::Index - Documents' do ...

The linter indicates that there are too many lines for documents_spec.rb (which I was thinking of correcting by adding a new documents_batch_spec.rb, or the less clean way of changing the linter, etc), and that index.rb has too many lines (this one I am less sure of, as there aren't really any other classes that separate methods and since this is my first PR here, I don't want to introduce anything that might break something). I'm unsure, so I'd like to get some feedback on these.

References issue #218.

@curquiza curquiza self-requested a review October 5, 2021 18:20
@FlamesRunner
Copy link
Contributor Author

Thanks for running integration tests, I'm looking through them now ^

@FlamesRunner
Copy link
Contributor Author

It appears that when I ran the linter originally, I mistakenly removed the "return" statements, which is why the test failed. Sorry about that!

@curquiza
Copy link
Member

curquiza commented Oct 6, 2021

Hello @FlamesRunner, thanks for your PR, I will review it as soon as possible. It's not a small one so I need time to read it carefully :)

@CaroFG
Copy link
Contributor

CaroFG commented Oct 12, 2021

Hi @FlamesRunner! For the linter errors, you can run bundle exec rubocop --auto-gen-config, as explained in the contributing guides. Thank you! 🙏

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Hello again @FlamesRunner

The PR seems good to me 🎉
I cannot merge with your linter issue, you can fix with updating the rubocop_todo.yml. See the contributing guidelines for more information: https://github.com/meilisearch/meilisearch-ruby/blob/main/CONTRIBUTING.md#linter-

@FlamesRunner
Copy link
Contributor Author

@curquiza Thanks to CaroFG -- I missed that line entirely! It should be fixed now, let me know if any further changes are needed :)

@FlamesRunner FlamesRunner requested a review from curquiza October 12, 2021 20:27
@curquiza
Copy link
Member

bors try

bors bot added a commit that referenced this pull request Oct 13, 2021
@bors
Copy link
Contributor

bors bot commented Oct 13, 2021

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

Thanks @FlamesRunner

bors merge

If you are participating in Hacktoberfest, and you would like to receive a small gift from MeiliSearch too, please complete this form.

@bors
Copy link
Contributor

bors bot commented Oct 13, 2021

@bors bors bot merged commit 68d4724 into meilisearch:main Oct 13, 2021
@curquiza curquiza changed the title Added methods add_documents_in_batches, update_documents_in_batches and their tests Added methods add_documents_in_batches, update_documents_in_batches and their tests Oct 13, 2021
@curquiza curquiza added the enhancement New feature or request label Nov 10, 2021
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.

Create add_documents_in_batches and update_documents_in_batches methodes

3 participants