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

Fix database cleanup step in pr-check #101

Merged
merged 3 commits into from
Jun 21, 2022
Merged

Fix database cleanup step in pr-check #101

merged 3 commits into from
Jun 21, 2022

Conversation

zijchen
Copy link
Contributor

@zijchen zijchen commented Jun 15, 2022

In #99 I forgot to pass in the cleanup script to the sed command to replace the database name with the correct one, resulting in an empty script file ran during the cleanup step.

@zijchen zijchen temporarily deployed to Automation test June 15, 2022 23:10 Inactive
@zijchen zijchen temporarily deployed to Automation test June 15, 2022 23:22 Inactive
@zijchen zijchen temporarily deployed to Automation test June 15, 2022 23:35 Inactive
@zijchen zijchen temporarily deployed to Automation test June 15, 2022 23:35 Inactive
@zijchen zijchen temporarily deployed to Automation test June 15, 2022 23:35 Inactive
@zijchen zijchen temporarily deployed to Automation test June 15, 2022 23:35 Inactive
@zijchen zijchen changed the title [Ignore] Test Fix database cleanup step in pr-check Jun 15, 2022
@zijchen zijchen marked this pull request as ready for review June 15, 2022 23:38
@zijchen zijchen requested review from dzsquared and yorek as code owners June 15, 2022 23:38
@zijchen zijchen requested review from kisantia, Benjin and ssreerama June 15, 2022 23:38
@zijchen zijchen merged commit e87bd5b into v2 Jun 21, 2022
@zijchen zijchen deleted the zijchen/fix-cleanup branch June 21, 2022 19:36
zijchen added a commit that referenced this pull request Sep 28, 2022
* v2 - Use tedious mssql module instead of sqlcmd (#96)

* Use tedious mssql library instead of sqlcmd

* Fix mssql connection

* Fix SqlUtils tests

* Use config instead of connection string

* Replace conn string builder with mssql config

* Connect to master db

* Restore connection string validation regex

* PR comments, fix error handling

* Update main.js

* Use try catch for error handling

* Fix typo

* v2 - Change script action from sqlcmd to mssql query (#99)

* Change script action from sqlcmd to mssql query

* Update action.yml

* Fully qualify Table1 in sql script

* Add more debug logging

* Clone config before changing db to master

* Cleanup

* Set TEST_DB name before cleanup

* Use runner.temp

* Always cleanup

* PR comments

* Fix database cleanup step in pr-check (#101)

* Debug script contents

* Fix sed command

* Remove debug

* v2 - Add support for AAD password, AAD default, and AAD service principal auth (#100)

* Use tedious mssql library instead of sqlcmd

* Fix mssql connection

* Fix SqlUtils tests

* Use config instead of connection string

* Replace conn string builder with mssql config

* Connect to master db

* Restore connection string validation regex

* AAD auth

* Add support for client and tenant IDs

* Add more debug messaging

* Fix connection string find array

* Make client-id and tenant-id action inputs

* Fix error handling

* More fixes

* Use try catch instead

* Add tests to pr-check.yml

* Change script action from sqlcmd to mssql query

* Update action.yml

* Fully qualify Table1 in sql script

* Add more debug logging

* Clone config before changing db to master

* Cleanup

* Set TEST_DB name before cleanup

* Use runner.temp

* Always cleanup

* Add tests for different auth types

* Mask tenant and client IDs

* Add AAD password test to pr-check.yml

* Fix yaml

* Limit max-parallel to 2

* Add test for service principal

* PR comments

* Fix typo

* Cleanup sqlcmd references (#102)

* Retry connection with user DB if master connection fails (#104)

* Retry with DB connection if master fails

* Add tests

* Add ConnectionResult interface

* Add missing doc comment

* PR comments

* PR Comments

* Download and extract go-sqlcmd before main action (#108)

* Add setup script to download go-sqlcmd

* Add sqlcmd call to pr-check.yml

* Add bz2 specific extract tar

* Move setup code to main

* Move setup code to main

* Fix casing of Setup.ts

* Use go-sqlcmd for script action (#113)

* call go-sqlcmd for script action

* Fix auth options not flowing through

* Add test cases

* Restore sqlcmd variable in cleanup script

* Fix pr-check cleanup

* Undo changes to pr-check.yml

* Undo pr-check.yml changes

* PR comments

* Change inputs (#105)

* Update SQL API version to 2021-11-01 (#117)

* Add support for Script, DeployReport, and DriftReport (#106)

* Change inputs

* Add other publish like actions

* Add tests

* Fix test

* PR comments

* Add mockReturnValue for auth type test

* Remove MacOS from ci.yml

* links to documentation on each argument type (#123)

* v2 - Remove client-id and tenant-id as inputs (#124)

* Set tenantId and clientId only when passed in

* Default to empty string

* Default to empty string

* Replace mssql call with go-sqlcmd query

* Capture sqlcmd error message

* Add more debug

* Capture stdout too

* Fix config passing

* Completely remove client-id and tenant-id

* cleanup

* Update sqlcmd query

* adding connection string format to docs (#138)

adding connection string format to docs

Co-authored-by: Drew Skwiers-Koballa <[email protected]>
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.

4 participants