contributing-docs icon indicating copy to clipboard operation
contributing-docs copied to clipboard

Add special note for EDD and EF

Open rkac-bw opened this issue 1 year ago â€ĸ 5 comments

đŸŽŸī¸ Tracking

📔 Objective

Add specific info in regards to implementing EDD process for Entity framework Only Databases with Code Migrations (Self host for example)

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

đŸĻŽ Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or â„šī¸ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or âš ī¸ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or â™ģī¸ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

rkac-bw avatar Jul 29 '24 14:07 rkac-bw

Logo Checkmarx One – Scan Summary & Details – 99c56dcc-ce4c-4f12-b2c1-b42937e06d81

No New Or Fixed Issues Found

github-actions[bot] avatar Jul 29 '24 14:07 github-actions[bot]

Is this needed? To my knowledge we don't use EF migrations for cloud and unified has no requirement for EDD.

Hinton avatar Jul 29 '24 14:07 Hinton

Is this needed? To my knowledge we don't use EF migrations for cloud and unified has no requirement for EDD.

EF migrations are used for SM in the cloud right? I'd rather be consistent in our approach nonetheless though. It would be ideal to see a PR that includes EDD-prescribed changes both in a traditional DbUp migration script as well as an EF one.

withinfocus avatar Jul 29 '24 17:07 withinfocus

No, SM uses regular migrations since using the EF framework for half our tables and hand written for the rest would be very difficult get running and even harder to follow.

For the delete scenario we should be fine having EF migrations that immediately delete the value. In cloud this means adding a migration for the next release to delete the value from the DB. If the value is required we make it not-required in the current release. This does mean we lose the ability to roll back using EF migrations, but that shouldn't occur on self-hosted instances.

There are more complex scenarios like column rename that requires some juggling with EF. And requires 3 deployments + 1 data migration.

  1. Add column new, update code to write both old and new. <- DB & code deploy.
  2. Run DB migration copying all old to new. <- DB
  3. Removing code references to old. <- Code
  4. Remove old from database. <- DB

Hinton avatar Jul 29 '24 20:07 Hinton

Is there a consensus here? I need to drop some columns currently - which approach should I be using?

Regarding the PR itself - it would be useful to have a timeline showing both database frameworks side-by-side. e.g.


Example: dropping a column

Step # C# Entity MSSQL EF
1. No change Give the parameter a default value in all sprocs No change
Release
2. Delete the property No change Configure the column as a shadow property (no migration)
Release
3. No change Drop the column and remove corresponding sproc parameters (migration) Delete the shadow property (migration)

Maybe there is a better way to represent this.

eliykat avatar Aug 08 '24 04:08 eliykat

As discussed we're got gonna EDD EF at this time.

withinfocus avatar Nov 13 '24 17:11 withinfocus