-
Notifications
You must be signed in to change notification settings - Fork 19
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
Revive old migrations PR #165
Open
Hinton
wants to merge
22
commits into
main
Choose a base branch
from
ps/migrations
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 21 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
0ff84eb
Revive old migrations PR
Hinton a75bd7a
Add todos
Hinton dd5c885
Fix spelling
Hinton 9105949
Fix links
Hinton b139920
Fix another links
Hinton 1ae24e1
Initial pass at updating our EDD database change processes
joseph-flinn a7db292
Fix file name of new image
joseph-flinn e0cd9d8
Switch from 'rerunnable' to 'repeatable'
joseph-flinn 9df35ad
Push the quick fixes from feedback
joseph-flinn 09e4213
Removed repeated use of Fowler's name as well as the repeated use of EDD
joseph-flinn 4717bea
Fix the image caption
joseph-flinn e00c9a8
Removing all added personal pronouns
joseph-flinn 2a11473
Use markdown text styling
joseph-flinn a37c363
Rename the application code version in the Phase definitions to be mo…
joseph-flinn 9711909
Merge branch 'update-db-migrations-docs' of github.com:bitwarden/cont…
Hinton f02a3b2
Resolve todo
Hinton 8f1b733
Merge branch 'master' of github.com:bitwarden/contributing-docs into …
Hinton a1e9518
Merge branch 'main' into ps/migrations
Hinton d6ba026
Update the PR
Hinton 7bad86a
Merge branch 'main' into ps/migrations
djsmith85 e91c759
Merge branch 'main' into ps/migrations
Hinton 188f126
Merge branch 'main' of github.com:bitwarden/contributing-docs into ps…
Hinton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,6 +52,7 @@ SDLC | |
Serilog | ||
signtool | ||
signup | ||
sprocs | ||
sqlcmd | ||
subprocessor | ||
toolset | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
label: "Database Migrations" | ||
position: 2 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
--- | ||
sidebar_position: 2 | ||
--- | ||
|
||
# Entity Framework | ||
|
||
If you alter the database schema, you must create an EF migration script to ensure that EF databases | ||
keep pace with these changes. Developers must do this and include the migrations with their PR. | ||
|
||
To create these scripts, you must first update your data model in `Core/Entities` as desired. This | ||
will be used to generate the migrations for each of our EF targets. | ||
|
||
Once the model is updated, navigate to the `dev` directory in the `server` repo and execute the | ||
`ef_migrate.ps1` PowerShell command. You should provide a name for the migration as the only | ||
parameter: | ||
|
||
```bash | ||
pwsh ef_migrate.ps1 [NAME_OF_MIGRATION] | ||
``` | ||
|
||
This will generate the migrations, which should then be included in your PR. |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,104 @@ | ||
--- | ||
sidebar_position: 1 | ||
--- | ||
|
||
# MSSQL | ||
|
||
:::info | ||
|
||
For instructions on how to apply database migrations, please refer to the | ||
[Getting Started](../../getting-started/server/database/mssql/index.md) documentation. | ||
|
||
::: | ||
|
||
## SQL database project | ||
|
||
We use a [SDK-style SQL project][MSBuildSQL] (`sqlproj`) to develop the database locally. This means | ||
we have an up-to-date representation of the database in `src/Sql`, and any modifications needs to be | ||
represented there as well. Since SDK-style SQL projects are still in preview the tooling is not yet | ||
available Visual Studio. However it is available in [Visual Studio Code][vscode] and [Azure Data | ||
Studio][azureds] with the [SQL Database Projects][SDPE] extension, which provides schema comparison | ||
and more. You may also modify the `.sql` files directly with any text editor. | ||
|
||
To make a database change, start by modifying the `.sql` files in `src/Sql/dbo`. These changes will | ||
also need to be applied in a migration script. Migration scripts are located in | ||
`util/Migrator/DbScripts`. | ||
|
||
You can either generate the migration scripts automatically using the _Schema Comparison_ | ||
functionality or by manually writing them. Do note that the automatic method will only take you so | ||
far and it will need to be manually edited to adhere to the code styles. | ||
|
||
## Modifying the database | ||
|
||
Since we follow [Evolutionary Database Design _(EDD)_](./edd.mdx), any migration that modifies | ||
existing columns most likely needs to be split into at least two parts: a backwards compatible | ||
transition phase, and a non-backwards compatible phase. | ||
|
||
### Best practices | ||
|
||
When writing a migration script there are a couple of best practices we follow. Please check the | ||
[T-SQL Code Style][code-style-sql] for more details. But the most important aspect is ensuring the | ||
script can be re-run on the database multiple times without producing any errors or data loss. | ||
|
||
### Backwards compatible | ||
|
||
Since we follow _EDD_ the first migration needs to retain backwards compatibility with existing | ||
production code. | ||
|
||
1. Modify the source `.sql` files in `src/Sql/dbo`. | ||
2. Write a migration script, and place it in `util/Migrator/DbScripts`. Each script _must_ be | ||
prefixed with the current date. | ||
|
||
Please take care to ensure any existing _Stored Procedure_ accepts the same input parameters which | ||
ensures backwards compatibility. In the case a column is renamed, moved care needs to be taken to | ||
ensure the existing sprocs first checks the new location before falling back to the old location. We | ||
also need to ensure we continue updating the old data columns, since in case a rollback is necessary | ||
no data should be lost. | ||
|
||
### Data migration | ||
|
||
We now need to write a script that migrates any data from the old location to the new locations. | ||
This script should ideally be written in a way that supports batching, i.e. execute for X number of | ||
rows at a time. This helps avoiding locking the database. When running the scripts against the | ||
server please keep running it until it affects `0 rows`. | ||
|
||
### Non-backwards compatible | ||
|
||
These changes should be written from the perspective of "all data has been migrated". And any old | ||
_Stored Procedures_ that were kept around for backwards compatibility should be removed. Any logic | ||
for syncing old and new data should also be removed in this step. | ||
|
||
Since `Sql/dbo` represents the current state we need to introduce a "future" state which we will | ||
call `dbo_finalization`. | ||
|
||
1. Copy the relevant `.sql` files from `src/Sql/dbo` to `src/Sql/dbo_finalization`. | ||
2. Remove the backwards compatibility which is no longer needed. | ||
3. Write a new Migration and place it in `src/Migrator/DbScripts_finalization`, name it | ||
`YYYY-0M-FinalizationMigration.sql`. | ||
- Typically migrations are designed to be run in sequence. However since the migrations in | ||
DbScripts_future can be run out of order, care must be taken to ensure they remain compatible | ||
with the changes to DbScripts. In order to achieve this we only keep a single migration, which | ||
executes all backwards incompatible schema changes. | ||
|
||
### [Not Yet Implemented] Manual MSSQL migrations | ||
|
||
There may be a need for a migration to be run outside of our normal update process. These types of | ||
migrations should be saved for very exceptional purposes. One such reason could be an Index rebuild. | ||
|
||
1. Write a new Migration with a prefixed current date and place it in | ||
`src/Migrator/DbScripts_manual` | ||
2. After it has been run against our Cloud environments and we are satisfied with the outcome, | ||
create a PR to move it to `DbScripts`. This will enable it to be run by our Migrator processes in | ||
self-host and clean installs of both cloud and self-host environments | ||
|
||
[repository]: | ||
https://docs.microsoft.com/en-us/dotnet/architecture/microservices/microservice-ddd-cqrs-patterns/infrastructure-persistence-layer-design | ||
[dapper]: https://github.com/DapperLib/Dapper | ||
[code-style-sql]: ../code-style/index.md#t-sql | ||
[MSBuildSQL]: | ||
https://learn.microsoft.com/en-us/sql/azure-data-studio/extensions/sql-database-project-extension-sdk-style-projects?view=sql-server-ver16 | ||
[vscode]: https://code.visualstudio.com/ | ||
[azureds]: | ||
https://docs.microsoft.com/en-us/sql/azure-data-studio/download-azure-data-studio?view=sql-server-ver16 | ||
[SDPE]: | ||
https://docs.microsoft.com/en-us/sql/azure-data-studio/extensions/sql-database-project-extension?view=sql-server-ver16 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to decide if data migrations are destructive or non destructive. I.e. duplicate the data for a while.