Worker Indexing - Moving common validate logic to utility #7807
Conversation
This file contains hidden or 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
mathewc
reviewed
Oct 29, 2021
test/WebJobs.Script.Tests/Description/FunctionDescriptorProviderTests.cs
Outdated
Show resolved
Hide resolved
mathewc
reviewed
Oct 29, 2021
src/WebJobs.Script/Utility.cs
Outdated
| } | ||
| } | ||
|
|
||
| public static void ValidateName(string name, bool isProxy = false) |
There was a problem hiding this comment.
Looks like we never pass isProxy=true here anymore? If not, I think we can remove this.
surgupta-msft
deleted the
surgupta/moving-common-validate-logic-to-utility
branch
surgupta-msft
added a commit
that referenced
this pull request
Nov 9, 2021
* Merge dev into branch containing all worker indexing code changes/additions. * Fix test failures by manually adding worker indexing environment variables/worker config variables to test cases. Add null checks to _workerRuntime and workerConfig code sections. * Fix test failures related to FunctionMetadataProviderFactory initialization. * Remove extra line of code in GrpcWorkerChannel and try something for integration tests * Remove unnecessary workerIndexing variable from FunctionMetadataManager * Remove experimental line * Fix build issue by removing unnecessary variable * Add experimental line to see if this fixes integration/non-integration tests * Removed _channels from RpcFunctionInvocationDispatcher, fixed boolean logic in FunctionMetadataProviderFactory * Renaming GRPC messages, fix validation and FinishInitialization * Renamed GrpcWorkerChannel worker indexing functions to match GRPC message names Fixed invocation troubles for worker indexed functions --> was just missing a few lines from ScriptHost * Removed WorkerCapabilities service * Adding utility function for checking worker indexing and also added lots of tests Replaced dispatcher.InitializeAsync(null) with new List<FunctionMetadata> as the parameter instead of null Renamed FunctionMetadataProviderTests as HostFunctionMetadataProviderTests * renaming a some methods and variables, changing some log messages * Removed the dispatcher from GetFunctionMetadata() method, removed the provider factory, updated the tests * Get the worker config based on runtime language * Updated Utility function, skip extension filtering if worker indexing is enabled * fixing tests * Fixed some Utility.CanWorkerIndex tests by adding env variable to testEnv Added new RawFunctionMetadata type Moved Binding parsing logic into WorkerFunctionMetadataProvider * Added retry options and configuration source to RpcFunctionMetadata and ValidateMetadata() validation logic Added simple HttpFunctionInvocationDispatcher implementation for worker indexing functions Cleaned ScriptHost worker indexing logic and WorkerFunctionMetadataProvider logic * Cleaning up some code after reviewing the diff comparison again * Add clarifying comments to new RpcFunctionMetadata attributes * Rename variable in protobuf * Add bundles test for bypassing filtering when worker is indexing Add detailed binding validation logic to WorkerFunctionMetadataProvider Add detailed comments to HTTP dispatcher * Add binding validation tests Throw NotSupportedException in HttpFunctionInvocationDispatcher for worker indexing-related functions * Sending runtime when invoking GetTestFunctionDispatcher * Making changes related to new proto file changes * Removing unnecessary imports * Removing blank line * Making forceRefresh as true in GetFunctionsMetadata Reinitializing functions array Setting env variable in test Removing env variable from tests debugging integration tests Integration tests Debugging integration tests Setting env variable for dotnet Env as node for test OnTimeoutException_OOP_HasExpectedLogs minor debug point minor Fixing test Validate_HostLogs Added extra logs for debugging tests Commenting extra log lines for test to pass * Making IFunctionMetadataProvider.GetFunctionMetadata() an async method (#7796) * Making IFunctionMetadataProvider.GetFunctionMetadata() an async method * Adding await Task.FromResult * Correcting the await operation * Removing duplicate GetFunctionMetadataAsync method * Minor fix * Worker Indexing - Moving common validate logic to utility (#7807) * Moving ValidateBinding to Utility * Moving ValidateFunction to utility and modifying tests to remove dupliation * Fixing PR comments * Removing proxy from message * Removing extra logs used for debugging * updating traces for appinsights node test * Adding comments * AppInsights tests correction * ApplicationInsights test fixing * using GetInitializedWorkerChannelsAsync instead of GetAllWorkerChannelsAsync Co-authored-by: soninaren <nasoni@microsoft.com> Co-authored-by: Surgupta <surgupta@microsoft.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.
Issue describing the changes in this PR
#7806
To move common validation logic into Utility -
HostFunctionMetadataProvider, WorkerFunctionMetadataProvider, and FunctionDescriptorProvider all seem to have similar validation logic that is based off each other. To reduce this code duplication, common elements across each of the validation pipelines should be moved to the Utility class. This has the potential to reduce test duplication too.
[Note]: The purpose of the PR is to verify if there is any more common logic to be moved to utility.