[AutoPR azure-ai-face] Migrate existing Azure AI Face service to TypeSpec #5858
Conversation
azure-sdk
force-pushed
the
sdkAuto/27576/azure-ai-face
branch
3 times, most recently
from
6ab969b
to
8ea1efd
Compare
azure-sdk
force-pushed
the
sdkAuto/27576/azure-ai-face
branch
6 times, most recently
from
b02da2a
to
fd4f2ea
Compare
azure-sdk
changed the title
azure-sdk
force-pushed
the
sdkAuto/27576/azure-ai-face
branch
from
fd4f2ea
to
1d7b973
Compare
azure-sdk
force-pushed
the
sdkAuto/27576/azure-ai-face
branch
2 times, most recently
from
65642b5
to
626cf0d
Compare
Merge f25d933a13e5f2b428f8179be8e72468f97bbac4 into 7d111b106a71ca148eda71d1f70b7929c1ef164a
azure-sdk
force-pushed
the
sdkAuto/27576/azure-ai-face
branch
from
626cf0d
to
fb66c91
Compare
| async def add_face_from_stream( | ||
| self, | ||
| face_list_id: str, | ||
| image_content: bytes, |
There was a problem hiding this comment.
If this is a stream - we should support IO[bytes]
| self._deserialize = input_args.pop(0) if input_args else kwargs.pop("deserializer") | ||
|
|
||
| @overload | ||
| async def create_list( # pylint: disable=inconsistent-return-statements |
There was a problem hiding this comment.
These method names shouldn't duplicate the noun - i.e. we should use something like:
client.face_lists.get/create/update/delete
Instead of:
client.face_lists.get_list/create_list/update_list/delete_list
| self.person_directory_persons = PersonDirectoryPersonsOperations( | ||
| self._client, self._config, self._serialize, self._deserialize | ||
| ) | ||
| self.person_directory_dynamic_person_groups = PersonDirectoryDynamicPersonGroupsOperations( |
There was a problem hiding this comment.
I think we will need to work on these operation group names.
| self._serialize.client_side_validation = False | ||
| self.face_lists = FaceListsOperations(self._client, self._config, self._serialize, self._deserialize) | ||
| self.large_face_lists = LargeFaceListsOperations(self._client, self._config, self._serialize, self._deserialize) | ||
| self.face_async_operations = FaceAsyncOperationsOperations( |
There was a problem hiding this comment.
We should avoid the term "async" in the operation group name.
| return cls(pipeline_response, None, {}) # type: ignore | ||
|
|
||
| @distributed_trace_async | ||
| async def list_face( |
There was a problem hiding this comment.
When we need to add a noun to a listing operation - it should be plural: list_faces
| async def list_face( | ||
| self, person_id: str, recognition_model: Union[str, _models.RecognitionModel], **kwargs: Any | ||
| ) -> _models.ListFaceResult: | ||
| """Operation that lists resources in a paginated way. |
There was a problem hiding this comment.
These operations seem to be missing docstrings
| :raises ~azure.core.exceptions.HttpResponseError: | ||
|
|
||
| Example: | ||
| .. code-block:: python |
There was a problem hiding this comment.
The identify methods don't need the suffix for clarification, as that's implied by their grouping. I would probably just call them all idenfity_faces or something similar.
| return deserialized # type: ignore | ||
|
|
||
|
|
||
| class PersonDirectoryDynamicPersonGroupsOperations: # pylint: disable=name-too-long |
There was a problem hiding this comment.
Should be just "DynamicPersonGroupOperations"
| :raises ~azure.core.exceptions.HttpResponseError: | ||
|
|
||
| Example: | ||
| .. code-block:: python |
There was a problem hiding this comment.
This takes the same payload and returns the same response as the create_session method above.
Are we able to merge these two "session" operation groups?
| :raises ~azure.core.exceptions.HttpResponseError: | ||
|
|
||
| Example: | ||
| .. code-block:: python |
| async def add_face_from_url( | ||
| self, | ||
| face_list_id: str, | ||
| body: _models.AddFaceFromUrlContent, |
azure-sdk
force-pushed
the
main
branch
4 times, most recently
from
4b0a56d
to
5652869
Compare
azure-sdk
force-pushed
the
main
branch
3 times, most recently
from
6680ffc
to
5652869
Compare
Create to sync Azure/azure-rest-api-specs#27576
You can install the use using pip install of the artifacts.