Skip to content
This repository has been archived by the owner on Dec 9, 2024. It is now read-only.

feat: Refactor runtime configuration to allow for non-node runtimes #348

Merged
merged 2 commits into from
Oct 3, 2019

Conversation

tbarlow12
Copy link
Contributor

What did you implement:

  • Refactored runtime to allow for non-node runtimes

How did you implement it:

  • Added FunctionRuntime configuration to provider
  • Extracting FunctionRuntime from runtime property of configuration within ConfigService
  • Refactored node-specific code in ARM template generation

How can we verify it:

  • Run a full deployment, check the ARM template

Todos:

Note: Run npm run test:ci to run all validation checks on proposed changes

  • Write tests and confirm existing functionality is not broken.
    Validate via npm test
  • Write documentation
  • Ensure there are no lint errors.
    Validate via npm run lint
    Note: Some reported issues can be automatically fixed by running npm run lint:fix
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

Copy link
Contributor

@PIC123 PIC123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Exciting that we will be able to support more than just node soon :D

@tbarlow12 tbarlow12 force-pushed the tabarlow/new-refactor-runtime branch from b46452c to 4e1420a Compare September 30, 2019 15:20
@tbarlow12 tbarlow12 closed this Sep 30, 2019
@tbarlow12 tbarlow12 reopened this Sep 30, 2019
@tbarlow12 tbarlow12 closed this Sep 30, 2019
@tbarlow12 tbarlow12 reopened this Sep 30, 2019
@tbarlow12 tbarlow12 closed this Sep 30, 2019
@tbarlow12 tbarlow12 reopened this Sep 30, 2019
@tbarlow12 tbarlow12 force-pushed the tabarlow/new-refactor-runtime branch from 4e1420a to 62e7284 Compare September 30, 2019 18:18
Copy link
Contributor

@wbreza wbreza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

functionRuntime?: FunctionRuntime;
}

export enum FunctionAppOS {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't look like this is being used anywhere? I'm assuming this will get used as Python support gets rolled in?

@tbarlow12 tbarlow12 merged commit e943b57 into dev Oct 3, 2019
wbreza pushed a commit that referenced this pull request Oct 6, 2019
…348)

- Added `FunctionRuntime` configuration to provider
- Extracting `FunctionRuntime` from `runtime` property of configuration within `ConfigService`
- Refactored node-specific code in ARM template generation
@tbarlow12 tbarlow12 deleted the tabarlow/new-refactor-runtime branch April 24, 2020 03:48
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.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants