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

Update to algoliasearch 2.0 #28

Merged
merged 2 commits into from Apr 26, 2019
Merged

Update to algoliasearch 2.0 #28

merged 2 commits into from Apr 26, 2019

Conversation

Natim
Copy link
Member

@Natim Natim commented Apr 26, 2019

Fixes #18
r? @nunomaduro

@@ -23,7 +23,7 @@ def join(self):
self.tasks = []

def set_extra_headers(self, headers):
self.client.set_extra_headers(**headers)
self.client._config.headers.update(headers)
Copy link
Member Author

Choose a reason for hiding this comment

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

That's the simplest way I found to not recreate the client each time.

Another solution would be to keep the application_id and api_key and recreate the client there.

Choose a reason for hiding this comment

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

Sounds fine.

@Natim
Copy link
Member Author

Natim commented Apr 26, 2019

@Natim Natim force-pushed the upgrade-to-algoliasearch-2.0 branch from f8fc0bb to 013331e Compare April 26, 2019 13:48
index.delete().wait()

def isalive(self):
self.client._transporter.read(Verb.GET, "1/isalive", {}, None)
Copy link
Member Author

Choose a reason for hiding this comment

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

That's eventually how I did it, but I guess it would have been better to keep the isalive() method.

Choose a reason for hiding this comment

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

@Natim Sounds good at the moment, as this moment is not in our plans for the v2.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why did you choose to remove it?

index.delete().wait()

def isalive(self):
self.client._transporter.read(Verb.GET, "1/isalive", {}, None)

Choose a reason for hiding this comment

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

@Natim Sounds good at the moment, as this moment is not in our plans for the v2.

@@ -37,7 +37,7 @@ def test_present_in_heartbeat(self):

def test_returns_false_if_connection_fails(self):
with mock.patch.object(self.app.app.registry.indexer, "client") as client:
client.is_alive.side_effect = AlgoliaException
client._transporter.read.side_effect = AlgoliaException

Choose a reason for hiding this comment

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

This side_effect property doesn't exist. Is this test passing?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is because client has been mocked on line 39 here.

@@ -23,7 +23,7 @@ def join(self):
self.tasks = []

def set_extra_headers(self, headers):
self.client.set_extra_headers(**headers)
self.client._config.headers.update(headers)

Choose a reason for hiding this comment

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

Sounds fine.

@Natim Natim merged commit d912e24 into master Apr 26, 2019
@Natim Natim deleted the upgrade-to-algoliasearch-2.0 branch April 26, 2019 14:28
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.

Upgrade to algoliasearch 2.0
2 participants