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

[1.x] Adds Microsoft SQL Server #717

Closed
wants to merge 1 commit into from

Conversation

DarkGhostHunter
Copy link

@DarkGhostHunter DarkGhostHunter commented Aug 16, 2024

What?

This brings Microsoft SQL Server (their latest version) to Sail. I did it because I usually deal with clients using Azure ootb, and MSSQL it's very picky.

Because the Docker Container is very non-standard, the whole setup is done by setting a custom entrypoint. If no command is issued, the script initializes the databases (default and testing) and then starts SQL Server again but through exec.

EULA shenanigans

The installation is done by manually accepting Microsoft SQL Server EULA, which is found on their Docker Hub. This is both required to install the database engine and the PHP extensions to connect. If the EULA is not accepted, the installation fails.

This sounds like a hurdle, but the correct thing to do is let the developer accept the EULA.

What adds and changes

  • Adds sqlsrv service.
  • Adds the DB_ROOT_PASWORD with a complex password (required by SQL Server)
  • All Dockerfile have an additional block to install sqlsrv and pdo_sqlsrv only if the EULA is accepted (by default, nope).
  • Extensions version are fixed. This is because latest versions drop support for old PHP versions.

Pending work

  • The Sail Server (laravel.build) should add slqsrv to the accepted services array.
  • The documentation (laravel.com/docs/sail) should mention sqlsrv support with the following caveats:
    • Microsoft SQL Server is a x64-only container.
    • Requires EULA Acceptance
    • Adds a separate "root" strong password due to default SQL Server policies.

Copy link

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

@DarkGhostHunter DarkGhostHunter force-pushed the feat/sqlsrv branch 5 times, most recently from 9574579 to 73ca5be Compare August 16, 2024 20:44
@DarkGhostHunter DarkGhostHunter marked this pull request as ready for review August 16, 2024 20:56
@DarkGhostHunter
Copy link
Author

Thanks for submitting a PR!

Note that draft PR's are not reviewed. If you would like a review, please mark your pull request as ready for review in the GitHub user interface.

Pull requests that are abandoned in draft may be closed due to inactivity.

Thanks for noting that, dear bot. The PR is ready for review, so this won't get abandoned.

@DarkGhostHunter DarkGhostHunter force-pushed the feat/sqlsrv branch 3 times, most recently from 321f30b to 637c0c2 Compare August 17, 2024 23:11
@taylorotwell
Copy link
Member

Thanks for your pull request to Laravel!

Unfortunately, I'm going to delay merging this code for now. To preserve our ability to adequately maintain the framework, we need to be very careful regarding the amount of code we include.

If applicable, please consider releasing your code as a package so that the community can still take advantage of your contributions!

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