editoast: explanation of why a simulation is not feasible refactor editoast #9434

hamz2a · 2024-10-23T11:18:54Z

closes #9476
Refactor error handling and method organization

Simplified how STDCMResponse::PathNotFound error is handled.
Reorganized STDCMRequestPayload methods for better structure.
Moved simulation_run_time to SimulationResponse for clarity.

codecov-commenter · 2024-10-23T11:21:09Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.49%. Comparing base (508ac32) to head (0f81762).

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

Additional details and impacted files

@@              Coverage Diff              @@
##                dev    #9434       +/-   ##
=============================================
+ Coverage     39.62%   87.49%   +47.86%     
=============================================
  Files          1300       31     -1269     
  Lines         99167     1535    -97632     
  Branches       3283        0     -3283     
=============================================
- Hits          39297     1343    -37954     
+ Misses        57937      192    -57745     
+ Partials       1933        0     -1933

Flag	Coverage Δ
core	`?`
editoast	`?`
front	`?`
gateway	`?`
osrdyne	`?`
railjson_generator	`87.49% <ø> (ø)`
tests	`?`

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

woshilapin

Thanks for the work, it's going into the right direction I believe. I'm not sure that I understand the first commit ❓Thank you for the separation and description of the other commits.

I believe we can continue on this track of isolating part of the STDCM flow, but there is already enough in the current PR so probably for later.

editoast/src/core/simulation.rs

woshilapin · 2024-10-25T09:23:14Z

editoast/src/core/stdcm.rs

 #[serde(tag = "status", rename_all = "snake_case")]
 // We accepted the difference of memory size taken by variants
 // Since there is only on success and others are error cases
 #[allow(clippy::large_enum_variant)]
-pub enum STDCMResponse {
+pub enum STDCMCoreResponse {


I'm not sure that I understand the renaming. This struct is defined inside a module name core so we already know it's relative to core's logic.

I renamed it to avoid conflicts and as, since we have another STDCMResponse in views, and both are used in the same file.

woshilapin · 2024-10-25T09:29:39Z

editoast/src/views/timetable/stdcm.rs

+        )
+        .await?;
+
+    let simulation_run_time = match virtual_train_sim_result.clone().simulation_run_time() {


I do not believe you need to .clone() here.

Suggested change

let simulation_run_time = match virtual_train_sim_result.clone().simulation_run_time() {

let simulation_run_time = match virtual_train_sim_result.simulation_run_time() {

We use it afterwards in PathNotFoundHandler.

woshilapin · 2024-10-25T09:32:49Z

editoast/src/views/timetable.rs

+    }
+}
+
+fn elapsed_since_time_ms(time: &NaiveDateTime, zero: &DateTime<Utc>) -> u64 {


zero means 0 in my mind. Maybe we can find another name? How about since (because it's in the name of the function)?

I changed it by start_time

woshilapin · 2024-10-25T09:35:29Z

editoast/src/views/timetable.rs

@@ -351,6 +359,29 @@ async fn conflicts(
    Ok(Json(conflict_detection_response.conflicts))
 }

+pub fn filter_core_work_schedule(


Should this function be in an impl WorkSchedule? I'm really asking the question because you might have a better understanding than me if it makes sense or not.

Done. (it was renamed by map_to_core_work_schedule)

Signed-off-by: hamz2a <atrari.hamza@gmail.com>

- Moved utility functions `get_earliest_step_time`, `get_earliest_step_tolerance_window`, `get_total_stop_time`, `get_maximum_departure_delay`, `get_earliest_departure_time` and `get_latest_simulation_end` into the `STDCMRequestPayload` struct as methods. - Updated the `stdcm` function to utilize these new methods for better readability and maintainability. - Removed redundant function definitions and improved code organization. Signed-off-by: hamz2a <atrari.hamza@gmail.com>

Signed-off-by: hamz2a <atrari.hamza@gmail.com>

hamz2a · 2024-10-29T14:09:43Z

Work has been moved into #9509 (just because of some git shenanigans!)

hamz2a requested review from a team as code owners October 23, 2024 11:18

hamz2a requested a review from shenriotpro October 23, 2024 11:18

hamz2a marked this pull request as draft October 23, 2024 11:19

github-actions bot added the area:editoast Work on Editoast Service label Oct 23, 2024

hamz2a requested review from woshilapin and leovalais October 23, 2024 11:19

hamz2a force-pushed the hai/editoast-explanation-of-why-a-simulation-is-not-feasible-refactor branch from 90fd83e to 3fcc0f0 Compare October 23, 2024 12:33

hamz2a changed the base branch from dev to hai/editoast-explanation-of-why-a-simulation-is-not-feasible October 23, 2024 12:49

hamz2a changed the title ~~Hai/editoast explanation of why a simulation is not feasible refactor~~ editoast: explanation of why a simulation is not feasible refactor Oct 23, 2024

hamz2a force-pushed the hai/editoast-explanation-of-why-a-simulation-is-not-feasible branch from 3012e7b to 4ecb4de Compare October 23, 2024 14:40

hamz2a mentioned this pull request Oct 24, 2024

editoast: return conflicts list when STDCM request fails #9327

Merged

hamz2a force-pushed the hai/editoast-explanation-of-why-a-simulation-is-not-feasible branch from 4ecb4de to a19b328 Compare October 24, 2024 10:12

Base automatically changed from hai/editoast-explanation-of-why-a-simulation-is-not-feasible to dev October 24, 2024 10:47

hamz2a force-pushed the hai/editoast-explanation-of-why-a-simulation-is-not-feasible-refactor branch from 3fcc0f0 to 23c11f4 Compare October 25, 2024 08:56

github-actions bot removed the area:editoast Work on Editoast Service label Oct 25, 2024

hamz2a force-pushed the hai/editoast-explanation-of-why-a-simulation-is-not-feasible-refactor branch from 23c11f4 to 217365c Compare October 25, 2024 09:01

github-actions bot added the area:editoast Work on Editoast Service label Oct 25, 2024

woshilapin approved these changes Oct 25, 2024

View reviewed changes

hamz2a force-pushed the hai/editoast-explanation-of-why-a-simulation-is-not-feasible-refactor branch 2 times, most recently from 374a154 to 0eb6401 Compare October 29, 2024 10:33

hamz2a changed the title ~~editoast: explanation of why a simulation is not feasible refactor~~ editoast: refactoring STDCM handler Oct 29, 2024

hamz2a added 6 commits October 29, 2024 12:05

editoast: return conflicts list when STDCM request fails

252aa8d

Signed-off-by: hamz2a <atrari.hamza@gmail.com>

editoast: move simulation_run_time to SimulationResponse

32700a0

Signed-off-by: hamz2a <atrari.hamza@gmail.com>

editoast: refactor path not found handling

06565d3

Signed-off-by: hamz2a <atrari.hamza@gmail.com>

editoast: add STDCMCoreResponse

257f22c

Signed-off-by: hamz2a <atrari.hamza@gmail.com>

editoast: move map_to_core_work_schedule to WorkSchedule

0f81762

Signed-off-by: hamz2a <atrari.hamza@gmail.com>

hamz2a force-pushed the hai/editoast-explanation-of-why-a-simulation-is-not-feasible-refactor branch from 0eb6401 to 0f81762 Compare October 29, 2024 11:05

hamz2a closed this Oct 29, 2024

hamz2a changed the title ~~editoast: refactoring STDCM handler~~ editoast: explanation of why a simulation is not feasible refactor editoast Oct 29, 2024

woshilapin mentioned this pull request Nov 4, 2024

editoast: refactoring STDCM handler #9509

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

editoast: explanation of why a simulation is not feasible refactor editoast #9434

editoast: explanation of why a simulation is not feasible refactor editoast #9434

hamz2a commented Oct 23, 2024 •

edited

Loading

codecov-commenter commented Oct 23, 2024 •

edited

Loading

woshilapin left a comment

woshilapin Oct 25, 2024

hamz2a Oct 29, 2024

woshilapin Oct 25, 2024

hamz2a Oct 29, 2024 •

edited

Loading

woshilapin Oct 25, 2024

hamz2a Oct 29, 2024

hamz2a Oct 29, 2024

woshilapin Oct 25, 2024

hamz2a Oct 29, 2024

hamz2a commented Oct 29, 2024 •

edited by woshilapin

Loading

	let simulation_run_time = match virtual_train_sim_result.clone().simulation_run_time() {
	let simulation_run_time = match virtual_train_sim_result.simulation_run_time() {

editoast: explanation of why a simulation is not feasible refactor editoast #9434

editoast: explanation of why a simulation is not feasible refactor editoast #9434

Conversation

hamz2a commented Oct 23, 2024 • edited Loading

codecov-commenter commented Oct 23, 2024 • edited Loading

Codecov Report

woshilapin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamz2a Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hamz2a commented Oct 29, 2024 • edited by woshilapin Loading

hamz2a commented Oct 23, 2024 •

edited

Loading

codecov-commenter commented Oct 23, 2024 •

edited

Loading

hamz2a Oct 29, 2024 •

edited

Loading

hamz2a commented Oct 29, 2024 •

edited by woshilapin

Loading