Skip to content

Conversation

LeoErcolanelli
Copy link

No description provided.

@LeoErcolanelli
Copy link
Author

Missing tests

Copy link

@clement-leprovost clement-leprovost left a comment

Choose a reason for hiding this comment

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

@ercolanelli-leo There are a few glitches to solve before merging this PR, most notably a wrong HTTP method for batch_rules. :)

before saving this batch? Default is False.
"""
params = {'forwardToReplicas': forward_to_replicas, 'clearExistingRules': clear_existing_rules}
return self._req(False, '/rules/batch', 'PUT', params, data)

Choose a reason for hiding this comment

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

The batch endpoint uses POST, not PUT.

params =- {'forwardToReplicas': forward_to_replicas}
return self._req(False, '/rules/%s' % str(objectID), 'DELETE', params)

def clear_rule(self, forward_to_replicas=False):

Choose a reason for hiding this comment

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

I would prefer clear_rules (plural) rather than clear_rule (singular), as this endpoint clears all rules.

params = {'forwardToReplicas': forward_to_replicas}
return self._req(False, '/rules/clear', 'POST', params)

def search_rules(self, query=None, anchoring=None, context=None, page=0, hitsPerPage=20):

Choose a reason for hiding this comment

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

It would be better not to hard-code the default values for page and hitsPerPage in the API client. If you omit these values, the API will use the default; so I had rather put None as default.


def search_rules(self, query=None, anchoring=None, context=None, page=0, hitsPerPage=20):
"""
Search for a rule inside the index.

Choose a reason for hiding this comment

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

Search for rules, not just a single one. :)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 89.972% when pulling 9984180 on query-rules into 6c1455c on master.

@LeoErcolanelli LeoErcolanelli merged commit 7b5f19c into master Jul 10, 2017
@LeoErcolanelli LeoErcolanelli deleted the query-rules branch July 10, 2017 14:53
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