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

Fix: Use correct displayName for cosmosDBTrigger #399

Merged
merged 2 commits into from
Mar 20, 2020

Conversation

ofekray
Copy link
Contributor

@ofekray ofekray commented Dec 26, 2019

What did you implement:

Used correct displayName for cosmosDBTrigger to fix getting "Binding not supported" error.

Closes #389

How did you implement it:

Changed the displayName from $cosmosDBIn_displayName to $cosmosDBTriggerIn_displayName

How can we verify it:

Try to deploy the following function before & after the change:

  cosmosChangeFeed:
    handler: cosmosChangeFeed.handler
    events:
      - cosmosDB:
        x-azure-settings:
          direction: in
          name: documents
          databaseName: databaseName
          collectionName: containerName
          leaseCollectionName: leases
          connectionStringSetting: COSMOS_DB_CONNECTION
          createLeaseCollectionIfNotExists: true

@ofekray
Copy link
Contributor Author

ofekray commented Dec 27, 2019

@tbarlow12, @wbreza.
This is a pretty small fix.
Would appreciate if you can take a look.

@tbarlow12
Copy link
Contributor

Going to close and re-open to see if the pipeline is working

@tbarlow12 tbarlow12 closed this Dec 30, 2019
@tbarlow12 tbarlow12 reopened this Dec 30, 2019
@tbarlow12
Copy link
Contributor

@mydiemho It looks like the pipelines aren't running for external contributors? Could you take a look when you get some time?

@tbarlow12
Copy link
Contributor

@ofekray Sorry for the radio silence. We're investigating why the pipelines aren't running for external contributors. Also, this change will require some additional fixes because the plugin first tries to get the bindings.json file from the official azure functions repo, where we got this original file. We'll probably just need to update that so it either pins to the version we have or check with that team to see why their file is wrong.

@mydiemho mydiemho closed this Mar 9, 2020
@mydiemho mydiemho reopened this Mar 9, 2020
@mydiemho
Copy link
Contributor

mydiemho commented Mar 9, 2020

/azp run

@codecov
Copy link

codecov bot commented Mar 9, 2020

Codecov Report

Merging #399 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #399   +/-   ##
=======================================
  Coverage   94.44%   94.44%           
=======================================
  Files          48       48           
  Lines        1782     1782           
  Branches      245      280   +35     
=======================================
  Hits         1683     1683           
  Misses         98       98           
  Partials        1        1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e85a8a...165d52b. Read the comment docs.

@mydiemho
Copy link
Contributor

mydiemho commented Mar 9, 2020

@tbarlow12 @ofekray the pipeline wasn't block for external contributor just access to a token. It is now unblock but we just have to fix a different issues on our end

Copy link
Contributor

@mydiemho mydiemho left a comment

Choose a reason for hiding this comment

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

thank you for this fix @ofekray

@tbarlow12 tbarlow12 self-requested a review March 20, 2020 21:19
Copy link
Contributor

@tbarlow12 tbarlow12 left a comment

Choose a reason for hiding this comment

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

Thanks @ofekray! I forgot that we had already resolved the fix that I mentioned above.

@tbarlow12 tbarlow12 merged commit 9b57dac into serverless:master Mar 20, 2020
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] <[email protected]>

* Fix displayName for cosmosDBTrigger (#399)

* Update bindings.json

Co-authored-by: [email protected] <Serverless Azure Functions>
Co-authored-by: My <[email protected]>
Co-authored-by: Wallace Breza <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ofek Bashan <[email protected]>
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] <[email protected]>

* Fix displayName for cosmosDBTrigger (#399)

* Update bindings.json

Co-authored-by: [email protected] <Serverless Azure Functions>
Co-authored-by: My <[email protected]>
Co-authored-by: Wallace Breza <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Ofek Bashan <[email protected]>
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.

CosmosDB bindings is broken in 1.0.2-13
3 participants