***NO_CI***
- string replace in package json: 12 => 14 for engines/node and for dependency @types/node
- eslint: update `json-engine-is-present` rule to 14.0.0 as LTS version
- identity: react to typing improvements for `readFile()`'s `options.encoding`
- trivial generated api.md file changes due to @types/node version bump
# Fixing LRO cancellation by deprecating `cancelOperation`
Canceling a long-running operation is currently done by calling `cancelOperation`: eb0aa1da67/sdk/core/core-lro/src/poller.ts (L79-L82) However the cancellation behavior could vary between APIs in a way that makes `cancelOperation` a confusing abstraction. In particular, when sending a cancellation request, the service may respond with either a 200 or a 202 response, depending on the API and/or the operation, where the former indicates the operation has been canceled immediately and the latter indicates it could be some time before the operation is deemed as canceled. Handling 202 responses needs care to set the customer's expectation about wait times and this is typically done by providing a poller object that customer could use to optionally block until the operation finishes. This discrepancy in behavior makes `cancelOperation` not a suitable abstraction for the case when cancellation returns a 202 response.
In this PR, I am deprecating `cancelOperation` and leaving it up to the library author to implement operation cancellation as they see fit. This proposal has been discussed with @bterlson, @joheredi, and @witemple-msft and they're all on-board with it.
## Pollers without cancellation abstraction
The PR also introduces a second poller interface that doesn't have `cancelOperation` at all, named `SimplePollerLike`. This interface is meant to be used for pollers for new operations that don't support cancellation.
The other difference between `PollerLike` and `SimplePollerLike` is in how the operation state is defined. `PollerLike` defines its state shape as a type that extends `PollOperationState<TResult>`: eb0aa1da67/sdk/core/core-lro/src/pollOperation.ts (L17-L38) Mainly, it has `isStarted`, `isCanceled`, and `isCompleted` booleans. However, the semantics of `isCanceled` can be confusing; should it be set at the time when the cancellation request is sent or when the service responds that the operation is canceled?
To avoid this confusion, `OperationState` replaces those booleans with a status field typed as a union of states:
```ts
export interface OperationState<TResult> {
/**
* The current status of the operation.
*/
status: "notStarted" | "running" | "succeeded" | "canceling" | "canceled" | "failed";
/**
* Will exist if the operation encountered any error.
*/
error?: Error;
/**
* Will exist if the operation produced a result of the expected type.
*/
result?: TResult;
}
```
Which makes it clear that it reflects operation status after each poll.
## Use case: Implement LRO cancellation in @azure/ai-language-text (https://github.com/Azure/azure-sdk-for-js/pull/22852)
The proposal is implemented in a different PR: https://github.com/Azure/azure-sdk-for-js/pull/22852
This LRO cancellation returns a 202 response so cancellation itself is an LRO. I am proposing to implement cancellation by returning a function named `sendCancellationRequest` alongside the operation poller, so the return type is:
```ts
interface AnalyzeBatchOperation {
poller: AnalyzeBatchPoller;
sendCancellationRequest(): Promise<void>;
}
```
And client code can simply destruct the output object as follows to get access to the poller and the cancellation function:
```ts
const { poller, sendCancellationRequest } = await client.beginAnalyzeBatch(actions, documents, "en");
...
// Let's cancel the operation
await sendCancellationRequest(); // poller.cancelOperation() no longer exists
```
Design considerations:
- cancellation as a concept is not related to polling so a poller is not the right place to send cancellation requests
- canceling an operation requires knowing the operation ID/location, but we shouldn't ask customers for that information because it is an implementation detail
- cancellation is an LRO but it doesn't have its own operation location, i.e. the operation location is the same for the operation being cancelled and the cancellation operation itself. This means there is no need to create a dedicated poller for the cancellation operation and customers could still use the existing poller to await their operation or its cancellation
- cancellation is a POST request with no interesting response, so it should be modeled programmatically as `(): Promise<void>` function
Resets the code base for the Text Analytics library to the one used to release v5.1.0. The reset was done by checking out the library folder from the github tag for the v5.1.0 release. However, I updated the code base to use @azure/core-tracing@^1.0.0 too.
This is needed because v6.0.0-beta.1 has been moved in https://github.com/Azure/azure-sdk-for-js/pull/22640 into a new package: @azure/ai-language-text.
***NO_CI***
This is a follow-up to PR #21536 and PR #21537.
- upgrade typescript to 4.6.0 for communication packages
- upgrade typescript to 4.6 for test-utils and dev-tool
- upgrade typescript to 4.6
Fixes https://github.com/Azure/azure-sdk-for-js/issues/22067
# Improving the cancellation behavior
## Problem statement
Our poller interface exposes a method for requesting the cancellation of the polled operation and the poller can no longer poll once that method is called. This behavior is problematic as follows:
1. Any potential partial results were not retrieved because polling stopped prematurely
2. `isCanceled` property is being set in two places unnecessarily making it difficult to reason about the poller state
## How cancellation works
The cancellation method is defined as follows: 56ad04745c/sdk/core/core-lro/src/poller.ts (L82)
And is basically implemented by: 56ad04745c/sdk/core/core-lro/src/poller.ts (L439-L449) where `cancelOnce` is the one that stops polling: 56ad04745c/sdk/core/core-lro/src/poller.ts (L354-L359) Notice the following:
1. The poller is marked as stopped as soon as the `cancelOperation` method is called (line 441)
2. The promise returned by `pollUntilDone()` is rejected if the `cancelOperation` promise is resolved (line 357).
3. The `poll` promise is not rejected and doesn't do anything useful after the `cancelOperation` one has resolved: e48b15de0f/sdk/core/core-lro/test/engine.spec.ts (L648-L657)
## Proposed cancellation behavior
Starting from the principal that the service knows best, the operation should be considered in a terminal state only if the service says so whether that is a canceled state or not. To implement this behavior, The proposed change updates `cancelOperation` to no longer mark the poller as stopped and updates `pollOnce` to do so instead only if after polling the operation is deemed as canceled. Similarly, the rejection of the `pollUntilDone()` promise has been moved to inside `pollOnce`. As the poller no longer prematurely stops, the `poll()` promise still works (i.e. sends a polling request) and will get rejected if the operation is canceled.
__Note that this proposal is a non-trivial behavior change__ but I think one could argue that it is more of a fix rather than a deliberate behavior change. I wonder if there is any code that relies on early stopping of polling.
* [core-lro] Set isCancelled when status is cancelled
* don't check for isCanceled in TA test
* fix lint
* address feedback and handle cancellation uniformly
* address feedback
* add tests
* edit
* revert behavioral change
* Update sdk/textanalytics/ai-text-analytics/package.json
Co-authored-by: Will Temple <witemple@microsoft.com>
The perf framework previously did not support true multi-core operation. This PR provides an implementation of multicore support based on a message-passing model.
Two new options are added to the perf framework as part of this PR:
* `--cpus`/`-c`: number specifying the number of cores (CPUs) to use. Defaults to 1, set to 0 to match the number of cores on the machine.
* `--use-worker-threads`: boolean, defaults to false. Pass this flag to use `worker_threads` instead of `child_process` for multithreading (see Multithreading implementations, below)
Changing @azure/core-rest-pipeline ‘s default maximum number of retries from 10 to 3.
3 is the maximum number of retries in other languages (I asked Python and .NET).
### Packages impacted by this PR
@azure/core-lro
### Issues associated with this PR
Fixes https://github.com/Azure/azure-sdk-for-js/issues/20647
### Describe the problem that is addressed by this PR
The [vNext draft of the REST guidelines](https://github.com/microsoft/api-guidelines/blob/vNext/azure/Guidelines.md#long-running-operations-with-status-monitor) retires the use of `Azure-AsyncOperation` header in favor of `Operation-Location`. For context, `Azure-AsyncOperation` is mainly used for scenarios where there are two URLs, one for polling (returned in the `Azure-AsyncOperation` header) and another to retrieve the resource being created (returned in the `Location` header).
### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
This PR enables clients to use `Operation-Location` for scenarios where `Azure-AsyncOperation` was used before by merging the logic for handling both into one. The merging was done by augmenting the existing logic for handling `Azure-AsyncOperation` as follows: To check if the polling was done (in `isPollingDone`), also check if the response status code was 202 or an unexpected one:
```ts
if (isUnexpectedPollingResponse(rawResponse) || rawResponse.statusCode === 202) {
return false;
}
```
The new merged logic is moved to `locationPolling.ts` and the old separate logic for handling `Operation-Location` was deleted.
I am open for alternative approaches, but I believe this is the simplest thing we can do.
### Are there test cases added in this PR? _(If not, why?)_
Yes
### Provide a list of related PRs _(if any)_
N/A
### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_
N/A
### Checklists
- [x] Added impacted package name to the issue description
- [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_
- [x] Added a changelog (if necessary)
This is a PR that is intended to solve issue #15746.
Summary of What I did:
- ran `rush add -p eslint-plugin-markdown --dev` in `common\tools\eslint-plugin-azure-sdk`
- modified `common\tools\eslint-plugin-azure-sdk\src\configs\azure-sdk-base.ts`:
- included the `eslint-plugin-markdown` plugin,
- added `override` configuration that splits typescript and javascript linting.
- used `no-restricted-import` to inhibit ES6 import usage.
- added `README.md` as target and removed `--ext` option of lint script in `sdk\textanalytics\ai-text-analytics\package.json` and fixed the existing error.
### Packages impacted by this PR
- `@azure/ai-text-analytics`
### Issues associated with this PR
- #19859
### Describe the problem that is addressed by this PR
Migrates Text Analytics to new recorder.
Thanks to #19530 we have a new HttpClient that uses Fetch. Currently, we can't make it the default because of recorded tests. However, we'd like folks to be able to try it out, which this PR makes possible.
The solution here is for tests that are dependent on XHR to pass in a custom HttpClient to allow the previous recordings to be used until we can migrate those packages to the new recorder.
* Rush update
* Format form-recognizer perf-tests
* Format form-recognizer
* Format ai-metrics-advisor
* Format perf-tests\ai-metrics-advisor
* format ai-text-analytics
* Format perf-tests\text-analytics
* Removing "src/**/*.ts" path from perf-test
* [Text Analytics] Fix rate limit reached issue
* create an options bag for the throttling retry policy
* use the public options to configure the max retries instead
* simplify the retry options type
* Update sdk/textanalytics/ai-text-analytics/test/public/utils/recordedClient.ts
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
* edit the comment
* Update sdk/core/core-rest-pipeline/CHANGELOG.md
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
As discussed in #17076, we no longer have the need for the `docs` script in each of our packages. This PR removes this script and the related dev dependency on typedoc
This PR makes the following updates regarding the `nyc` dependency
- Update to v15 from v14 across all packages
- Updates to `@azure/monitor-opentelemetry-exporter` as it failed to run the tests with the updated nyc.
- Update the test scripts to use the js files in the dist-esm folder like all other packages instead of using the ts-node plugin.
- Update one of the tests for `@azure/monitor-opentelemetry-exporter` to use the right path for package.json file now that the tests are being run from the dist-esm folder.
Random set of live tests were triggered from this PR to ensure that nyc works as expected.
The failure for data-tables is an unrelated service side issue
Resolves#19232
Reported in #18033.
Basically did a bulk find+replace everywhere. Things _seem_ to be working OK, but wouldn't be surprised if there's something somewhere I've missed, or somewhere where I've replaced something I shouldn't.
I've split the rename of PerfStress -> Perf and runAsync -> run into two commits for ease of review :)
## What
- Update API Extractor to the latest version (currently 7.18.11)
- Regenerate all API reviews by building all the packages
## Why
This is something we keep bumping into. First, we needed to upgrade API Extractor to allow us to re-export directly from
`@opentelemetry/api`. Then, it looks like we needed this to upgrade TypeScript to 4.4.
We are way behind on this version, and it's time to upgrade.
## Callouts
How noisy is this?! Here's what's happening - somewhere around 7.9 I think API Extractor improved the way it detects name
collisions with predefined globals. Things like KeyType, Response, etc.
If there's a clash, the generated API markdown file will rename <Item> to <Item_2> to make the name collision explicit.
Talking to folks on the team, and the poor souls that will be doing API Review approvals, we agreed that doing the upgrade
in one fell swoop is the way to go.
Resolves#9410
* Adding a note in the readme to release publicly
* rename `@azure/test-utils-recorder` to `@azure-tools/test-recorder`
* lock file
* delete recorder new file
- Our convention now is to use `types`.
- Some packages output type definition files into `types` directory but the `clean` scripts still use `typings`.
* [Identity] Incrementing the package version
* setting samples to the last released version of Identity
* upgraded non-samples to 2.0.0-beta.6
* found a fix for the CI issue
This PR adds missing changelog entries for the times we
- updated tracing dependencies to use the GA version of OpenTelemetry
- updated to target ES2017
## What
- Remove `setTracer`
- Remove `NoOpTracer` and `NoOpSpan`
- Use Otel's APIs where possible (like when creating a no-op)
- Respect AZURE_TRACING_DISABLED environment variable
- Include test support for tracing utilizing OTel's trace API
- Avoid injecting invalid `tracerparent` header fields
## Why
- `setTracer` was added before OTel had their own implementation of a global tracer provider. For consistency with other libraries we should use the global tracer that was registered. It also leaves us out of the business of maintaining caches, global tracers, and other annoying things.
- These are no longer needed, since OTel has a recommended way to wrap a span in a NoOp. And, if no tracer provider was registered, a non-recording tracer (NoOp) will be created. All managed by OTel
- Finally, AZURE_TRACING_DISABLED is one of the env vars our guidelines say we should support
Resolves#16088Resolves#15730Resolves#10205
## What
- Move `TestTracer`, `TestSpan`, and `SpanGraph` from @azure/core-tracing to @azure/test-utils
## Why
1. Having a folder called src/**test**/ does not play nicely with defaults generated by tools such as Yarn (specifically Yarn autoclean)
2. These are test support interfaces, and shouldn't be part of our public API anyway - now that we have @azure/test-utils it's the more appropriate location for them
Resolves#16265Resolves#16201
Related #13367
* displayed links as a list rather than a single line
* appconfiguration\Readme: displayed links as a list rather than a single line
* confidential-ledger-rest/README.md: display links as list
* container-registry/README.md: display links as list
* cosmos/README.md: display links as list
* iot-device-update/README.md: display links as list
* eventgrid/README.md: display links as list
* event-hubs/README.md: display links as list
* eventhubs-checkpointstore-blob/README.md: display links as list
* formrecognizer/README.md: display links as list
* identity/README.md: display links as list
* iot-modelsrepository/README.md: display links as list
* keyvault-admin/README.md: display links as list
* keyvault-certificates/README.md: display links as list
* keyvault-keys/README.md: display links as list
* keyvault-secrets/README.md: display links as list
* ai-metrics-advisor/README.md: display links as list
* mixedreality-authentication/README.md: display links as list
* monitor-query/README.md: display links as list
* purview-catalog-rest/README.md: display links as list
* purview-scanning-rest/README.md: display links as list
* quantum-jobs/README.md: display links as list
* search-documents/README.md: display links as list
* schema-registry/README.md: display links as list
* schema-registry-avro/README.md: display links as list
* service-bus/README.md: display links as list
* storage/storage-blob/README.md: display links as list
* storage-blob-changefeed/README.md: display links as list
* storage-file-datalake/README.md: display links as list
* storage-file-share/README.md: display links as list
* storage-queue/README.md: display links as list
* data-tables/README.md: display links as list
* ai-text-analytics/README.md: display links as list
* video-analyzer-edge/README.md: display links as list
* web-pubsub/README.md: display links as list
* web-pubsub-express/README.md: display links as list
* core-lro/README.md: display links as list
* changed from the word master to main
* changed the word master to main
* another update to the final reandme to change the word master to main
* Update README.md
fixed a type in the link
* Update sdk/anomalydetector/ai-anomaly-detector/README.md
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
* record weird tests
* Manually escaped recording text
* remove only
* Apply suggestions from code review
Co-authored-by: Will Temple <witemple@microsoft.com>
* rename recordings
* more renamings
Co-authored-by: Will Temple <witemple@microsoft.com>
Jeremy Meng
Minh-Anh Phan
Jeff Fisher
Deyaaeldeen Almahallawi
Jeremy Meng
Timo van Veenendaal
praveenkuttappan
Will Temple
Daniel Rodríguez
Wei Jun
Jonathan Cárdenas
Ramya Rao
Jiao Di (MSFT)
Azure SDK Bot
Timo van Veenendaal
Maor Leger
Harsha Nalluru
bashiMoha
