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.

Sign up for GitHub

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

Jump to bottom

Upgrade settings to schema 300 #56

Merged
merged 55 commits into from
May 22, 2024
Merged

Upgrade settings to schema 300 #56

merged 55 commits into from
May 22, 2024

Conversation

margrietpalm
Copy link
Contributor

@margrietpalm margrietpalm commented Mar 7, 2024 •
edited
Loading

Schema changes overview: https://nelenschuurmans.sharepoint.com/:x:/s/IT/ETXQ-VUlD5ROozqgewH-FG0BuB-qm-kvmeE_6lWRqvIN6Q?e=1lsmdO

These changes, in combination with those in gridbuilder and modelchecker are tested in https://github.com/nens/schema_300_integration_tests. All integration tests pass, indicating that the model checker gives identical results (where expected) and identical gridadmin files are produced.

Note that a downgrade is not implemented because this is not worth the effort.

margrietpalm and others added 22 commits March 4, 2024 10:54
@margrietpalm
Change aggregation_settings and groundwater
f42d338
@margrietpalm
Upgrade interflow and numerical settings
6a5aab2
@margrietpalm
upgrade simple_infiltration
439432e
@margrietpalm
upgrade vegetation_drag
eaeb2e2
@margrietpalm
Move flooding_threshold to numerical_setting
18914bd
@margrietpalm
Split adding columns and moving data
f110132
@margrietpalm
Start new InitialConditions table
e00f93d
@margrietpalm
Add all new tables, and a dirty trick to pass tests (will investigate)
1757fc8
@margrietpalm
Rename global settings and columns, except for id's because those are…
e8c2477 
… complicated
@margrietpalm
Change settings relationships
7d80b15
@margrietpalm
Remove several columns
81359d0
@margrietpalm
Make all renamed columns nullable and migrate
490d382
@margrietpalm
More nullable related stuff
e278d50
@margrietpalm
Set use_groundwater_flow based on grounwater settings
8f1584d
@margrietpalm
Modify dem file paths
0947a2b
@margrietpalm
Remove output_time_step and minimum_sim_time_step from ModelSettings
4d23db8
@jpprins1
try to fix problem
b1efa4d
@jpprins1
merge
58794c0
@margrietpalm
Clean up the mess, a bit at least
bbe4ebf
@margrietpalm
Fix some mistakes and reorganize migration with eye on re-usability a…
3527b1d 
…nd testability
@margrietpalm
Add tests for migration to 300
7acd19c
@margrietpalm
Implement dem file test
022f432
@margrietpalm margrietpalm changed the title WIP Upgrade settings to schema 300 Upgrade settings to schema 300 Mar 12, 2024
@margrietpalm margrietpalm requested a review from jpprins1 March 12, 2024 12:28
@margrietpalm margrietpalm changed the title Upgrade settings to schema 300 WIP: Upgrade settings to schema 300 Mar 13, 2024
@margrietpalm margrietpalm requested review from caspervdw and elisalle and removed request for jpprins1 April 5, 2024 13:20
caspervdw
caspervdw approved these changes Apr 8, 2024
View reviewed changes
Copy link
Contributor

@caspervdw caspervdw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking good, very readable. I can see you spent effort on getting in exactly right. Some minor remarks, you can merge when you want.

threedi_schema/migrations/versions/0300_upgrade_db_settings.py Outdated Show resolved Hide resolved
threedi_schema/migrations/versions/0300_upgrade_db_settings.py Outdated Show resolved Hide resolved
threedi_schema/migrations/versions/0300_upgrade_db_settings.py Outdated Show resolved Hide resolved
threedi_schema/migrations/versions/0300_upgrade_db_settings.py Outdated Show resolved Hide resolved
threedi_schema/migrations/versions/0300_upgrade_db_settings.py Outdated Show resolved Hide resolved
threedi_schema/migrations/versions/0300_upgrade_db_settings.py Outdated Show resolved Hide resolved
threedi_schema/migrations/versions/0300_upgrade_db_settings.py Outdated Show resolved Hide resolved
threedi_schema/migrations/versions/0300_upgrade_db_settings.py Outdated Show resolved Hide resolved
@caspervdw caspervdw mentioned this pull request Apr 8, 2024
Merged
@elisalle elisalle removed their request for review April 8, 2024 09:24
margrietpalm and others added 19 commits April 8, 2024 11:32
@margrietpalm
Correct down revision
c7e3af5
@margrietpalm
Merge branch 'master' into margriet_46_schema_300
e1093fa
@margrietpalm
Use latest pre-upgrade version in migration tests
2debfb2
@margrietpalm
Add new tables with all columns at once
02e4338
@margrietpalm
Check for None dem_path in modifying dem_path
8f8976b
@margrietpalm
fix pytest config
29ac85e
@margrietpalm
Raise instead of print warning on downgrade
2033e43
@margrietpalm
Add renamed test data
3657b2e
@margrietpalm
Fix typehint in migration
09506a9
@margrietpalm @caspervdw
Apply suggestions from code review
08899b9 
Co-authored-by: Casper van der Wel <casper.vanderwel@nelen-schuurmans.nl>
@margrietpalm
Set version to 222 instead of 300
1c442c0
@margrietpalm
Merge branch 'margriet_46_schema_300' of github.com:nens/threedi-sche…
a582267 
…ma into margriet_46_schema_300
@margrietpalm
Add properties to schema ORM for API compatibility
50b4040
@margrietpalm
Add properties to schema ORM for API compatibility
c0a0b80
@margrietpalm
Merge branch 'margriet_46_schema_300' of github.com:nens/threedi-sche…
c04d65a 
…ma into margriet_46_schema_300
@margrietpalm
fix typo
d0c21ec
@margrietpalm
up dev version
dace609
@margrietpalm
Merge branch 'master' into margriet_46_schema_300
c1ccbf6
@margrietpalm
Write changelog for these changes
00e49d1
@margrietpalm margrietpalm merged commit 1763cdd into master May 22, 2024
6 checks passed
@margrietpalm margrietpalm deleted the margriet_46_schema_300 branch May 22, 2024 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@caspervdw caspervdw caspervdw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@margrietpalm @caspervdw @jpprins1