-
Notifications
You must be signed in to change notification settings - Fork 46
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
editoast: add stdcm search environment model and endpoints #8257
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## dev #8257 +/- ##
============================================
+ Coverage 27.18% 27.48% +0.29%
- Complexity 2127 2155 +28
============================================
Files 1310 1316 +6
Lines 157758 158158 +400
Branches 3256 3262 +6
============================================
+ Hits 42887 43467 +580
+ Misses 112891 112708 -183
- Partials 1980 1983 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
b9c2767
to
cc387d6
Compare
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.
I'm not sure that I understand completely how it will be used. In the description of the migration, it's mentioned that the object will almost be a singleton. And most of the code does seems to reflect that. But the name of the function retrieve_latest
almost gives you a hint that there might be other instances of the object ("if there is a latest, it's relative to others"). It might just be a naming problem.
Apart from that, very clean PR with lot of tests, thank you 🎉
@woshilapin when designing this implem with @flomonster we considered making the sql table an actual singleton. So we opted to not enforce the singleton-ness. Thus, if by some manual manipulation, the database has several rows, the "retrieve" function handles it gracefully. What do you think ? |
From the original goal of https://github.com/osrd-project/osrd-confidential/issues/481, don't we want to support multiple environments and select the correct one based on the requested date ? This seems to override all the envs every time a POST is made |
@Khoyo with @flomonster we discussed that there actually probably wasn't a need to keep history. We could decide to actually keep history and add indexes on the time windows fields. However, given that there are no "ON DELETE" clauses on the fields (so the env can't be accidentally deleted), all objects referenced by the history wouldn't be deletable. |
3ee8b31
to
da38382
Compare
close #8168