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

TGR-66: PHP 8 Update #7

Open
wants to merge 89 commits into
base: development
Choose a base branch
from
Open

TGR-66: PHP 8 Update #7

wants to merge 89 commits into from

Conversation

jberube-nypl
Copy link
Collaborator

@jberube-nypl jberube-nypl commented Sep 20, 2024

Updates codebase for compatibility with PHP 8, and changes Lambda type from Node.js to Docker image.

Bref
Previously, this codebase was deployed to Lambda as a Node.JS type, and Node was used to execute a PHP binary to run the PHP code. This PR changes the Lambda type to Docker Image so that the PHP code can be run inside a Docker container without using Node. Because of this, index.js and Gruntfile.js were eliminated. A Dockerfile was added and it loads the bref/php-83-fpm, which is an image that extends the base AWS Docker image and adds a runtime to allow PHP to run in Lambda. AWS recommends using the third-party Bref dependency here:
https://aws.amazon.com/blogs/compute/the-serverless-lamp-stack-part-3-replacing-the-web-server/
and here:
https://aws.amazon.com/blogs/compute/building-php-lambda-functions-with-docker-container-images/

Github Actions
Github Actions have been added at .github/workflows and are replacing Travis, so .travis.yml was deleted.

PHP Microservice Starter
Updates also had to be made to the dependent NYPL library php-microservice-starter. It will now point to the PHP 8 branch of this library "dev-php8-update". PR is here: NYPL/php-microservice-starter#16.
Slim - This framework library needed to be upgraded from v3 to v4 for PHP 8 compatibility. This required many changes to the codebase to implement its API changes. The Aura/DI library was also added since the new version of Slim requires it.

Swagger PHP
Swagger PHP - the dependent library changed its name from Swagger to OpenAPI, so docblock comments have been changed from SWG to OA.

Updated Libraries:
nypl/microservice-starter 1.0 => dev-php8-update
phpunit/phpunit 6.2.1 => 10.5.40
satooshi/php-coveralls 1.0.1 => php-coveralls/php-coveralls => 2.7.0
squizlabs/php_codesniffer 3.0.0 => 3.11.2

Added libraries:
doctrine/annotations => 2.0.2
bref/bref 2.3

@jberube-nypl jberube-nypl changed the title Php8 update PHP 8 Update Sep 20, 2024
@jberube-nypl jberube-nypl changed the title PHP 8 Update TGR-66: PHP 8 Update Dec 23, 2024
@jberube-nypl jberube-nypl requested a review from nonword December 23, 2024 15:22
"description": "Lambda for the NYPL Item Service",
"require": {
"nypl/microservice-starter": "^1.0"
"nypl/microservice-starter": "dev-php8-update",
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed, we can prob make this be v2, right?

~~~~

You can then make a request to the Lambda: `http://localhost:8888/api/v0.1/items`.
You can then make a request to the Lambda at localhost:8000, (e.g. `http://localhost:8000/api/v0.1/items`).

Copy link
Contributor

Choose a reason for hiding this comment

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

🎉

@@ -1,12 +1,10 @@
DB_CONNECT_STRING=pgsql:host=item-service-production.cvy7z512hcjg.us-east-1.rds.amazonaws.com;dbname=item_service_production
DB_USERNAME=itemserviceprod
DB_PASSWORD=
Copy link
Contributor

Choose a reason for hiding this comment

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

Overridden on line 9?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's delete this file since we don't maintain a "development" env any more.

config/qa.env Outdated
@@ -1,12 +1,10 @@
DB_CONNECT_STRING=pgsql:host=item-services-qa.cvy7z512hcjg.us-east-1.rds.amazonaws.com;dbname=item_service_qa
DB_USERNAME=itemserviceprod
DB_PASSWORD=
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't this overridden on line 9?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

* }
* }
* )
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

No @OA\Post comments needed for createItem?

* @param FixedField $fixedField
*/
public function addFixedField($fieldType = '', FixedField $fixedField)
public function addFixedField(string $fieldType, FixedField $fixedField)
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume use of the type declaration implicitly sets default of ''?

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.

2 participants