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

Azure SQL is not supported #95

Closed
rouke-broersma opened this issue Jul 6, 2021 · 6 comments
Closed

Azure SQL is not supported #95

rouke-broersma opened this issue Jul 6, 2021 · 6 comments

Comments

@rouke-broersma
Copy link

rouke-broersma commented Jul 6, 2021

See: https://github.com/EdwinVW/pitstop/blob/master/src/NotificationService/Repositories/SqlServerNotificationRepository.cs#L39

DB_ID always returns NULL for azure sql (single database).

We could instead use

"IF NOT EXISTS(SELECT * FROM master.sys.databases WHERE name='Notification') CREATE DATABASE Notification;"

The USE command is also not allowed on azure sql single database which is used by ChangeDatabase: https://github.com/EdwinVW/pitstop/blob/master/src/NotificationService/Repositories/SqlServerNotificationRepository.cs#L44

We can replace this with closing the sql connection to master and creating a new sql connection to the target database.

This does work on azure sql. Would this change be acceptable?

@rouke-broersma rouke-broersma changed the title IF DB_ID() IS NULL does not work for azure sql Azure SQL is not supported Jul 6, 2021
@EdwinVW
Copy link
Owner

EdwinVW commented Jul 13, 2021

Yes that's fine - providing it also keeps working on SQL Server (which I assume is the case).

@EdwinVW
Copy link
Owner

EdwinVW commented Jul 13, 2021

Fixed in commit 9383def
Successfully tested these changes on SQL Server.

@EdwinVW
Copy link
Owner

EdwinVW commented Jul 13, 2021

Can you check whether this is sufficient to fix the issue?

@rouke-broersma
Copy link
Author

This is the same change we made to our fork 👍

The only difference is that we did a conn.OpenAsync(); in the new connection scope but if you say the change works fine without it on SQL server then I guess it's not neccesary? 🤷

@EdwinVW
Copy link
Owner

EdwinVW commented Jul 13, 2021

Yes, it makes sense to explicitly open the connection. I'll create a fix for that.

@EdwinVW
Copy link
Owner

EdwinVW commented Jul 13, 2021

See commit f243358

I've also changed the SqlServerWorkshopPlanningEventSourceRepository of the WorkshopManagementAPI so its structure is identical to that of the other two repos.

@EdwinVW EdwinVW closed this as completed Jul 13, 2021
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

No branches or pull requests

2 participants