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

Update mssql driver #6956

Closed
wants to merge 2 commits into from
Closed

Update mssql driver #6956

wants to merge 2 commits into from

Conversation

sapk
Copy link
Member

@sapk sapk commented May 15, 2019

The mssql driver was blocked to an old version from 2016 because of a replace rule.

Changelog: denisenkom/go-mssqldb@e32ca50...master

@lafriks
Copy link
Member

lafriks commented May 15, 2019

Migrations does not seem to work anymore

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 15, 2019
@techknowlogick
Copy link
Member

Looking at some of the logs it seems that during migration tests an earlier migration didn’t empty as there are errors about duplicate table:

Error: mssql: There is already an object named 'access_token' in the database.

@zeripath
Copy link
Contributor

Looks like dropping the database between the migration tests didn't work

@zeripath
Copy link
Contributor

They should be being dropped by:

_, err = db.Exec("DROP DATABASE IF EXISTS gitea")

But this doesn't seem to be happening and it doesn't seem to be complaining about it

@zeripath
Copy link
Contributor

Has some auto transaction support been added? It might be that it's expecting us to commit before it moving on?

@sapk
Copy link
Member Author

sapk commented Jun 27, 2019

I forget that I have done this PR before 🤦‍♂️.
This is now superseded by #7306 and futur work in #7294 to completely remove the replace in go.mod.

@sapk sapk closed this Jun 27, 2019
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants