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

Merged
merged 9 commits into from
Jun 2, 2024

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
Copy link
Member

@pmattmann pmattmann left a comment

Choose a reason for hiding this comment

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

works on my machine :)

@usu
Copy link
Member Author

usu commented Jun 1, 2024

Just tested on macOS. To quote a famous coder above:

works on my machine :)

, too!

@usu usu added this pull request to the merge queue Jun 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 2, 2024
@usu usu added this pull request to the merge queue Jun 2, 2024
Merged via the queue into ecamp:devel with commit 22fbc8d Jun 2, 2024
31 of 32 checks passed
@usu usu mentioned this pull request Jun 2, 2024
6 tasks
@BacLuc BacLuc mentioned this pull request Jul 10, 2024
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)
3 participants