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

Enable file path to data on bref-local with CDK #1627

Merged
merged 6 commits into from
Aug 31, 2023

Conversation

theoboldalex
Copy link
Contributor

This change provides command line access to the --path flag where an application is built on AWS CDK rather than Serverless framework. This enables the passing of a path to a JSON file when running vendor/bin/bref--local instead of passing data as a string.

If you agree with the premise of this change, I will also update the relevant documentation and include in the PR.

@mnapoli
Copy link
Member

mnapoli commented Aug 31, 2023

Hey, that sounds like a good idea. I'm not familiar with getopt, so I'm not sure what the command will look like. If you add docs that will probably be clearer to me 👍

@theoboldalex
Copy link
Contributor Author

Great, I will work on the docs and update the PR in due course. Thanks

@mnapoli
Copy link
Member

mnapoli commented Aug 31, 2023

Thanks!

@mnapoli mnapoli merged commit 6c9bbec into brefphp:master Aug 31, 2023
8 checks passed
@theoboldalex
Copy link
Contributor Author

theoboldalex commented Aug 31, 2023

I have had another look at this and think it might actually be better to match the way we invoke locally with the way we would if using Serverless.

Serverless
serverless bref:local -f hello --path=event.json

CDK/TF
vendor/bin/bref-local -f my-function.php --path event.json

If you think that is worth doing, I follow up with another PR.

Thanks

@mnapoli
Copy link
Member

mnapoli commented Aug 31, 2023

Sorry I'm missing what would be different with what you implemented?

@theoboldalex
Copy link
Contributor Author

Apologies, I didn't explain very well above.

In the current implementation, when we pass the --path flag, it must be passed before the handler file.

When running Bref with Serverless, the handler file has the -f flag. I could add this to the non-Serverless bref-local script to ensure consistency in how functions are invoked locally regardless of whether we are using Serverless, CDK or Terraform.

@mnapoli
Copy link
Member

mnapoli commented Sep 1, 2023

Oh yes that would be great indeed, in most PHP CLIs (artisan, Symfony Console) that's how it works. If you can do that without pulling in more dependencies that would be a very good improvement 👍

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