add basic function implementation

[edwardfoyle]

Issue #, if available:

Description of changes:
Basic functions implementation. This is pretty much just a passthrough to the native CDK Function construct at this point. But it's a starting point for us to build some of our higher level abstractions like introspecting inline functions and wiring up auth triggers, etc.

I also updated the existing integration test to include data and functions. Adding data should have probably gone in a separate PR, but it was easy enough to add it here so I went for it. I can split it into a separate PR if it's an issue though

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

[sobolk]

Looks good overall.

[sobolk]

Should we make generateContainerEntry async ?

In general I'd use async apis if they're available and make our stack async top bottom where applicable.

[edwardfoyle]

The difficulty is all of this rolls up into the Backend class constructor which cannot be async. So we can either wrap this in an IIFE or use the sync version of the function

[sobolk]

This makes me ask similar question Praveen asked below https://github.com/aws-amplify/samsara-cli/pull/87/files/f5268928d7669a2e8fb371aa837409c59e12f27f#r1268260426 .

Is this right place to do this ?

Perhaps we should wrap/extend (not sure which is right but I'd try composition first) function construct and hook up build into synthesis process ?

[edwardfoyle]

hook up build into synthesis process

isn't that what's happening here? All of this runs during synth, including this build. I can't think of how we would get this to execute during deploy time unless the scripts were defined in a separate config file outside of the TS definition

[edwardfoyle]

I suppose one idea could be something like this:

const myFunc = await new Func({}).build('tsc whatever')

The build method would look like:

class AmplifyFunctionFactory {
  async build(command: string): Promise<AmplifyFunctionFactory> {
    await execaCommand(command);
    return this;
  }
}

This would require customers are using node 18 and define the function in a module so they can use top-level await, but it would allow the build to happen async before the backend class is initialized

[edwardfoyle]

The tryBundle function that local bundlers must implement is synchronous. So we could use that but we'd still be forced to use synchronous functions for executing build commands there

[edwardfoyle]

I don't see the CDK bundler being fundamentally better than the solution this PR implements. Even if a local bundler is specified, it still has to be sync and if it fails, CDK automatically falls back to trying to bundle in docker which would also fail for customers who haven't set that up. Amplify has made explicit decisions in the past to not take a dependency on customers having docker installed locally and I don't think this PR is the time to revisit that decision. The implementation here allows us to write a normal async build function and leaves the door open to allowing customers to provide an async build function in the future

[sobolk]

I agree.
Fitting function build into sync handler, while inviting, seems fundamentally wrong if it involves I/O.

[Amplifiyer]

I agree that this PR is not the place for such decisions/discussions and that it needs to be properly designed and reviewed with the right audience (hopefully with cdk team as well). Based on the conversation in this thread, I propose that we

1. Merge this PR iff it unblocks us on the future implementation tasks
2. Engage product to get the product requirements clarified for the functions construct (Current CLI is plagued with bugs and feature requests)
3. Do a design review and revisit this PR if needed.

This design will also help ensure that the conclusions we are making in this PR regarding future assumptions (we will need I/O for building/bundling javascript functions) or based on our past decisions (we don’t want docker) will hold ground as we grow.

Some highly requested related bug/features in current CLI

• Support deploying lambda functions with custom CDK category amplify-cli#9055
• Support for Mocking functions with layers and Docker amplify-cli#4826
• Lambda functions as Docker Container Images amplify-cli#8120

[edwardfoyle]

Going to go ahead and merge this because it will unblock us to implement auth triggers.

I think this impl is simple enough that we won't have to backtrack much if we decide to change the approach for function building.

I agree we should review some of our longstanding function issues with product to see what we want to address in the future.

[Amplifiyer]

Few thoughts

• How does CDK accept a build command? I couldn't see much in their documentation except for bundling code where customer can specify multiple commands that CDK will execute during synth
• If CDK has code building support, why don't we use it as-is?
• If we do have to build 'building' nevertheless, shouldn't we do it in a docker container (similar to CDK) or in a VM (similar to amplify)?
• Is lambda layer in scope? If not now, would this solution work with lambda layers?

[Amplifiyer]

Thanks! Can you also leave a TODO on the build method with a github issue to revisit it later?

[sobolk]

factory method to produce factory that produces something :-)

[josefaidt]

does this mean if I specify npm run build-my-function it will run from amplify/functions/my-function? does this require me to have a corresponding package.json in that location?