Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More of annotate endpoint property for functions #1009

Merged
merged 9 commits into from
Dec 10, 2021

Conversation

taeold
Copy link
Contributor

@taeold taeold commented Nov 29, 2021

Follows up #999 to annotate each funuctions with __endpoint property.

Highlight of changes:

  • Extend unit test coverage for all v1 providers
  • Add __endpoint annotation to v1 task queue functions
  • Add __requiredAPIs annotation to task queue and scheduler functions
  • Explicitly add an empty __endpoint annotation to functions defined in the handler namespace, similar in style to the __trigger annotation.
  • Other minor nits

One more follow up to expect: I'll move the responsibility of adding labels for callable/scheduled function (e.g. deployment-callable, deployment-scheduled) from the SDK to the CLI soon.

@google-cla google-cla bot added the cla: yes label Nov 29, 2021
@taeold taeold marked this pull request as ready for review November 29, 2021 22:26
@taeold taeold requested review from inlined and colerogers November 29, 2021 22:26
Copy link
Contributor

@colerogers colerogers left a comment

Choose a reason for hiding this comment

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

Nice work on adding all these tests!

src/cloud-functions.ts Show resolved Hide resolved
src/v2/providers/pubsub.ts Show resolved Hide resolved
Copy link
Member

@inlined inlined left a comment

Choose a reason for hiding this comment

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

Good god that must have required patience to author. Good job!

spec/v1/providers/https.spec.ts Outdated Show resolved Hide resolved
src/cloud-functions.ts Show resolved Hide resolved
src/cloud-functions.ts Outdated Show resolved Hide resolved
src/cloud-functions.ts Outdated Show resolved Hide resolved
options,
'egressSettings',
'vpcConnectorEgressSettings'
);
endpoint.vpc = vpc;
}
convertIfPresent(endpoint, options, 'availableMemoryMb', 'memory', (mem) => {
const memoryLookup = {
Copy link
Member

Choose a reason for hiding this comment

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

FML that Entity has already been defined to have memoryMB yet Run now uses these friendly names (almost) directly.

src/providers/https.ts Show resolved Hide resolved
@taeold taeold merged commit 9a2cd47 into dl-container-contract Dec 10, 2021
@taeold taeold deleted the dl-endoint-prop-2 branch December 10, 2021 05:18
taeold added a commit that referenced this pull request Jan 19, 2022
* Annotate function triggers with __endpoint property (#999)

In addition to annotating function triggers with `__trigger` property, we add `__endpoint` annotation. This property will be used by the to-be-developed functions runtime to generate/declare deployment manifest that the CLI will use to deploy the function.

There are lots of code duplication between the utility functions for annotating the `__trigger` and `__endpoint` properties. I didn't try to refactor the common code since I expect that we will favor `__endpoint` property in the future.

* Add missing import.

* More of annotate endpoint property for functions (#1009)

Follows up #999 to annotate each funuctions with `__endpoint` property.

Highlight of changes:

* Extend unit test coverage for all v1 providers
* Add `__endpoint` annotation to v1 task queue functions
* Add `__requiredAPIs` annotation to task queue and scheduler functions
* Explicitly set `__endpoint` to undefined in the handler namespace
* No SDK-level label setting in the __endpoint annotation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants