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

Base php 7.4 #1812

Merged
merged 7 commits into from
May 20, 2024
Merged

Base php 7.4 #1812

merged 7 commits into from
May 20, 2024

Conversation

megahirt
Copy link
Collaborator

@megahirt megahirt commented May 16, 2024

This is a PR to update the base-php image to 7.4. We will not update to future PHP versions, so it makes sense to just name this base-php

We also add the mongodb-org-tools apt package to the base-php image for use in the backup/restore planned task.

Once this is merged, I can manually create a new base-php image via GHA.

TODO:

  • - figure out what debian package has mongoexport in it - which version of debian are we running?
  • - make build-base-php locally
  • - make locally
  • - run PHP tests locally
  • - confirm that mongoexport is available in the running app container

PHP tests complete successfully on my local machine, but will fail in this PR build since base-php hasn't been built properly until this is merged.
image

we will no longer attempt further PHP upgrades.  Staying at PHP 7.4
@megahirt megahirt added the engineering Tasks which do not directly relate to a user-facing feature or fix label May 16, 2024
@megahirt megahirt self-assigned this May 16, 2024
Copy link

github-actions bot commented May 16, 2024

Unit Test Results

362 tests   362 ✅  13s ⏱️
 37 suites    0 💤
  1 files      0 ❌

Results for commit a62b4f9.

♻️ This comment has been updated with latest results.

mongodb-org-tools is required for the backup/restore project command being added to the PHP application
@megahirt
Copy link
Collaborator Author

This PR tests are not expected to pass because we are changing back to the base-php tag from the current base-php-7.4 tag. This has the effect that unit tests are now running against the old base-php image which is PHP 7.3 and fail. Once this PR is merged, I can manually push a new base-php image that is updated to 7.4 and then tests will begin passing again.

- move composer install into base-php image
- xdebug 3.1 in both test and development app image
docker/app/Dockerfile Show resolved Hide resolved
docker/app/Dockerfile Show resolved Hide resolved
docker/base-php/Dockerfile Show resolved Hide resolved
docker/test-php/Dockerfile Show resolved Hide resolved
@megahirt megahirt requested a review from rmunn May 20, 2024 08:47
Copy link
Collaborator

@rmunn rmunn left a comment

Choose a reason for hiding this comment

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

LGTM. One concern about the composer extension, but I don't think that should delay getting this merged into develop; we can back that one change out later if needed.

docker/base-php/Dockerfile Show resolved Hide resolved
mongodb doesn't install on arm64 images of this type
docker/base-php/Dockerfile Outdated Show resolved Hide resolved
Co-authored-by: Robin Munn <rmunn@pobox.com>
@megahirt megahirt merged commit 0194f27 into develop May 20, 2024
17 checks passed
@megahirt megahirt deleted the base-php-7.4 branch May 20, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
engineering Tasks which do not directly relate to a user-facing feature or fix
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants