Skip to content

Conversation

@Nyholm
Copy link
Contributor

@Nyholm Nyholm commented Sep 20, 2021

This is an alternative to #889

It will reuse the function layer. It will avoid using bootstrap.php if APP_RUNTIME is defined.

FYI @deleugpn

@deleugpn
Copy link
Member

This change couples Bref internal runtime with something that is very Symfony Runtime specific in such a way that if a Bref user, by sheer luck, defines an environment variable called APP_RUNTIME just for the sake of their own application, their deployment would break without any sense

@Nyholm
Copy link
Contributor Author

Nyholm commented Sep 21, 2021

Thank you for the review.

I see a few ways forward:
A) Do you prefer #889 over this?
B) Should we make a release with a deprecation notice for users that is using the function layer an environment variable APP_RUNTIME?
C) Introduce a second environment variable to control if we should use runtime or not. ie BREF_USE_SYMFONY_RUNTIME.
D) Ignore the fact that a new environment variable might change the behavior. Just like #812, #556 etc.

What would you prefer? Do you see other alternatives?

@deleugpn
Copy link
Member

deleugpn commented Sep 21, 2021

I don't really have a suggestion at this time. From my personal perspective, this PR touches upon the foundation of Bref in a way that doesn't really make much sense for Bref. To put it in other words, if the LambdaRuntime class is internal because it's part of the internal layer, then the bootstrap file is even "more internal" and this change, to address a specific framework and a specific package, doesn't seem to be the way to go.

#889 brings more layers per PHP version into maintenance scope and support for a specific framework and a specific package which doesn't fully fit Bref's position.

I interpret #1032 as Bref making a suggestion about an approach which at present isn't simple. From an end-user standpoint, if they're already trying to lift-and-shift their application into Serverless, the less friction we have the better it is. The more options the end user have, the more overwhelming the exercise becomes. You can see that even though you can deploy with Terraform, SAM or CDK, Bref only documents the Serverless Framework to make things simpler. Having to learn Bref and Symfony Runtime for it wouldn't be a good recommendation. In different circumstances when Serverless is much more mainstream and/or Runtime is much more mainstream, such a suggestion could make more sense.


On a more fundamental level, Bref is here to provide the PHP Binary for people that want to build Serverless Application. There are 2 major flavours: lift-and-shift 25+ years of knowledge into AWS Lambda (Web Apps) or building from scratch (Function). One is much closer to PHP existing ecosystem and the other is much closer to AWS Lambda ecosystem. Symfony Runtime end up reimplementing things that have been solved on Bref already in a way that adds less friction to the end user given the current state of Serverless.

Take Requests with Super Global as an example, Symfony Runtime is hiding $_SERVER behind a package, but isn't really reimplementing the FastCGI protocol. These attempts to bridge Runtime and Bref seems to put Bref in a more awkward position just because it was written in PHP. As a thought experiment, if we still had Bref 0.3 with NodeJS Runtime, Bref would be invoking the PHP Binary with the FastCGI protocol and Symfony Runtime would just accept global variables as-is without trying to abstract Bref away.

Perhaps if you try to implement a strategy where Symfony Runtime only replaces PHP-FPM portion of Bref, which is the actual Runtime, instead of replacing the root of Bref, the current state of Bref would probably serve better that approach. One challenge here is the fact that Bref delegates the execution process to it's user only when a Runtime execution array $event has happened, which is motivated by AWS Lambda execution model. However, it's still possible to make a Handler class where the constructor is an opportunity for "compile-time" dependencies while the handle method is the runtime dependency. It just so happens that the Symfony Runtime philosophy of making things an anonymous function doesn't leave room for this approach.

As for internal classes, I think a good way to interpret them is that any PHP file that is natively embedded into the layer are classified as internal and any file that comes via composer require respects the natural PHP ecosystem. This is because for Web Apps, Bref actually run PHP twice: the internal runtime and PHP-FPM and for the Function Runtime (which is closer to other AWS Lambda Runtime) what's important is to keep parity with other languages in the ecosystem. It's not that we can't make LambdaRuntime be public, it's just that it doesn't meet any fundamental requirements of the library and we need a strong argument to violate that foundation.

I apologize for the apparent difficulty I bring into this, but as open source maintainers we can all agree that we're looking out for what we think is best for our users with the information we have at hand. Having a single source of runtime is a bold move and as such it's only natural for it to face a lot of challenges.

@Nyholm
Copy link
Contributor Author

Nyholm commented Sep 21, 2021

On a more fundamental level, Bref is here to provide the PHP Binary for people that want to build Serverless Application.

Excellent.

As a thought experiment, if we still had Bref 0.3 with NodeJS Runtime, Bref would be invoking the PHP Binary

Yes. NodeJS would just do execute PHP with my index.php file.

Perhaps if you try to implement a strategy where Symfony Runtime only replaces PHP-FPM portion of Bref

That is exactly what the runtime component is doing.

One challenge here is the fact that Bref delegates the execution process to it's user only when a Runtime execution array $event has happened, which is motivated by AWS Lambda execution model.

That is not 100% true. The function layer will call autoload.php before the request comes in.

This is because for Web Apps, Bref actually run PHP twice: the internal runtime and PHP-FPM

One could argue that this is overhead. And a more "normal" process should not be do this. We should do what other AWS Lambda Runtime do. Which is exactly what I propose with the Symfony Runtime component.

It's not that we can't make LambdaRuntime be public, it's just that it doesn't meet any fundamental requirements of the library and we need a strong argument to violate that foundation.

So you are saying that you don't see my use case in runtime/bref to use the LambdaRuntime class?
If I ask my question differently then: Could you help me come up with a way forward so Symfony 5.3+ applications (which use the runtime component) can work with Bref without any pain?

@deleugpn
Copy link
Member

Perhaps if you try to implement a strategy where Symfony Runtime only replaces PHP-FPM portion of Bref

That is exactly what the runtime component is doing.

From what I saw, the runtime is replacing the bootstrap file and the entire start up process, it's not just replacing FPM.

One could argue that this is overhead. And a more "normal" process should not be do this. We should do what other AWS Lambda Runtime do. Which is exactly what I propose with the Symfony Runtime component.

We could implement the entire LambdaRuntime class using Bash, but that would be harder to maintain, so the overhead is justified as it makes it a thousand times easier for maintenance/contribution from/by PHP developers.

Could you help me come up with a way forward so Symfony 5.3+ applications (which use the runtime component) can work with Bref without any pain?

if you use the FPM Layer and return the HTTP Kernel on your handler, does it not work?

@Nyholm
Copy link
Contributor Author

Nyholm commented Sep 21, 2021

if you use the FPM Layer and return the HTTP Kernel on your handler, does it not work?

Thank you for this suggestion and for trying to think of a solution.

It does work. However you have the following problem/issues:

  • The timeout issue (can be solved if not using FPM)
  • User has to set the trusted_headers on the Request to support HTTPS
  • You have different layers for S3/SQS handlers compared to HTTP applications (more options for the user)
  • Overhead with using PHP-FPM (it also isn't how other AWS Lambda runtimes are doing)

For these reasons, I would like that Symfony 5.3+ applications (and other Symfony/Laravel applications) to be able to use the Runtime component in a more native way. Ie, without PHP-FPM.

Is there another way that you think we can move forward with this?

@mnapoli
Copy link
Member

mnapoli commented Sep 27, 2021

As discussed on Slack, could #694 be used to run whatever Symfony Runtime needs to run to resolve handlers?

@deleugpn
Copy link
Member

@mnapoli in the current state, no, because Symfony Runtime hijacks the bootstrap and call exit(). We don't get a chance to do anything besides running the require autoload_runtime.php. They don't delegate the control back so the rest of the original bootstrap.php never runs

@mnapoli
Copy link
Member

mnapoli commented Dec 30, 2021

Thanks for sparking all these discussions, glad we finally have Symfony Runtime implemented in https://github.com/brefphp/symfony-bridge

@mnapoli mnapoli closed this Dec 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants