osrd icon indicating copy to clipboard operation
osrd copied to clipboard

editoast: update temporary speed limit groups import endpoint

Open Sh099078 opened this issue 1 year ago • 4 comments

fixes https://github.com/OpenRailAssociation/osrd/issues/9399

  • [x] Add an endpoint /infra/{infra_id}/delimited_area that returns the list of track sections inside an area delimited by entry and exit directed locations (i.e. a position on a track and a direction, similar to a light version of a sign with only the relevant positional information).
  • [x] Stop the graph exploration at a specified distance from the entries if an end position is missing. It is considered that and end position is missing if none has been found after exploring the track sections on a specified maximum distance from the entries without having found any exit.
  • [x] Refactorize temporary_speed_limit.rs tests.

Sh099078 avatar Oct 17 '24 22:10 Sh099078

:warning: Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 88.54806% with 56 lines in your changes missing coverage. Please review.

Project coverage is 38.06%. Comparing base (bae2942) to head (79b6e32). Report is 210 commits behind head on dev.

Files with missing lines Patch % Lines
editoast/src/views/infra/delimited_area.rs 88.42% 44 Missing :warning:
editoast/src/views/temporary_speed_limits.rs 92.53% 5 Missing :warning:
front/src/common/api/generatedEditoastApi.ts 66.66% 3 Missing :warning:
...nt/src/applications/stdcm/utils/formatStdcmConf.ts 0.00% 2 Missing :warning:
editoast/src/infra_cache/graph.rs 83.33% 1 Missing :warning:
front/src/applications/stdcm/hooks/useStdcmEnv.tsx 0.00% 1 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@             Coverage Diff              @@
##                dev    #9381      +/-   ##
============================================
- Coverage     39.62%   38.06%   -1.57%     
============================================
  Files          1300      995     -305     
  Lines         99158    91661    -7497     
  Branches       3282     1174    -2108     
============================================
- Hits          39294    34888    -4406     
+ Misses        57931    56319    -1612     
+ Partials       1933      454    -1479     
Flag Coverage Δ
core ?
editoast 73.26% <89.47%> (-0.05%) :arrow_down:
front 20.09% <57.14%> (+9.85%) :arrow_up:
gateway 2.18% <ø> (-0.02%) :arrow_down:
osrdyne 3.28% <ø> (-0.01%) :arrow_down:
railjson_generator 87.49% <ø> (ø)
tests 86.74% <ø> (+0.03%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


🚨 Try these New Features:

codecov-commenter avatar Oct 17 '24 22:10 codecov-commenter

I've just seen the issue. I don't really like the idea of having several endpoints to create the same object.

wdym? there is only /temporary_speed_limit_group

I'd really prefer this to be done on the import side and not in editoast. When importing, do you have information on the "line" "track" and "pk" of the start and end of the temporary speed limit zone?

We have signals and their location, so we could get that, yes. The import side does not have the full track graph with the various corrections (eg. we do not run the kd-tree), and can't run them easily since it lacks track geometry.

Khoyo avatar Oct 25 '24 14:10 Khoyo

I've just seen the issue. I don't really like the idea of having several endpoints to create the same object.

wdym? there is only /temporary_speed_limit_group

I mean that we should keep the CRUD endpoints of an object quite simple and close from it's model. Usualy transformation are done by the user (using utility endpoint or by it's own). Example: Train schedule / Workschedule / Infra / RS...

Since we don't have LRS in OSRD (for now) it's really difficult to import linear objects such as speed limits (all of them). However, all these objects are converted from a start point to an end point into a list of track ranges in the import script and not by editoast. This PR goes against this paradigm, hence my caution.

I'd really prefer this to be done on the import side and not in editoast. When importing, do you have information on the "line" "track" and "pk" of the start and end of the temporary speed limit zone?

We have signals and their location, so we could get that, yes. The import side does not have the full track graph with the various corrections (eg. we do not run the kd-tree), and can't run them easily since it lacks track geometry.

Maybe editoast can help with this, but with a more generic and reusable endpoint :shrug:

flomonster avatar Oct 25 '24 15:10 flomonster

Following discussions with @Khoyo and @flomonster the temporary speed limits group endpoint has been split into two:

  • A temporary speed limit groups creation endpoint (unchanged, accepts an LTV with a list of track sections).
  • A /infra/{infra_id}/delimited_area endpoint that returns the list of track sections inside an area delimited by a list of entries and exits (positions on a track with a direction).

Sh099078 avatar Oct 29 '24 11:10 Sh099078