Release 22.0 #1805
Merged
Release 22.0 #1805
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
Sync develop to release/20.0
* #1712 Migrate to Polly 8.0 * code review post merge * post PR * #1712 Migrate to Polly 8.0 * code review post merge * Update src/Ocelot.Provider.Polly/PollyQoSProvider.cs Co-authored-by: Raman Maksimchuk <[email protected]> * namespaces * Refactor QoS provider * Refactor AddPolly extension * Remove single quote because semicolon ends sentence --------- Co-authored-by: Ray <[email protected]> Co-authored-by: Raman Maksimchuk <[email protected]>
…nfiguration of a route (#1172) @EngRajabi, Mohsen Rajabi (7): add header to file cache option fix private set fix <none> <none> fix build fail fix: fix review comment. add unit test for change @raman-m, Raman Maksimchuk (1): Update caching.rst @raman-m (7): Fix errors Fix errors Fix styling warnings Refactor tests Add Delimiter Refactor generator Add unit tests
* use random ports in integration tests * Remove and Sort Usings * Code modern look * code review * code review: fix messages * code review: fix some messages * code review: Use simple `using` statement * Add Ocelot.Testing project --------- Co-authored-by: raman-m <[email protected]>
* Fix issue #952 and #1174 * Fix compiling errors * Fix warnings * Fix errors * Remove and Sort Usings * CA1845 Use span-based 'string.Concat' and 'AsSpan' instead of 'Substring'. Use 'AsSpan' with 'string.Concat' * IDE1006 Naming rule violation: These words must begin with upper case characters: {should_*}. Fix name violation * Add namespace * Fix build errors * Test class should match the name of tested class * Simplify too long class names, and they should match * Move to the parent folder which was empty * Fix warnings * Process dictionaries using LINQ to Objects approach * Fix code review issues from @RaynaldM * Remove tiny private helper with one reference * Fix warning & messages * Define theory instead of 2 facts * Add unit test for issue #952 * Add additional unit test for #952 to keep param * Add tests for issue #1174 * Remove unnecessary parameter * Copy routing.rst from released version * Refactor the middleware body for query params * Update routing.rst: Describe query string user scenarios --------- Co-authored-by: Stjepan Majdak <[email protected]> Co-authored-by: raman-m <[email protected]>
Sync develop to release/20.0.1
…g issue (#1753) * When using the QoS option "ExceptionsAllowedBeforeBreaking" the circuit breaker never opens the circuit. * merge issue, PortFinder * some code improvements, using httpresponsemessage status codes as a base for circuit breaker * Adding more unit tests, and trying to mitigate the test issues with the method "GivenThereIsAPossiblyBrokenServiceRunningOn" * fixing some test issues * setting timeout value to 5000 to avoid side effects * again timing issues * timing issues again * ok, first one ok * Revert "ok, first one ok" This reverts commit 2e4a673. * inline method * putting back logging for http request exception * removing logger configuration, back to default * adding a bit more tests to check the policy wrap * Removing TimeoutStrategy from parameters, it's set by default to pessimistic, at least one policy will be returned, so using First() in circuit breaker and removing the branch Policy == null from delegating handler. * Fix StyleCop warnings * Format parameters * Sort usings * since we might have two policies wrapped, timeout and circuit breaker, we can't use the name CircuitBreaker for polly qos provider, it's not right. Using PollyPolicyWrapper and AsnycPollyPolicy instead. * modifying circuit breaker delegating handler name, usin Polly policies instead * renaming CircuitBreakerFactory to PolicyWrapperFactory in tests * DRY for FileConfiguration, using FileConfigurationFactory * Add copy constructor * Refactor setup * Use expression body for method * Fix acceptance test * IDE1006 Naming rule violation: These words must begin with upper case characters * CA1816 Change ReturnsErrorTests.Dispose() to call GC.SuppressFinalize(object) * Sort usings * Use expression body for method * Return back named arguments --------- Co-authored-by: raman-m <[email protected]>
* Add "WebSocket Secure" and "SSL Errors" sections (#1180) Co-authored-by: raman-m <[email protected]>
Sync 'develop' to 'release/net8' (hotfix process)
…ngs.json (#1745) * changing string parameter for IOcelotLogger function to Func<string>, modifying asp dot net logger, only one main method and verifying if LogLevel is enabled. If log level isn't enabled, then return. pick 847dac7 changing string parameter for IOcelotLogger function to Func<string>, modifying asp dot net logger, only one main method and verifying if LogLevel is enabled. If log level isn't enabled, then return. pick d7a8397 adding back the logger methods with string as parameter, avoiding calling the factory when plain string are used. pick d413201 simplify method calls * adding back the logger methods with string as parameter, avoiding calling the factory when plain string are used. * simplify method calls * adding unit test case, If minimum log level not set then no logs are written * adding logging benchmark * code cleanup in steps and naming issues fixes pick c4f6dc9 adding loglevel acceptance tests, verifying that the logs are returned according to the minimum log level set in appsettings pick 478f139 enhanced unit tests, verifying 1) that the log method is only called when log level enabled 2) that the string function is only invoked when log level enabled * adding loglevel acceptance tests, verifying that the logs are returned according to the minimum log level set in appsettings * enhanced unit tests, verifying 1) that the log method is only called when log level enabled 2) that the string function is only invoked when log level enabled * weird issue with the merge. * adding comment * Update src/Ocelot/ServiceDiscovery/ServiceDiscoveryProviderFactory.cs Co-authored-by: Raman Maksimchuk <[email protected]> * Update src/Ocelot/Claims/Middleware/ClaimsToClaimsMiddleware.cs Co-authored-by: Raman Maksimchuk <[email protected]> * Update src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs Co-authored-by: Raman Maksimchuk <[email protected]> * Update src/Ocelot/DownstreamRouteFinder/Finder/DownstreamRouteProviderFactory.cs Co-authored-by: Raman Maksimchuk <[email protected]> * Update src/Ocelot/Logging/AspDotNetLogger.cs Co-authored-by: Raman Maksimchuk <[email protected]> * Update test/Ocelot.AcceptanceTests/LogLevelTests.cs Co-authored-by: Raman Maksimchuk <[email protected]> * Update src/Ocelot/Configuration/Repository/FileConfigurationPoller.cs Co-authored-by: Raman Maksimchuk <[email protected]> * As mentioned, using OcelotLogger instead of AspDotNeLogger as default logger name * Some code refactoring and usage of factories in LogLevelTests * Update src/Ocelot/Claims/Middleware/ClaimsToClaimsMiddleware.cs Co-authored-by: Raman Maksimchuk <[email protected]> * using overrided method WriteLog for strings, some changes as requested, * code changes after review 2 pick ad0e060 Update test/Ocelot.UnitTests/Middleware/OcelotPiplineBuilderTests.cs * checking test cases * adding ms logger benchmarks with console provider. Unfortunately, benchmark.net doesn't support "quiet" mode yet. * 2 small adjustments * Adding multi targets support for serilog * Fix warnings * Review new logger * Fix unit tests * The last change but not least * Update logging.rst: Add draft * Update logging.rst: Add RequestId section * Update logging.rst: "Best Practices" section * Update logging.rst: "Top Logging Performance?" subsection * Update logging.rst: Rewrite "Request ID" section * Update requestid.rst: Review and up to date * Update logging.rst: "Run Benchmarks" section --------- Co-authored-by: Raman Maksimchuk <[email protected]>
* #1783 More accurate logs for circuit breakers (and other "polly" exceptions) Remove try/catch in PollyPoliciesDelegatingHandler and add a more generic AddPolly<T> to be able to use a specific PollyQoSProvider * fix should_be_invalid_re_route_using_downstream_http_version UT * fix remarks on PR * arrange code * fix UT * merge with release/net8 branch * switch benchmark to Net8 * Fix warnings * Final review --------- Co-authored-by: Ray <[email protected]> Co-authored-by: raman-m <[email protected]>
RaynaldM
previously approved these changes
Nov 27, 2023
ggnaegi
requested changes
Nov 27, 2023
| public HttpContent Content { get; set; } | ||
| public virtual bool HasContent { get => _request?.Content != null; } | ||
|
|
||
| public virtual Task<string> ReadContentAsync() => HasContent |
There was a problem hiding this comment.
I think we shouldn't allow that kind of virtual methods for the purpose of mocks in unit tests. It's virtual so it could be modified and it would hide the original implementation.
There was a problem hiding this comment.
Let's double check this after release!
Now I'm going to revert #1172 ...
There was a problem hiding this comment.
After revert this virtual property will be removed.
| var cacheOptionsHeader = downstreamRoute?.CacheOptions?.Header; | ||
| if (!string.IsNullOrEmpty(cacheOptionsHeader)) | ||
| { | ||
| var header = downstreamRequest.Headers |
There was a problem hiding this comment.
I'm not sure it's really what we want. Here we are using the header value to create the MD5 key. I'm not sure the design is safe. Besides, no acceptance tests were provided by the author, so I'm concerned about that change.
There was a problem hiding this comment.
Design wasn't even discussed by the team. Raynald had provided review and gave approval.
The author just wanted to merge it. I was not careful allowing to merge without acceptance tests...
This is the incident with dev process violation! This is my fault!
There was a problem hiding this comment.
Reverted to old implementation... in commit 27a5999
Merged
ggnaegi
approved these changes
Nov 27, 2023
RaynaldM
approved these changes
Nov 28, 2023
Merged
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.
October 2023 release
Milestone: October'23
Milestone Status: Past due by 24 days because of #1790 and etc.
Version: 22.0
Features
P.S.
Removed from the release