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

chore: change from php-fpm to frankenphp #5123

Open
wants to merge 8 commits into
base: devel
Choose a base branch
from

Conversation

usu
Copy link
Member

@usu usu commented May 4, 2024

Fixes #4453
Changes from php-fpm to frankenphp + take over any other useful change from official api-platform template.

For a diff of all upstream changes since our last sync see
api-platform/api-platform@7642f86...v3.3.2

Open To Dos:

Excluded from this PR:

  • Worker mode (runtime mode)
  • Upgrade to postgres 16
  • api platform config rfc_7807_compliant_errors (upstream is true; kept at false)
  • doctrine config validate_xml_mapping (upstream is true; kept at false)
  • upstream switched from phpdocumentor/reflection-docblock to phpstan/phpdoc-parser (this generated wrong API responses; didn't debug further)
  • Some renaming of files to keep review of the changes a bit simpler. We could still do these renamings afterwards in a separate PR (docker-compose.yml is now compose.yml; api-platform.ini is now app.ini; etc.)

xdebug

In upstream, the default working folder has changed from /srv/api to /app.
In order for debugging to work again, I had to change the pathMappings in my debug config in VScode.
In other IDEs, something similar might be needed in order to find the correct sources.
Alterative would be that we stick to /srv/api instead of /app.

Old .vscode/launch.json:

{
    "version": "0.2.0",
    "configurations": [
        {
            "name": "XDebug: eCamp API",
            "type": "php",
            "request": "launch",
            "hostname": "localhost",
            "port": 9003,
            "log": true,
            "pathMappings": {
                "/srv/api": "${workspaceRoot}/api"
            }
        }
    ]
}

New .vscode/launch.json:

{
    "version": "0.2.0",
    "configurations": [
        {
            "name": "XDebug: eCamp API",
            "type": "php",
            "request": "launch",
            "hostname": "localhost",
            "port": 9003,
            "log": true,
            "pathMappings": {
                "/app": "${workspaceRoot}/api"
            }
        }
    ]
}

@usu usu added the deploy! Creates a feature branch deployment for this PR label May 4, 2024
@usu usu marked this pull request as ready for review May 5, 2024 12:22
@usu usu requested review from BacLuc and carlobeltrame May 5, 2024 12:23
@carlobeltrame carlobeltrame mentioned this pull request May 7, 2024
9 tasks
@BacLuc
Copy link
Contributor

BacLuc commented May 12, 2024

Very cool.
I will look at it as soon as the caching PR is through.

Copy link
Contributor

@BacLuc BacLuc left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just some questions.
Also worked locally on my machine.
(linux with phpstorm)

.helm/ecamp3/templates/api_deployment.yaml Show resolved Hide resolved
api/Dockerfile Show resolved Hide resolved
api/Dockerfile Show resolved Hide resolved

RUN set -eux; \
install-php-extensions \
@composer \
Copy link
Contributor

Choose a reason for hiding this comment

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

the version of composer is not explicit anymore.
ok for me

docker-compose.yml Show resolved Hide resolved
COPY --link docker/php/docker-healthcheck.sh /usr/local/bin/docker-healthcheck
RUN chmod +x /usr/local/bin/docker-healthcheck
# Dev FrankenPHP image
FROM frankenphp_base AS frankenphp_dev
Copy link
Contributor

Choose a reason for hiding this comment

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

ah, here is the frankenphp_dev branch

api/Dockerfile Show resolved Hide resolved
@usu usu requested a review from pmattmann May 25, 2024 05:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy! Creates a feature branch deployment for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Synchronize changes from api-platform template (aka switch to FrankenPHP)
2 participants