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 grate always create Version record when no script to run. #425

Merged
merged 8 commits into from
Feb 20, 2024

Conversation

hoangthanh28
Copy link
Contributor

Hi Erik.
This PR intends to fix the issue with the Version table. See #388
Running with docker image, looks ok so far, see the detail below:

[SqlServer]:

docker network create grate-test &&
docker run -d -e SA_PASSWORD=gs8j4AS7h87jHg \
 -e ACCEPT_EULA=Y -e MSSQL_PID=Express \
 --network grate-test --name db -p 1433:1433 \
mcr.microsoft.com/mssql/server:2019-latest

[grate docker (using my local docker build docker build -f ./installers/docker/Dockerfile -t hoangthanh28/grate:latest . ]

docker run -v ./:/db --network grate-test \
 -e APP_CONNSTRING="Server=db;Database=grate_test;User Id=sa;Password=gs8j4AS7h87jHg;TrustServerCertificate=True"  \
-e VERSION=1.0.0.1 \
hoangthanh28/grate:latest

[logs]
image

[output]

docker run \
    --network grate-test \
    --entrypoint /opt/mssql-tools/bin/sqlcmd \
    mcr.microsoft.com/mssql-tools \
    -S db -U sa -P gs8j4AS7h87jHg -d grate_test -Q "select * from grate.Version;"

image

@erikbra
Copy link
Owner

erikbra commented Feb 12, 2024

Hi, @hoangthanh28 - sorry this has taken so long. Could you please rebase this on main, so that it is easier to look through it again? I had a couple of comments, there might be more, but I've been busy making sure the "run grate as a nuget package" feature could make it

Copy link
Owner

@erikbra erikbra left a comment

Choose a reason for hiding this comment

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

OK

@erikbra
Copy link
Owner

erikbra commented Feb 20, 2024

I rebased this one on main, and cleaned it up. Thanks a lot for your contribution, @hoangthanh28

@erikbra erikbra merged commit f5f42fc into erikbra:main Feb 20, 2024
10 checks passed
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