feat: refactor postgres sample to use slim app #1099

jdpedrie · 2020-04-30T18:37:15Z

No description provided.

cloud_sql/postgres/pdo/README.md

grayside · 2020-05-01T18:13:27Z

cloud_sql/postgres/pdo/tests/VotesTest.php

@@ -0,0 +1,150 @@
+<?php


Oh yeah, got some tests!

Is there a plan to add integration/end-to-end tests in the future? Mocking the database for a database usage sample only gets you so far.

@bshaffer what's the standard course here?

The Cloud SQL DPEs have an OKR around e2e testing for every sample next quarter.

please don't forget this one!!

cloud_sql/postgres/pdo/README.md

kurtisvg

I know very little about PHP, so I'll defer to @shubha-rajan and @grayside to provide a more throughout review. This is OK with me as long as it's consistent with the MySQL and SQLServer samples going forward.

cloud_sql/postgres/pdo/README.md

kurtisvg · 2020-05-05T00:05:16Z

cloud_sql/postgres/pdo/README.md


 ```bash
-$ gcloud app deploy
+$ gcloud beta app deploy


Does this really need to be beta?

I got that from here and given that the app.yaml has a beta_settings section in it, assumed that beta was required in the command.

Looks like this is still a beta setting for flex, but not standard: https://cloud.google.com/sql/docs/mysql/connect-app-engine#flexible

FYI I tested this using gcloud app deploy (no beta flag) for flex and it worked as expected with no discernible differences.

I removed beta_settings and the app failed. But it seems that using beta does not make a difference.

Co-authored-by: Kurtis Van Gent <[email protected]>

shubha-rajan · 2020-05-07T19:15:40Z

@jdpedrie it looks like there's a merge conflict in the README. do you mind fixing that and updating the PR?

cloud_sql/postgres/pdo/src/app.php

shubha-rajan

LGTM since most of this looks similar to the other two samples. PTAL at my comment about the timeout region tag.

cloud_sql/postgres/pdo/README.md

grayside · 2020-05-11T18:15:02Z

cloud_sql/postgres/pdo/README.md


 ```bash
-$ gcloud app deploy
+$ gcloud beta app deploy


Looks like this is still a beta setting for flex, but not standard: https://cloud.google.com/sql/docs/mysql/connect-app-engine#flexible

shubha-rajan · 2020-05-21T05:40:23Z

@GoogleCloudPlatform/php-admins ping for codeowner review

cloud_sql/postgres/pdo/app.yaml

cloud_sql/postgres/pdo/composer.json

cloud_sql/postgres/pdo/phpunit.xml.dist

shubha-rajan · 2020-05-26T20:31:24Z

@jdpedrie just a few more minor changes before we can merge this. PTAL at the review comments

…les into cloudsql/postgres

jdpedrie · 2020-05-27T20:00:16Z

@bshaffer @shubha-rajan done

shubha-rajan · 2020-06-03T01:17:30Z

@bshaffer ping for review

bshaffer

Seems like a lot of dependencies for a microframework, but if that's what is required then it's fine by me.

cloud_sql/postgres/pdo/composer.json

shubha-rajan · 2020-06-16T21:08:39Z

@bshaffer good to merge?

SaketramDurbha · 2020-07-06T17:01:58Z

I've created PR #1130 that depends on the changes made in this pull request. @bshaffer, if you could review the new changes to this PR at your convenience, I would greatly appreciate it.

cloud_sql/postgres/pdo/README.md

cloud_sql/postgres/pdo/src/app.php

cloud_sql/postgres/pdo/README.md

bshaffer · 2020-07-06T21:47:22Z

cloud_sql/postgres/pdo/app.flex.yaml

+# serve a custom PHP front controller (e.g. "serve backend/index.php") or to
+# run a long-running PHP script as a worker process (e.g. "php worker.php").
+#
+# entrypoint: serve index.php


I believe this comment is only valid for app engine standard, and not for app engine flex

I tested this and found some very strange behavior:

entrypoint: serve index.php => entrypoint1: serve index.php - throws error "unexpected attribute entrypoint2"

entrypoint: serve index.php => entrypoint: serve index1.php - does not throw error, application works as expected, even though index1.php doesn't exist.

entrypoint: serve1 index.php => entrypoint: serve index1.php - does not throw error, application works as expected, even though serve1 doesn't exist.

My theory is that this attribute has been deprecated and while is "valid" is no longer being used in App Engine Flex.

Adding @tmatsuo for comment

Just realized I've been using gcloud app deploy instead of gcloud beta app deploy. I'll test everything again with the beta command.

OK, done. The beta command had no effect.

feat: refactor postgres sample to use slim app

c31bd09

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 30, 2020

shubha-rajan requested review from grayside, kurtisvg and shubha-rajan April 30, 2020 22:54

replace MY-DATABASE with INSTANCE-NAME

abd5c73

grayside reviewed May 1, 2020

View reviewed changes

shubha-rajan suggested changes May 4, 2020

View reviewed changes

cloud_sql/postgres/pdo/README.md Outdated Show resolved Hide resolved

kurtisvg approved these changes May 5, 2020

View reviewed changes

jdpedrie and others added 3 commits May 5, 2020 16:21

fix style, use hostname for choosing tcp mode, add GAE standard docs

f003fca

fix copyright and secret manager link

4019b60

Update cloud_sql/postgres/pdo/README.md

76e3ec4

Co-authored-by: Kurtis Van Gent <[email protected]>

shubha-rajan added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. and removed kokoro:run Add this label to force Kokoro to re-run the tests. labels May 7, 2020

jdpedrie added 2 commits May 7, 2020 16:19

fix conflict

a8e2788

Merge branch 'master' into cloudsql/postgres

95e45d2

kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 7, 2020

fix sample gae definition

6dedd10

shubha-rajan added the kokoro:run Add this label to force Kokoro to re-run the tests. label May 7, 2020

kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label May 7, 2020

shubha-rajan reviewed May 7, 2020

View reviewed changes

cloud_sql/postgres/pdo/src/app.php Show resolved Hide resolved

shubha-rajan approved these changes May 7, 2020

View reviewed changes

grayside approved these changes May 11, 2020

View reviewed changes

shubha-rajan requested a review from bshaffer May 13, 2020 16:39

address review

c741d01

jdpedrie requested a review from a team as a code owner May 14, 2020 15:13

shubha-rajan added the kokoro:run Add this label to force Kokoro to re-run the tests. label May 14, 2020

kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label May 14, 2020

bshaffer requested changes May 21, 2020

View reviewed changes

cloud_sql/postgres/pdo/app.yaml Show resolved Hide resolved

cloud_sql/postgres/pdo/composer.json Show resolved Hide resolved

cloud_sql/postgres/pdo/phpunit.xml.dist Show resolved Hide resolved

jdpedrie added 2 commits May 27, 2020 15:58

Merge branch 'cloudsql/postgres' of github.com:jdpedrie/php-docs-samp…

1b728f5

…les into cloudsql/postgres

address review comments

d52148d

shubha-rajan requested a review from bshaffer May 28, 2020 00:02

shubha-rajan added the kokoro:run Add this label to force Kokoro to re-run the tests. label May 28, 2020

kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label May 28, 2020

bshaffer reviewed Jun 9, 2020

View reviewed changes

cloud_sql/postgres/pdo/composer.json Outdated Show resolved Hide resolved

remove guzzle

56325a1

shubha-rajan requested a review from bshaffer June 10, 2020 16:56

shubha-rajan added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 15, 2020

kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jun 15, 2020

SaketramDurbha mentioned this pull request Jul 6, 2020

update CloudSQL samples to reflect updated TCP and Unix Socket connection methods #1130

Merged

kurtisvg added cla: yes This human has signed the Contributor License Agreement. and removed cla: yes This human has signed the Contributor License Agreement. labels Jul 6, 2020

bshaffer requested changes Jul 6, 2020

View reviewed changes

shubha-rajan added 3 commits July 6, 2020 15:06

Address review comments in README.md

6f48414

Remove comment in app.flex.yaml

4c52fea

Update README.md

0ea3794

shubha-rajan requested a review from bshaffer July 6, 2020 22:20

bshaffer approved these changes Jul 6, 2020

View reviewed changes

kurtisvg added the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 6, 2020

kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Jul 6, 2020

shubha-rajan merged commit 39a13e2 into GoogleCloudPlatform:master Jul 6, 2020

jdpedrie deleted the cloudsql/postgres branch July 7, 2020 00:20

feat: refactor postgres sample to use slim app #1099

feat: refactor postgres sample to use slim app #1099

Uh oh!

Conversation

jdpedrie commented Apr 30, 2020

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kurtisvg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubha-rajan commented May 7, 2020

Uh oh!

Uh oh!

shubha-rajan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shubha-rajan commented May 21, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shubha-rajan commented May 26, 2020

Uh oh!

jdpedrie commented May 27, 2020

Uh oh!

shubha-rajan commented Jun 3, 2020

Uh oh!

bshaffer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shubha-rajan commented Jun 16, 2020

Uh oh!

SaketramDurbha commented Jul 6, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone