-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Design an API that lets you use graphql-upload with apollo-server-lambda #4951
Comments
Probably it's best for
Unfortunately serverless environments tend to buffer the whole multipart request into memory, then pass that into the function after. This means the file uploads can't be processed while they stream in, and memory requirements balloon by whatever the size of the files is. Existing hacks using the jaydenseric/graphql-upload#155 (comment)
Ideally the new serverless API would be agnostic to all the serverless environments that exist (and might exist in the future) and instead of supporting specific vendors by name, can easily be adapted to either AWS Lambda or Google Cloud Functions. Maybe by processing a raw request body string? The Apollo community is welcome to contribute ideas for the above issues and submit a PR. |
Thanks @jaydenseric. I do agree that using today's graphql-upload with Lambda isn't optimal. On the other hand, people actually do use it, so my main goal is not completely breaking those folks when the built-in integration is removed in AS3. I think my ideas in #3508 (comment) are generic enough that they may just generally make apollo-server-lambda more usable, while also solving the graphql-upload integration issue. Hopefully the same hook would also support a non-capacitor graphql-upload implementation! |
One other approach here would be to completely replace |
Before this change, apollo-server-lambda contained a lot of code that tried to understand the formats of Lambda event (request) and result (response) formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion 1.0 and 2.0 as well as ALB. But understanding the intricacies of the various Lambda message formats isn't really what the Apollo Server project is about. A pretty large fraction of all maintenance on Apollo Server in 2021 has gone into tweaking details of the Lambda event parsing and making the Lambda handler support all AS features without the help of a library supporting composable middleware. This PR (targeted for AS3) throws away the laboriously constructed bespoke Lambda parsing and faux-middleware implementation and replaces it with two packages that solve these problems for us: `@vendia/serverless-express` which understands a variety of Lambda input and output formats and converts them to Express format, and `express`, the most popular Node library for defining HTTP server behavior. (Note that `@vendia/serverless-express` is not related to the `serverless` CLI/framework.) Now `apollo-server-lambda` is just a convenience wrapper around `apollo-server-express`. It has to deal with the difference in startup logic between serverless and non-serverless environments, but it doesn't have to reimplement all of the Lambda and HTTP logic from scratch. As an added advantage, you can now optionally provide your own express app to `apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to customize any of its behavior past the particular options we define, because Lambda doesn't have a built-in middleware framework. Since we are removing some built-in features like `graphql-upload` integration in AS3, it's important that we continue a way to add custom behavior to your Lambda server. Letting you define that custom behavior with a standard Express app seems reasonable. We recognize that Lambda users generally care strongly about bundle size, so adding two new dependencies may seem problematic. That said, we don't currently have a principled way of evaluating Lambda bundle sizes when we make choices in this project, and compared to other dependencies of Apollo Server, these new dependencies are not very large. For now, the improvement in maintainability and flexibility seems worth the bundle size increase. If `apollo-server-lambda` users want to help out with a new project of focusing on Lambda bundle size optimization, we can work together to define benchmarks based on realistic build/bundler conditions, and find other ways to reduce bundle size (eg, there is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`). Fixes #5078. Fixes #4951 (because that API is just "Express").
Before this change, apollo-server-lambda contained a lot of code that tried to understand the formats of Lambda event (request) and result (response) formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion 1.0 and 2.0 as well as ALB. But understanding the intricacies of the various Lambda message formats isn't really what the Apollo Server project is about. A pretty large fraction of all maintenance on Apollo Server in 2021 has gone into tweaking details of the Lambda event parsing and making the Lambda handler support all AS features without the help of a library supporting composable middleware. This PR (targeted for AS3) throws away the laboriously constructed bespoke Lambda parsing and faux-middleware implementation and replaces it with two packages that solve these problems for us: `@vendia/serverless-express` which understands a variety of Lambda input and output formats and converts them to Express format, and `express`, the most popular Node library for defining HTTP server behavior. (Note that `@vendia/serverless-express` is not related to the `serverless` CLI/framework.) Now `apollo-server-lambda` is just a convenience wrapper around `apollo-server-express`. It has to deal with the difference in startup logic between serverless and non-serverless environments, but it doesn't have to reimplement all of the Lambda and HTTP logic from scratch. As an added advantage, you can now optionally provide your own express app to `apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to customize any of its behavior past the particular options we define, because Lambda doesn't have a built-in middleware framework. Since we are removing some built-in features like `graphql-upload` integration in AS3, it's important that we continue a way to add custom behavior to your Lambda server. Letting you define that custom behavior with a standard Express app seems reasonable. We recognize that Lambda users generally care strongly about bundle size, so adding two new dependencies may seem problematic. That said, we don't currently have a principled way of evaluating Lambda bundle sizes when we make choices in this project, and compared to other dependencies of Apollo Server, these new dependencies are not very large. For now, the improvement in maintainability and flexibility seems worth the bundle size increase. If `apollo-server-lambda` users want to help out with a new project of focusing on Lambda bundle size optimization, we can work together to define benchmarks based on realistic build/bundler conditions, and find other ways to reduce bundle size (eg, there is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`). Fix inconsistency in the content-type returned from health checks (the old Lambda used `application/json` instead of `application/health+json` like most other integrations). This new version passes all of the existing integration tests (plus the `testApolloServer` suite from `apollo-server-integration-testsuite/src/ApolloServer.ts` which wasn't being run previously!) essentially out of the box. (I fleshed out the "mock server" implementation which converts from Node http requests to Lambda events a bit more, and changed, but no "core" code or test changes were needed other than fixing the health check `content-type`.) Fixes #5078. Fixes #4951 (because that API is just "Express").
Before this change, apollo-server-lambda contained a lot of code that tried to understand the formats of Lambda event (request) and result (response) formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion 1.0 and 2.0 as well as ALB. But understanding the intricacies of the various Lambda message formats isn't really what the Apollo Server project is about. A pretty large fraction of all maintenance on Apollo Server in 2021 has gone into tweaking details of the Lambda event parsing and making the Lambda handler support all AS features without the help of a library supporting composable middleware. This PR (targeted for AS3) throws away the laboriously constructed bespoke Lambda parsing and faux-middleware implementation and replaces it with two packages that solve these problems for us: `@vendia/serverless-express` which understands a variety of Lambda input and output formats and converts them to Express format, and `express`, the most popular Node library for defining HTTP server behavior. (Note that `@vendia/serverless-express` is not related to the `serverless` CLI/framework.) Now `apollo-server-lambda` is just a convenience wrapper around `apollo-server-express`. It has to deal with the difference in startup logic between serverless and non-serverless environments, but it doesn't have to reimplement all of the Lambda and HTTP logic from scratch. As an added advantage, you can now optionally provide your own express app to `apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to customize any of its behavior past the particular options we define, because Lambda doesn't have a built-in middleware framework. Since we are removing some built-in features like `graphql-upload` integration in AS3, it's important that we continue a way to add custom behavior to your Lambda server. Letting you define that custom behavior with a standard Express app seems reasonable. We recognize that Lambda users generally care strongly about bundle size, so adding two new dependencies may seem problematic. That said, we don't currently have a principled way of evaluating Lambda bundle sizes when we make choices in this project, and compared to other dependencies of Apollo Server, these new dependencies are not very large. For now, the improvement in maintainability and flexibility seems worth the bundle size increase. If `apollo-server-lambda` users want to help out with a new project of focusing on Lambda bundle size optimization, we can work together to define benchmarks based on realistic build/bundler conditions, and find other ways to reduce bundle size (eg, there is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`). Fix inconsistency in the content-type returned from health checks (the old Lambda used `application/json` instead of `application/health+json` like most other integrations). This new version passes all of the existing integration tests (plus the `testApolloServer` suite from `apollo-server-integration-testsuite/src/ApolloServer.ts` which wasn't being run previously!) essentially out of the box. (I fleshed out the "mock server" implementation which converts from Node http requests to Lambda events a bit more, and changed, but no "core" code or test changes were needed other than fixing the health check `content-type`.) Fixes #5078. Fixes #4951 (because that API is just "Express").
Before this change, apollo-server-lambda contained a lot of code that tried to understand the formats of Lambda event (request) and result (response) formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion 1.0 and 2.0 as well as ALB. But understanding the intricacies of the various Lambda message formats isn't really what the Apollo Server project is about. A pretty large fraction of all maintenance on Apollo Server in 2021 has gone into tweaking details of the Lambda event parsing and making the Lambda handler support all AS features without the help of a library supporting composable middleware. This PR (targeted for AS3) throws away the laboriously constructed bespoke Lambda parsing and faux-middleware implementation and replaces it with two packages that solve these problems for us: `@vendia/serverless-express` which understands a variety of Lambda input and output formats and converts them to Express format, and `express`, the most popular Node library for defining HTTP server behavior. (Note that `@vendia/serverless-express` is not related to the `serverless` CLI/framework.) Now `apollo-server-lambda` is just a convenience wrapper around `apollo-server-express`. It has to deal with the difference in startup logic between serverless and non-serverless environments, but it doesn't have to reimplement all of the Lambda and HTTP logic from scratch. As an added advantage, you can now optionally provide your own express app to `apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to customize any of its behavior past the particular options we define, because Lambda doesn't have a built-in middleware framework. Since we are removing some built-in features like `graphql-upload` integration in AS3, it's important that we continue a way to add custom behavior to your Lambda server. Letting you define that custom behavior with a standard Express app seems reasonable. We recognize that Lambda users generally care strongly about bundle size, so adding two new dependencies may seem problematic. That said, we don't currently have a principled way of evaluating Lambda bundle sizes when we make choices in this project, and compared to other dependencies of Apollo Server, these new dependencies are not very large. For now, the improvement in maintainability and flexibility seems worth the bundle size increase. If `apollo-server-lambda` users want to help out with a new project of focusing on Lambda bundle size optimization, we can work together to define benchmarks based on realistic build/bundler conditions, and find other ways to reduce bundle size (eg, there is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`). Fix inconsistency in the content-type returned from health checks (the old Lambda used `application/json` instead of `application/health+json` like most other integrations). This new version passes all of the existing integration tests (plus the `testApolloServer` suite from `apollo-server-integration-testsuite/src/ApolloServer.ts` which wasn't being run previously!) essentially out of the box. (I fleshed out the "mock server" implementation which converts from Node http requests to Lambda events a bit more, and changed, but no "core" code or test changes were needed other than fixing the health check `content-type`.) Fixes #5078. Fixes #4951 (because that API is just "Express").
Before this change, apollo-server-lambda contained a lot of code that tried to understand the formats of Lambda event (request) and result (response) formats. It tried to work with APIGatewayProxy messages of payloadFormatVersion 1.0 and 2.0 as well as ALB. But understanding the intricacies of the various Lambda message formats isn't really what the Apollo Server project is about. A pretty large fraction of all maintenance on Apollo Server in 2021 has gone into tweaking details of the Lambda event parsing and making the Lambda handler support all AS features without the help of a library supporting composable middleware. This PR (targeted for AS3) throws away the laboriously constructed bespoke Lambda parsing and faux-middleware implementation and replaces it with two packages that solve these problems for us: `@vendia/serverless-express` which understands a variety of Lambda input and output formats and converts them to Express format, and `express`, the most popular Node library for defining HTTP server behavior. (Note that `@vendia/serverless-express` is not related to the `serverless` CLI/framework.) Now `apollo-server-lambda` is just a convenience wrapper around `apollo-server-express`. It has to deal with the difference in startup logic between serverless and non-serverless environments, but it doesn't have to reimplement all of the Lambda and HTTP logic from scratch. As an added advantage, you can now optionally provide your own express app to `apollo-server-lambda`. Previously, `apollo-server-lambda` gave no real way to customize any of its behavior past the particular options we define, because Lambda doesn't have a built-in middleware framework. Since we are removing some built-in features like `graphql-upload` integration in AS3, it's important that we continue a way to add custom behavior to your Lambda server. Letting you define that custom behavior with a standard Express app seems reasonable. We recognize that Lambda users generally care strongly about bundle size, so adding two new dependencies may seem problematic. That said, we don't currently have a principled way of evaluating Lambda bundle sizes when we make choices in this project, and compared to other dependencies of Apollo Server, these new dependencies are not very large. For now, the improvement in maintainability and flexibility seems worth the bundle size increase. If `apollo-server-lambda` users want to help out with a new project of focusing on Lambda bundle size optimization, we can work together to define benchmarks based on realistic build/bundler conditions, and find other ways to reduce bundle size (eg, there is a fair amount of low hanging fruit inside `apollo-reporting-protobuf`). Fix inconsistency in the content-type returned from health checks (the old Lambda used `application/json` instead of `application/health+json` like most other integrations). This new version passes all of the existing integration tests (plus the `testApolloServer` suite from `apollo-server-integration-testsuite/src/ApolloServer.ts` which wasn't being run previously!) essentially out of the box. (I fleshed out the "mock server" implementation which converts from Node http requests to Lambda events a bit more, and changed, but no "core" code or test changes were needed other than fixing the health check `content-type`.) Fixes #5078. Fixes #4951 (because that API is just "Express").
We will remove the direct integration with
graphql-upload
from Apollo Server 3.It's easy to plug your favorite version of
graphql-upload
back in to a middleware-style web library like Express in front of Apollo Server's middleware, but that's not the case for environments like Lambda (the second most popular place to use Apollo Server). We should add some hooks toapollo-server-lambda
that allow people to use (among other things!) graphql-upload. The Apollo Server core team is happy to review such a PR but since we are not Lambda users ourselves, we aren't best situated to write that PR.Ideally we can get this new API out in ASv2 so that folks who want to use
graphql-upload
with Lambda can migrate to the new API before the old integration is removed in v3.See #3508 (comment) for some thoughts on how to implement this.
The text was updated successfully, but these errors were encountered: