-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[App config] Perf tests #15763
[App config] Perf tests #15763
Conversation
… harshan/app-config/perf-tests
… harshan/app-config/perf-tests
… harshan/app-config/perf-tests
4. Run the tests as follows | ||
|
||
- list settings | ||
- `npm run perf-test:node -- ListSettingsTest --warmup 2 --duration 7 --iterations 2 --parallel 2` |
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.
is this service rate-limited?
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.
There is throttling similar to Form recognizer or metrics advisor.
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.
if the operation is not a LRO, can you experiment with the number for the parallel param if the limit is not too bad?
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.
The limit is 20,000 requests per hour, it is quite simple to hit the limit.
We'll soon get the proxy-tool so that we don't have to hit the live service all the time.
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.
Looks like you and @deyaaeldeen have some other review feedback to sort out.
I think the code changes look good but something looks wrong about your pnpm-lock changes.
sdk/appconfiguration/perf-tests/app-configuration/test/appConfigBase.spec.ts
Show resolved
Hide resolved
sdk/appconfiguration/perf-tests/app-configuration/test/index.spec.ts
Outdated
Show resolved
Hide resolved
sdk/appconfiguration/perf-tests/app-configuration/test/listSettings.spec.ts
Show resolved
Hide resolved
sdk/appconfiguration/perf-tests/app-configuration/test/listSettings.spec.ts
Show resolved
Hide resolved
3. Create a storage account and populate the `.env` file with `APPCONFIG_CONNECTION_STRING` variable. | ||
4. Run the tests as follows | ||
|
||
- list settings |
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.
What's special about these settings?
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.
list configuration settings? what do you mean?
Hello @HarshaNalluru! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
[Hub Generated] Public private branch 'raonon-MixedReality-UpdateResourceResponse' (Azure#15763) * Updated response payload to include missing properties * Reverted Identity property * Updated stable/2020_05_01/proxy.json to include missing object properties Co-authored-by: Raymond Ononiwu <raonon@microsoft.com>
Adds the list test
Closes #13981