This repository has been archived by the owner on Dec 9, 2024. It is now read-only.
feat: ConfigService for centralizing configuration and simplifying BaseService #338
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
Codecov Report
@@ Coverage Diff @@
## dev #338 +/- ##
==========================================
+ Coverage 94.67% 94.73% +0.06%
==========================================
Files 47 48 +1
Lines 1727 1747 +20
Branches 242 248 +6
==========================================
+ Hits 1635 1655 +20
Misses 92 92
Continue to review full report at Codecov.
|
tbarlow12
force-pushed
the
tabarlow/new-config-service
branch
from
56d00e9 to
9e860be
Compare
wbreza
approved these changes
Sep 23, 2019
src/services/configService.test.ts
Outdated
| }); | ||
|
|
||
| describe("Service Principal Configuration", () => { | ||
|
|
src/services/configService.test.ts
Outdated
| }); | ||
|
|
||
| describe("Configurable Variables", () => { | ||
|
|
tbarlow12
force-pushed
the
tabarlow/new-config-service
branch
from
5ba7f05 to
8deee9f
Compare
tbarlow12
added a commit
that referenced
this pull request
Sep 27, 2019
…seService (#338) `ConfigService` and its usage. This removes the configuration logic from `BaseService` and keeps it all in one place. Also adds some constants for default configuration.
wbreza
pushed a commit
that referenced
this pull request
Oct 6, 2019
…seService (#338) `ConfigService` and its usage. This removes the configuration logic from `BaseService` and keeps it all in one place. Also adds some constants for default configuration.
tbarlow12
added a commit
that referenced
this pull request
May 7, 2020
* feat: ConfigService for centralizing configuration and simplifying BaseService (#338) `ConfigService` and its usage. This removes the configuration logic from `BaseService` and keeps it all in one place. Also adds some constants for default configuration. * release: Update prerelease version to 1.0.2-0 ***NO_CI*** * fix: Sync triggers on external package deployment (#339) Fix updating of function app settings (SDK call wasn't working) and syncing triggers for function apps running from external package. * release: Update prerelease version to 1.0.2-1 ***NO_CI*** * ci: fix failing Node 8 builds on windows agent (#345) Hosted agent roll out a fix that broke our builds: 1. Previously, npm wasn’t getting packaged with the version of node in the tool cache, ie. npm 5.6.0 should be used alongside Node 8.10.0. 1. The fix is to pin to a later version of Node 8 (e.g. 8.16.1) which comes with npm 6+ - https://nodejs.org/en/download/releases/. * This will probably slow the build down a little bit since the agent will have to download the version (instead of it being pre-installed), but we'll get the right version of npm for free. * fix job name * fix job name restrictiosn * still trying to get the right job name format * clean up job name * release: Update prerelease version to 1.0.2-2 ***NO_CI*** * release: Update prerelease version to 1.0.2-3 ***NO_CI*** * fix: Fix typing errors in ARM params and add interfaces (#347) * release: Update prerelease version to 1.0.2-4 ***NO_CI*** * fix: Update to support CosmosDB bindings (#350) Updatings the binding schema that is generated to support the changes made to Cosmos DB * release: Update prerelease version to 1.0.2-5 ***NO_CI*** * feat: Refactor runtime configuration to allow for non-node runtimes (#348) - Added `FunctionRuntime` configuration to provider - Extracting `FunctionRuntime` from `runtime` property of configuration within `ConfigService` - Refactored node-specific code in ARM template generation * release: Update prerelease version to 1.0.2-6 ***NO_CI*** * fix: Update GitHub Issue and PR templates (#353) * release: Update prerelease version to 1.0.2-7 ***NO_CI*** * fix: Sort deployments in descending order and fix APIM arm template (#354) - Updated parameter name in APIM arm template - Fixed bug of sorting deployments in ascending order, when it should have been descending. This would have pretty serious consequences, because it means that the comparison of ARM templates would always be targeting the first ever deployment, not the most recent. - Because the `sort()` function sorts the array in place, this bug was not being detected by the tests. Updated `resourceService` tests to create copies of the array rather than using the original reference when testing the validity of the result. * release: Update prerelease version to 1.0.2-8 ***NO_CI*** * release: Update patch version to 1.0.2 ***NO_CI*** * ci: Add GitHub workflow to move new issues to "To triage" column (#381) * build(deps): bump handlebars from 4.1.2 to 4.5.3 (#400) Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.1.2 to 4.5.3. - [Release notes](https://github.com/wycats/handlebars.js/releases) - [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md) - [Commits](handlebars-lang/handlebars.js@v4.1.2...v4.5.3) Signed-off-by: dependabot[bot] <support@github.com> * Fix displayName for cosmosDBTrigger (#399) * Update bindings.json Co-authored-by: sls-az@microsoft.com <Serverless Azure Functions> Co-authored-by: My <mydiemho@users.noreply.github.com> Co-authored-by: Wallace Breza <wallace@breza.me> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ofek Bashan <ofekray@gmail.com>
tbarlow12
added a commit
that referenced
this pull request
May 12, 2020
* feat: ConfigService for centralizing configuration and simplifying BaseService (#338) `ConfigService` and its usage. This removes the configuration logic from `BaseService` and keeps it all in one place. Also adds some constants for default configuration. * release: Update prerelease version to 1.0.2-0 ***NO_CI*** * fix: Sync triggers on external package deployment (#339) Fix updating of function app settings (SDK call wasn't working) and syncing triggers for function apps running from external package. * release: Update prerelease version to 1.0.2-1 ***NO_CI*** * ci: fix failing Node 8 builds on windows agent (#345) Hosted agent roll out a fix that broke our builds: 1. Previously, npm wasn’t getting packaged with the version of node in the tool cache, ie. npm 5.6.0 should be used alongside Node 8.10.0. 1. The fix is to pin to a later version of Node 8 (e.g. 8.16.1) which comes with npm 6+ - https://nodejs.org/en/download/releases/. * This will probably slow the build down a little bit since the agent will have to download the version (instead of it being pre-installed), but we'll get the right version of npm for free. * fix job name * fix job name restrictiosn * still trying to get the right job name format * clean up job name * release: Update prerelease version to 1.0.2-2 ***NO_CI*** * release: Update prerelease version to 1.0.2-3 ***NO_CI*** * fix: Fix typing errors in ARM params and add interfaces (#347) * release: Update prerelease version to 1.0.2-4 ***NO_CI*** * fix: Update to support CosmosDB bindings (#350) Updatings the binding schema that is generated to support the changes made to Cosmos DB * release: Update prerelease version to 1.0.2-5 ***NO_CI*** * feat: Refactor runtime configuration to allow for non-node runtimes (#348) - Added `FunctionRuntime` configuration to provider - Extracting `FunctionRuntime` from `runtime` property of configuration within `ConfigService` - Refactored node-specific code in ARM template generation * release: Update prerelease version to 1.0.2-6 ***NO_CI*** * fix: Update GitHub Issue and PR templates (#353) * release: Update prerelease version to 1.0.2-7 ***NO_CI*** * fix: Sort deployments in descending order and fix APIM arm template (#354) - Updated parameter name in APIM arm template - Fixed bug of sorting deployments in ascending order, when it should have been descending. This would have pretty serious consequences, because it means that the comparison of ARM templates would always be targeting the first ever deployment, not the most recent. - Because the `sort()` function sorts the array in place, this bug was not being detected by the tests. Updated `resourceService` tests to create copies of the array rather than using the original reference when testing the validity of the result. * release: Update prerelease version to 1.0.2-8 ***NO_CI*** * release: Update patch version to 1.0.2 ***NO_CI*** * ci: Add GitHub workflow to move new issues to "To triage" column (#381) * build(deps): bump handlebars from 4.1.2 to 4.5.3 (#400) Bumps [handlebars](https://github.com/wycats/handlebars.js) from 4.1.2 to 4.5.3. - [Release notes](https://github.com/wycats/handlebars.js/releases) - [Changelog](https://github.com/wycats/handlebars.js/blob/master/release-notes.md) - [Commits](handlebars-lang/handlebars.js@v4.1.2...v4.5.3) Signed-off-by: dependabot[bot] <support@github.com> * Fix displayName for cosmosDBTrigger (#399) * Update bindings.json Co-authored-by: sls-az@microsoft.com <Serverless Azure Functions> Co-authored-by: My <mydiemho@users.noreply.github.com> Co-authored-by: Wallace Breza <wallace@breza.me> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Ofek Bashan <ofekray@gmail.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
What did you implement:
ConfigServiceand its usage. This removes the configuration logic fromBaseServiceand keeps it all in one place. Also adds some constants for default configuration.In a future PR, the
ConfigServicewill cache its configuration so that the service is only configured one time in the initialization of the firstBaseServicesubclass. It will store the data in theserverless.variablesobject and checks to see if that value is populated on future runs.Also added some constants for default configuration values.
How did you implement it:
Removed the configuration logic from the
BaseServiceand placed inside theConfigService, which is initialized in the constructor of theBaseServiceHow can we verify it:
Todos:
Note: Run
npm run test:cito run all validation checks on proposed changesValidate via
npm testValidate via
npm run lintNote: Some reported issues can be automatically fixed by running
npm run lint:fixIs this ready for review?: YES
Is it a breaking change?: NO