-
Notifications
You must be signed in to change notification settings - Fork 0
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
[Internal review] Million-scale Face SDK generated with local swagger branch and related tests. #2
Conversation
@huxuan, this is for internal review. |
Gotcha. Will do it today. |
82c4dcf
to
a487748
Compare
client.LargePersonGroupPerson.AddPersonFaceFromStreamAsync(largePersonGroupId, createPersonResult.PersonId, stream).Wait(); | ||
} | ||
|
||
client.LargePersonGroup.TrainAsync(largePersonGroupId).Wait(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait for training here? Same for other similar places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, added in latest commit.
The overall looks good. Please make sure it is compatible with both of the old and new services. The only concern is about the wait for training which may be misleading to new comers/customers. |
@huxuan Yes. Confirmed this SDK works with our new service as well, in terms of functionalities regardless of faceList/personGroup. |
static BaseTests() | ||
{ | ||
// Retrieve the configuration information. | ||
FaceSubscriptionKey = ""; | ||
//Environment.SetEnvironmentVariable("AZURE_TEST_MODE", "Record"); | ||
//FaceSubscriptionKey = "793315696cd04d5ab78232a792136610"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should remove the actual value for FaceSubscriptionKey
though it is a fake one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is just for code sharing. Raised formal PR to upstream repo.
6b2046b
to
e57928a
Compare
e57928a
to
0e3f092
Compare
… SDKs and test records.
update the version and changelog.md
Updated by local MillionScale Swagger branch.
After getting this reviewed, will first get the Swagger PR merged to azure-rest-api-specs and then generate SDK from its master branch.