-
Notifications
You must be signed in to change notification settings - Fork 494
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] UnitTests: Fixes flaky test and properly dispose of object #2477
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
j82w
requested review from
ealsur,
FabianMeiswinkel,
khdang,
kirankumarkolli,
kirillg,
neildsh and
sboshra
as code owners
May 18, 2021 17:30
ealsur
previously approved these changes
May 18, 2021
Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/LocationCacheTests.cs
Show resolved
Hide resolved
FabianMeiswinkel
previously approved these changes
May 18, 2021
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.
LGTM - Thanks
…ransient database not found exception.
…azure-cosmos-dotnet-v3 into users/jawilley/ha/testFix
FabianMeiswinkel
approved these changes
May 19, 2021
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.
LGTM
j82w
added a commit
that referenced
this pull request
May 25, 2021
* [Preview] Item and Query Request Options: Change Dedicated Gateway MaxIntegratedCacheStaleness type in the header (#2425) To make MaxIntegratedCacheStaleness value language agnostic, we take TimeSpan as an input from the customer but send TotalMilliseconds in the header instead. * Changelog : Adds missing changes and fixes the dates (#2426) * Update changelog for 3.19.0-preview * Internal Gates: Fixes Tests and update pipelines (#2427) Fixes the read many test which is only used in preview runs Fixes the pipelines to all use the same windows version. Hoping this might be the cause the perf gates are failing for the preview build but not others. Updated the perf tests to only update the values if it has a change greater than 2% and to list the tests that exceeded the 5% in a separate list to make it easy to see which ones changed. * Patch : Refactors the TrySerializeValueParameter method for PatchOperation and makes it public (#2398) Exposes the TrySerializeValueParameter method for PatchOperation so that it can be consumed in Encryption package. Need access to the serialized value parameter so that it can be encrypted according to the client encryption policy defined by the user. * 3.19.0-Preview: Adds changelog updates (#2430) * Client Encryption: Adds integration with latest CosmosDb Preview Package 3.19.0-preview (#2433) This PR will upgrade the dependent CosmosDB Preview Package to 3.19.0-preview. Updates the Encryption preview release version to 1.0.0-previewV14 and the change log. Adds implementation for new methods. * [Internal] Encryption project: Adds gates to prevent breaking changes (#2434) * Change Feed Processor: Fixes initialization when client is set with EnableContentResponseOnWrite (#2436) * disabling default options * tests * using the input * optimizing * on delete * Changelog: Adds PR 2424 (#2439) The following PR was missed because it was merged after the initial changelog creation. This adds the missing change and all other changes were reviewed to verify no other PRs are missing. * Query: Fixes NotImplementedException in OrderByCrossPartitionQueryPipelineStage (#2437) * Dont throw NotImplementedException on cassandra types * Add more test coverage for OrderByCorssPartitionQueryPipelineStage * Add tests for multiple continuation tokens and multi item order by tokens * "not" -> "NOT" * remove gratuitous inner class Co-authored-by: j82w <j82w@users.noreply.github.com> * README: Adds NuGet badges (#2406) Replaced useless ADO pipeline badge "never built" with working badges for NuGet packages. Note: shield.io is a 3rd party website but is considered one of the most popular across OSS * Bulk: Refactors async task handling and reduces allocations (#2440) * reducing lookups * tests * Removing ContinueWith * less parameters * TaskCreationOptions * nre on trace * more TaskCreationOptions.RunContinuationsAsynchronously * no asynccache * adding base benchmark files * adding mocked cache * mocking batch * correct response mock * contracts * yml * yml fix * Reducing allocations Co-authored-by: j82w <j82w@users.noreply.github.com> * [Internal] Availability: Adds partition level retry logic for request timeout (#2446) If a write request timeouts out it should retry on another region. It's possible that the current region is down and the backend has failed over to another region. This will ensure that the SDK will failover in such scenarios. Write request can not be retried. It's not clear if the request made it to the service or not. Only being used for write requests as if it is not the primary region it will give a fast and clear write forbidden exception. This allows the SDK to figure out the correct primary region. Improved the trace messages to include the location and resource address. This is needed to help which SDK instance is hitting the client retry policy. * Client Encryption: Fixes issue with clients using incorrect/stale Encryption Policy or Encryption Keys from the cache. (#2403) This PR fixes issue with clients using incorrect or stale policy when cache is not updated with the latest Encryption policy or Encryption Key, which could occur when a container or database is deleted and recreated with same Id but with different policy and Keys. * Availability: Fixes account refresh logic on gateway outage (#2447) The previous behavior would not refresh the account information if there was a gateway outage, and it would depend on the background refresh to update the information. The background refresh can take up to 5 minutes before detecting a change. HttpRequestExceptions will trigger an account refresh every 15 seconds. A config was added to override the minimum time between refreshes. Account refresh will be a no-op if a different thread is already updating the information Updated the traces to include the name to make it easier to discover GlobalEndpointManager is refactored to use 2 methods instead of different overloads on 1 method. This makes it clear that 1 method loads the account information and starts the background refresh. The other is forcing a refresh outside the background refresh. * Query: Fixes native c# parser not recognizing query with multiple IN statements (#2451) * Modifies the grammar * Adds test * addressed code reviews Co-authored-by: Minh Le <leminh@microsoft.com> Co-authored-by: j82w <j82w@users.noreply.github.com> * Client Encryption : Adds the implementation for new ChangeFeed APIs (#2452) * Implement the new ChangeFeed APIs for Encryption * Address comments. * Bulk: Fixes diagnostic traces by removing redundant info and adding correct retry context (#2455) * wiring trace * adding trace as a list * fixing tests * Baseline tests should be independent ops * updating baseline * adding retry baseline * adding test * more tests * removing client config duplication * Adding AddChild * baselines * more tests * sealed * Client Encryption : Fixes query support on encrypted parameters and fixes samples (#2445) This PR add fix to support non primitive type value as parameter value for Query support on Encrypted parameters. The changes also allows for creating documents when a client/user has no access to a Key provided Null is passed as property value. * [INTERNAL] Client Telemetry: Adds shaded HdrHistogram Library (#2459) As part of this PR, We are adding HDRHistogram support for Client Telemetry implementation where it will used to calculate different metrics. Source We are shading this library from https://github.com/HdrHistogram/HdrHistogram.NET Legal Made below changes to get approval from Legal team: I have registered the component in CG, using cgmanifest.json, for security and legal approval. There are no alerts. Copied the HdrHistogram license.txt in the repo. Mentioned Creative Commons License in the header of each file. * Permission: Fixes documentation on resource token range limits (#2460) ResourceToken has a minimum expiry period of 10 minutes (or 600 seconds). In the current code documentation, it is captured as 10 seconds, which is not in parity with CosmosDB limits. Aim of the change, is to update the correct range in code documentation. Supporting detail for 10 minutes: Azure/azure-sdk-for-js#10077 https://docs.microsoft.com/en-us/azure/cosmos-db/concepts-limits#per-request-limits * [Client Encryption]: Removes Plaintext encryption type support. (#2453) The PR removes the support for Plaintext encryption type. * [Client Encryption] Patch: Adds client encryption support for patch (#2448) * Add support for Patch for public model of client-side encryption * Client encryption: Adds validation that partition key paths are not encrypted (#2449) Validate partition key paths (i.e. their top level ancestors) are not specified for encryption in the client encryption policy. Ensure that policies with newer versions can de-serialize correctly, and expose the version so that the encryption package can fail correctly upon newer policy versions. * Diagnostics: Adds wrapper exception to include traces on ObjectDisposedException (#2465) Adds the internal CosmosObjectDisposedException. The new internal type is used include the CosmosDiagnostics into the Exception. It is just a wrapper to modify the Message and ToString() to include the additional information. * [Internal] Engineering: Refactors code to reduce technical debt (#2470) This pull request is very simple and aims to reduce the technical debt. It specifically targets the "Duplicate casts should not be made" rule in SonarQube. Update: As pointed out by @ealsur, the SonarQube fixes touched OSS files that probably should't be modified. To make this contribution more valuable, I fixed the following ReSharper issues: "Replace if statement with null-propagating code" rule "Merge sequential checks in && or || expressions" rule * 3.19.0-preview1: Adds new release version and update changelog (#2472) This PR is bumping the version to 3.19.0-preview1, updating the changelog, and adding the contract changes. * Client Encryption: Adds integration with latest CosmosDb package 3.19.0-preview1 and check for Client Encryption Policy format version. (#2475) This PR integrates the encryption package with the latest CosmosDb preview package and adds a check for Client Encryption Policy format version. * [Internal] UnitTests: Fixes flaky test and properly dispose of object (#2477) * Fix flaky test and properly dispose of object * Remove validation * Fix EndToEndTraceWriterBaselineTests to use async method and to fix transient database not found exception. * Change Feed: Fixes exceptions generating "Change Feed should always have a next page" (#2474) * hasMoreResults * Tests * Fix: Unhandled exceptions * tests * more tests * another hasmoreresults * not datatest * note * removing extra * Reducing nesting * Fixing exception stack * fixing nesting * renaming tests * Rethinking the solution * fixing test Co-authored-by: j82w <j82w@users.noreply.github.com> * UserAgent: Adds flag to user agent to show if region failover is configured (#2487) Do to recent outages and customer issues a flag is being added to the user agent to show if a client is properly configure to failover to new region in the case of a region outage. D - Endpoint discovery disabled S - Single application region is configured L - List of regions specified N - No regions specified * [Preview] AAD: Fixes token refresh interval, exception handling, and retry logic (#2481) 1. The default token refresh interval is now 50% of the tokens lifespan. If it fails to get the token it will retry again at 50% of the remaining lifespan until less 1 minute remains in which case the requests will start to fail with the token refresh exception. 2. Refactors several APIs to use the new interface to allow the ITrace to be passed in to the Auth logic. 3. SDK just throws the exception from TokenCredential rather than wrapping it in a CosmosException. This follows the Java and Azure Core implementation and will avoid confusion on if the error came from AAD or Cosmos. 4. Reduced the number of retries to 2 to avoid to many requests to AAD. 5. Removed the timeout logic on token refresh as this is handled by the TokenCredential implementation. * Availability: Fixes the get account information to stop the background refresh after Client is disposed (#2483) Fixes a bug where the get account information was continuing after the client and GlobalEndpointManager was disposed. It now wires through the cancellation token. Adding the missing using statements which are missing from several tests. This causes the background threads to continue to run after the test is done. * Azure Active Directory: Adds CosmosClient TokenCredential APIs to GA SDK (#2482) This change the Azure Active Directory APIs to the GA SDK. * Client Encryption : Adds Encryption Package 1.0.0-previewV15 to samples (#2492) Updates the Encryption Package used in the sample code. * Change Feed: Fixes CancellationToken support on ChangeFeedIterator (#2490) * wiring * tests * fixing flow * more tests * validating pipeline * With reset * Polishing test to verify success case Co-authored-by: j82w <j82w@users.noreply.github.com> * Bulk: Adds retry for patch operations (#2390) * adding substatuscode * tests * more tests * merge with master Co-authored-by: j82w <j82w@users.noreply.github.com> * CosmosNullReferenceException: Adds a new internal exception to add CosmosDiagnostics to NullReferenceExceptions (#2493) * Add cosmos diagnostics to null ref exception * Add query test * Diagnostics: Fixes default setting in Consistency Config Serialization (#2498) When serializing ConsistencyConfig class, if the user is not setting the ConsistencyLevel, we defaulted it to Strong. This PR adds "NotSet" to such cases. * 3.19.0: Adds new release, changelog, and contract updates (#2495) This PR is bumping the SDK version to 3.19.0, updating the changelog, and the contract files. Co-authored-by: Soyoung Eom <soeom@microsoft.com> Co-authored-by: anujtoshniwal <62551957+anujtoshniwal@users.noreply.github.com> Co-authored-by: Matias Quaranta <ealsur@users.noreply.github.com> Co-authored-by: neildsh <35383880+neildsh@users.noreply.github.com> Co-authored-by: Alexander Batishchev <abatishchev@gmail.com> Co-authored-by: Santosh Kulkarni <66682828+kr-santosh@users.noreply.github.com> Co-authored-by: leminh98 <leminh.ams@gmail.com> Co-authored-by: Minh Le <leminh@microsoft.com> Co-authored-by: Sourabh Jain <sourabh.jain.107@gmail.com> Co-authored-by: arorainms <75715791+arorainms@users.noreply.github.com> Co-authored-by: abhijitpai <abpai@microsoft.com> Co-authored-by: Martin <modermatt@tuta.io> Co-authored-by: Asket Agarwal <asket.agarwal96@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Description
This fixes the flaky GetDatabaseAccountFromAnyLocationsMockTestAsync by removing some of the time validation that caused it to fail on slower machines like the gates.
LocationCacheTests now properly dispose of the GlobalEndpointManager which stop the background refreshes. This is likely the cause of the random traces showing up in the CI output about the account refresh.
Type of change
Please delete options that are not relevant.
Closing issues
To automatically close an issue: closes #IssueNumber