Skip to content

feat(cloud-sql): Add connection sample for SQL Server/node-mssql #1745

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
May 5, 2020
Merged

feat(cloud-sql): Add connection sample for SQL Server/node-mssql #1745

merged 12 commits into from
May 5, 2020

Conversation

@shubha-rajan
Copy link
Contributor

@shubha-rajan shubha-rajan commented Apr 26, 2020

Fixes #1589
@shubha-rajan shubha-rajan requested a review from kurtisvg as a code owner April 26, 2020 21:43
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 26, 2020
@fhinkel
Copy link
Contributor

fhinkel commented Apr 27, 2020

Thanks, can you add the kokoro config as well please
@shubha-rajan shubha-rajan changed the title [cloud-sql] - Add connection sample for SQL Server/node-mssql feat(cloud-sql): Add connection sample for SQL Server/node-mssql Apr 27, 2020
kurtisvg
kurtisvg suggested changes Apr 27, 2020
View reviewed changes
@shubha-rajan shubha-rajan requested a review from kurtisvg April 27, 2020 23:30
@kurtisvg
Copy link
Contributor

kurtisvg commented Apr 28, 2020

@shubha-rajan LGTM - let me know if you need me to take a look at the kokoro config.
@shubha-rajan shubha-rajan requested a review from fhinkel as a code owner April 28, 2020 19:39
@shubha-rajan
Copy link
Contributor Author

shubha-rajan commented Apr 28, 2020

@fhinkel I added the kokoro config and updated build.sh, but I'm not sure how to create a new SQL server instance to test the sample against or where to add the instance connection name to the stored secrets (looks like all the secrets are read from a storage bucket?). Can you take a look please?
@fhinkel fhinkel added the kokoro:run Add this label to force Kokoro to re-run the tests. label May 1, 2020
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label May 1, 2020
@fhinkel
Copy link
Contributor

fhinkel commented May 1, 2020

Let's wait until the CL is merged so we see if the new CI test is run and passes. Thanks!
@shubha-rajan shubha-rajan added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 1, 2020
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 1, 2020
@shubha-rajan
Copy link
Contributor Author

shubha-rajan commented May 1, 2020

The new test passes, and the 1 failing test is unrelated to this PR and not required, so I'm going to merge.
@shubha-rajan
Copy link
Contributor Author

shubha-rajan commented May 1, 2020 •
edited
Loading

Wait, after checking the logs, it looks like the test errors out, but still shows up as a success. I'll investigate this.
image

Edit: This is an issue with other samples that connect to Cloud SQL too: https://source.cloud.google.com/results/invocations/f8c1a2e6-2247-478f-a4e1-0200c5f0f80c/targets/cloud-devrel%2Fnodejs-docs-samples%2Fpresubmit%2Fappengine%2Fnode8%2Fcloudsql_postgresql/log
@shubha-rajan shubha-rajan mentioned this pull request May 4, 2020
Closed
@shubha-rajan shubha-rajan force-pushed the sqlserver-sample branch 2 times, most recently from 75e4be1 to 2be6488 Compare May 4, 2020 21:17
@shubha-rajan shubha-rajan force-pushed the sqlserver-sample branch 2 times, most recently from 8da6874 to 9794b28 Compare May 5, 2020 17:37
@shubha-rajan shubha-rajan mentioned this pull request May 5, 2020
Merged
@shubha-rajan
Copy link
Contributor Author

shubha-rajan commented May 5, 2020

Merging this now, will resolve test failures in #1783
@shubha-rajan shubha-rajan merged commit 74ab5d7 into GoogleCloudPlatform:master May 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

5 participants

@shubha-rajan @fhinkel @kurtisvg @googlebot @kokoro-team