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

defineFunction initial implementation #742

Merged
merged 29 commits into from
Nov 30, 2023
Merged

defineFunction initial implementation #742

merged 29 commits into from
Nov 30, 2023

Conversation

edwardfoyle
Copy link
Contributor

@edwardfoyle edwardfoyle commented Nov 27, 2023

Issue #, if available:

Description of changes:
Initial implementation of defineFunction. This PR includes:

  1. Removing the function-construct-alpha package
  2. Overhauling the interface of backend-function to remove Func.fromDir and Func.build in favor of defineFunction which acts as a wrapper around CDK's NodejsFunction
  3. Updating and adding tests for these changes

What's not included:

  1. The full set of properties we may want to expose in defineFunction is not implemented. Things like lambda memory size, timeout, and environment variables are easy additions in follow up PRs
  2. Additional features like fetching secrets within a lambda and cross-resource permissions are not included yet. These features will require some additional thinking / design.
  3. Python lambdas. There was discussion around using CDK's PythonLambda construct but this is still in alpha. I propose this goes in a follow up PR where we can evaluate python-specific challenges in isolation of the API changes that are happening here.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@edwardfoyle edwardfoyle added the run-e2e Label that will include e2e tests in PR checks workflow label Nov 27, 2023
Copy link

changeset-bot bot commented Nov 27, 2023

🦋 Changeset detected

Latest commit: 3cdb1a3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages
Name Type
@aws-amplify/backend-function Minor
create-amplify Patch
@aws-amplify/plugin-types Minor
@aws-amplify/backend Minor
@aws-amplify/auth-construct-alpha Patch
@aws-amplify/backend-auth Patch
@aws-amplify/backend-data Patch
@aws-amplify/backend-deployer Patch
@aws-amplify/backend-secret Patch
@aws-amplify/backend-storage Patch
@aws-amplify/platform-core Patch
@aws-amplify/sandbox Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@edwardfoyle edwardfoyle marked this pull request as ready for review November 28, 2023 23:48
sobolk
sobolk previously approved these changes Nov 29, 2023
Comment on lines +88 to +99
// if entry is not set, default to handler.ts
if (!this.props.entry) {
return path.join(getCallerDirectory(this.callerStack), 'handler.ts');
}

// if entry is absolute use that
if (path.isAbsolute(this.props.entry)) {
return this.props.entry;
}
return constructContainer.getOrCompute(
this.generator
) as AmplifyLambdaFunction;

// if entry is relative, compute with respect to the caller directory
return path.join(getCallerDirectory(this.callerStack), this.props.entry);
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have req for these fallbacks?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does req mean requirements? I'm not sure the PMs have explicitly stated it anywhere, but they have written examples where they seem to expect this behavior.

Amplifiyer
Amplifiyer previously approved these changes Nov 29, 2023
sobolk
sobolk previously approved these changes Nov 29, 2023
@edwardfoyle edwardfoyle dismissed stale reviews from sobolk and Amplifiyer via 8453633 November 29, 2023 18:54
@edwardfoyle edwardfoyle merged commit c6c39d0 into main Nov 30, 2023
22 checks passed
@edwardfoyle edwardfoyle deleted the func-reimpl branch November 30, 2023 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-e2e Label that will include e2e tests in PR checks workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants