Skip to content

Commit operations improvements#15

Open
eduardocampano wants to merge 10 commits into
algolia:algoliafrom
orchardmile:feature/improvements
Open

Commit operations improvements#15
eduardocampano wants to merge 10 commits into
algolia:algoliafrom
orchardmile:feature/improvements

Conversation

@eduardocampano

Copy link
Copy Markdown
  • Removed unnecessary call to self.run_auto_commit() in constructor
  • Replaced BATCH_SIZE and AUTO_COMMIT_DELAY_S to have the same constructor parameters as other doc managers.
  • Added optional parameter commit_sync to use synchronous commits to algolia with a False default value
  • Added optional parameter commit_waittask_interval with default value 1 (second) to reduce the number of waitTask requests (previously was 100ms, waitTask default value)

Comment thread tests/test_algolia.py Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We also need to put commit_sync=True, right?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Correct, I also removed the parameter from the commit calls from the tests

@Nagriar

Nagriar commented Aug 13, 2015

Copy link
Copy Markdown
Member

Hi @eduardocampano,

Sorry for the delay, I missed the pull request and it takes me some time to test it.

I tested the code and I spotted some bug in the connector itself and in our python client.

I just have one question, why you didn't keep the default parameter names in the constructor? With this change, it's not anymore possible to set it from the command line.

Thanks a lot for improvement of the connector I love it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants