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

Frankenphp #678

Merged
merged 8 commits into from
Jan 3, 2025
Merged

Frankenphp #678

merged 8 commits into from
Jan 3, 2025

Conversation

benbrummer
Copy link
Collaborator

  • frankenphp
    • Avoids workaround for react updates
    • Worker mode for Laravel Octane (invoiceninja needs to be made ready for it)
    • gzip, brotli and zstd for static files
  • mariadb 11.4 (lts)
  • valkey 8 (redis)
  • Removes supervisord
    • Use separate containers for worker/scheduler
      image

@benbrummer benbrummer changed the title Frankenphp, mariadb, valkey Frankenphp, mariadb, valkey (Breaking change) Dec 28, 2024
@benbrummer benbrummer changed the title Frankenphp, mariadb, valkey (Breaking change) Frankenphp Dec 28, 2024
@turbo124
Copy link
Member

@benbrummer

This looks awesome!

My vote for this one would be to create a new branch and build this as an optional image. There will need to be some careful testing around this due to the long lived PHP process approach with octane.

@benbrummer
Copy link
Collaborator Author

@turbo124 Do you have a suggestions for the branch-name?

Locally I'm already able to run with Laravel Octane, but I have an issue with
composer require laravel/octane if I do not set a my GitHub token in advance with composer config --global github-oauth.github.com TOKEN.

image

With --prefer-source
image

@benbrummer benbrummer marked this pull request as draft December 31, 2024 08:11
@turbo124
Copy link
Member

Let's use octane for the branch name, I will include Laravel octane in the dependencies

@benbrummer benbrummer changed the base branch from debian to octane December 31, 2024 11:36
@benbrummer benbrummer marked this pull request as ready for review December 31, 2024 16:54
@benbrummer
Copy link
Collaborator Author

  • composer is now handled in a separate stage
  • octane was tested locally but fails at the moment to install without providing a github token

Comment on lines +11 to +12
- ./.env:/app/.env
- ./php/php.ini:/usr/local/etc/php/conf.d/zzz-php.ini:ro

Choose a reason for hiding this comment

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

Do you think having a default zzz-1-php.ini baked in the image would be better?
And users could mount additional ini's to set more or override things like zzz-2-php.ini

Copy link
Collaborator Author

@benbrummer benbrummer Jan 9, 2025

Choose a reason for hiding this comment

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

Makes totaly sense to delivere a base php-file inside the image. Will be integrated with the next PR.

@turbo124 turbo124 merged commit 237abe4 into invoiceninja:octane Jan 3, 2025
1 check failed
@turbo124
Copy link
Member

turbo124 commented Jan 3, 2025

@benbrummer

The next time we tag a release, the octane dependency will be automatically handled from the main invoiceninja/invoiceninja repo.

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