Skip to content

Conversation

@the-glu
Copy link
Contributor

@the-glu the-glu commented Aug 8, 2025

This PR contrib to #1100 , point 1

It change try_end_flight behavior by ignoring wrong responses when the status code of the query is not 200.

In such cases, the response will have a limited field available. It's up to the caller to handle such cases.

The code base already handle this situation correctly, by ignoring 404 errors when doing wild cleanup: https://github.com/interuss/monitoring/blob/main/monitoring/uss_qualifier/scenarios/flight_planning/test_steps.py#L424

We don't have a rogue-dss implementation / tests, so I tested locally the new behavior with a special mock uss:

  • When returning empty 404 on unknown flights, the test suite pass
  • When always returning 404 on any flight delete, the test suite fail (as wanted, since we want to ensure deletion in such case).

Another test for the new behavior:

image

We can see that the remove valid flight complained about the wrong response and the cleanup part happily ignored the 404 :)

Copy link
Contributor

@mickmis mickmis left a comment

Choose a reason for hiding this comment

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

In client_scd.py we have (and had already before) a different behavior.

Why not reject here the 404 and adapt callers to catch and interpret the 404 as acceptable?
That way both implementations of the interface will behave the same.

@the-glu the-glu force-pushed the fix_1100_p2 branch 2 times, most recently from ce96aa2 to 73cadd2 Compare August 13, 2025 13:02
f"Deletion of {flight_id}",
f"Deletion of flight {flight_id} returned a status of '{resp.flight_plan_status}' ({FlightPlanStatus.Closed} wanted)",
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NB: I removed the if for the status code: it's not possible to get there without a status_code == 200

@the-glu the-glu requested a review from mickmis August 13, 2025 13:09
Copy link
Contributor

@mickmis mickmis left a comment

Choose a reason for hiding this comment

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

LGTM

@mickmis mickmis merged commit 7236a3c into interuss:main Aug 14, 2025
21 checks passed
@mickmis mickmis deleted the fix_1100_p2 branch August 14, 2025 13:36
github-actions bot added a commit that referenced this pull request Aug 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants