Skip to content

Conversation

@rakshith91
Copy link
Contributor

@rakshith91 rakshith91 commented Apr 30, 2020

Fixes #10836

@rakshith91 rakshith91 marked this pull request as ready for review April 30, 2020 22:08
@rakshith91 rakshith91 requested review from bryevdv and johanste May 1, 2020 17:41
return result.indexers

@distributed_trace
def delete_indexer(self, indexer, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

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

Why we ask user to give entire indexer? Name is good enough. Not correct?

Copy link
Member

Choose a reason for hiding this comment

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

And you have ':param name: The name of the indexer to delete.' :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indexers is Union[str, Indexer] - name is enough to delete unconditionally - we need the model to use match conditions - have documented it in the docstrings

Comment on lines 150 to 151
:param name: The name of the indexer to delete.
:type name: str
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
:param name: The name of the indexer to delete.
:type name: str
:param indexer: The indexer to delete.
:type name: str or ~search.models.Indexer

@distributed_trace
def delete_indexer(self, indexer, **kwargs):
# type: (Union[str, Indexer], **Any) -> None
"""Deletes an indexer. To use only_if_unchanged, the Indexer model
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
"""Deletes an indexer. To use only_if_unchanged, the Indexer model
"""Deletes an indexer. To use access conditions, the Indexer model

Copy link
Member

Choose a reason for hiding this comment

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

Have we had an API review?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not yet for Indexers operations, tho if you are asking about only_if_unchanged vs access conditions, @johanste weighed in on that in #11116

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed the documentation to match conditions

xiangyan99
xiangyan99 previously approved these changes May 1, 2020
@rakshith91 rakshith91 merged commit 0a83a8c into Azure:master May 1, 2020
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.

Implement Indexers Operations for Search

3 participants