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.

Sign up for GitHub

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

Jump to bottom

[CS Face] Add python sdk snapshot test. #4239

Merged
merged 2 commits into from
Mar 8, 2019
Merged

[CS Face] Add python sdk snapshot test. #4239

merged 2 commits into from
Mar 8, 2019

Conversation

acured
Copy link

@acured acured commented Jan 25, 2019

No description provided.

@acured acured requested a review from lmazuel as a code owner January 25, 2019 08:45
@codecov-io
Copy link

codecov-io commented Jan 25, 2019 •
edited
Loading

Codecov Report

Merging #4239 into restapi_auto_cognitiveservices/data-plane/Face will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@                                Coverage Diff                                 @@
##           restapi_auto_cognitiveservices/data-plane/Face    #4239      +/-   ##
==================================================================================
+ Coverage                                           53.76%    53.8%   +0.04%     
==================================================================================
  Files                                                9864     9864              
  Lines                                              210011   210011              
==================================================================================
+ Hits                                               112913   113003      +90     
+ Misses                                              97098    97008      -90
Impacted Files Coverage Δ
...ault/azure/keyvault/v7_0/models/secret_item_py3.py 100% <0%> (ø) ⬆️
...ource_sql_server_task_output_database_level_py3.py 41.66% <0%> (ø) ⬆️
...ure/mgmt/monitor/models/metric_availability_py3.py 100% <0%> (ø) ⬆️
...etwork/v2018_11_01/models/virtual_hub_route_py3.py 57.14% <0%> (ø) ⬆️
...zure/mgmt/network/v2018_08_01/models/subnet_py3.py 100% <0%> (ø) ⬆️
...t/recoveryservicesbackup/models/bek_details_py3.py 50% <0%> (ø) ⬆️
...zure-mgmt-redis/azure/mgmt/redis/models/sku_py3.py 100% <0%> (ø) ⬆️
...nitiveservices/vision/face/models/image_url_py3.py 71.42% <0%> (ø) ⬆️
...8_01/models/express_route_circuit_reference_py3.py 66.66% <0%> (ø) ⬆️
...network_configuration_diagnostic_parameters_py3.py 55.55% <0%> (ø) ⬆️
... and 3808 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b98e82...db9d1a9. Read the comment docs.

@adxsdk6
Copy link

adxsdk6 commented Jan 25, 2019

Can one of the admins verify this patch?

1 similar comment
@adxsdk6
Copy link

adxsdk6 commented Jan 25, 2019

Can one of the admins verify this patch?

@lebronJ
Copy link

lebronJ commented Jan 25, 2019

Hi @lmazuel , @acured is from our Face API team and working on the SDKs.

This is to align Face Python SDK with latest published Face API with Snapshot features (rollout started on Jan 21st and will last a week), which only adds new API interfaces for Face API, without changing any existing interfaces.

This PR is to add tests for the new Snapshot interfaces. Please help review it, thanks.

@lmazuel
Copy link
Member

lmazuel commented Feb 6, 2019

Hi @lebronJ , thanks for the tests! LGTM!

lmazuel
lmazuel reviewed Feb 6, 2019
View reviewed changes
azure-cognitiveservices-vision-face/tests/test_face.py Outdated
while operationStatus != "succeeded" and operationStatus != "failed":
getOperationStatusResponse = face_client.snapshot.get_operation_status(takeOperationId)
operationStatus = getOperationStatusResponse.additional_properties["Status"]
sleep(1)
Copy link
Member

Choose a reason for hiding this comment

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

please change it to:

if self.is_live:
    sleep(1)

since we want to sleep in real mode, but in record we can be fast. We will make our CI faster.

Copy link
Author

Choose a reason for hiding this comment

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

Done.

@lmazuel
Copy link
Member

lmazuel commented Feb 6, 2019

Hi @lebronJ @acured I actually have a small comment that should be super fast to care of ;)

@lmazuel lmazuel changed the title Add python sdk snapshot test. [CS Face] Add python sdk snapshot test. Feb 15, 2019
@lmazuel lmazuel merged commit 11c6f61 into Azure:restapi_auto_cognitiveservices/data-plane/Face Mar 8, 2019
lmazuel pushed a commit that referenced this pull request Mar 8, 2019
@AutorestCI @lmazuel
[AutoPR] cognitiveservices/data-plane/Face (#4220)
a2ccb46 
* Generated from 1b9b098a5d61cbc372acb108a29c29328a4120b2 (#4219)

Resolve comments.

* [AutoPR cognitiveservices/data-plane/Face] [Cogs Face] Refine the documentation for Face API's new Snapshot features.  (#4240)

* Generated from 2662c92f89bbd0d23bcd042cb8ee4401d9ec33ff

Add period after quota description.

* Generated from c88a6f0cf2096a86ec5e870bd7524d0b01b152ae

Change 。 to .

* [AutoPR cognitiveservices/data-plane/Face] [Cogs Face] Remove Face API reference of Gender::genderless to avoid confusion. (#4347)

* Generated from 6cf5bbc0433e906413c9312b93441c491c06c529

Remove API reference of Gender::genderless.

* Packaging update of azure-cognitiveservices-vision-face

* [CS Face] Add python sdk snapshot test. (#4239)

* Add python sdk snapshot test.

* change 'sleep' works in live.

* Packaging update of azure-cognitiveservices-vision-face

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

@lmazuel lmazuel lmazuel left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@acured @codecov-io @adxsdk6 @lebronJ @lmazuel