-
Notifications
You must be signed in to change notification settings - Fork 20
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
[uss_qualifier/resources/f3548/dss/delete_op_intent] Use QueryError for error handling #483
[uss_qualifier/resources/f3548/dss/delete_op_intent] Use QueryError for error handling #483
Conversation
@BenjaminPelletier I'm requesting review as the code implemented seems to be working fine, however it seems to introduce an issue with the |
…or error handling
cf66a60
to
56d6643
Compare
@BenjaminPelletier found the (strange) hanging issue I mentioned above. It is in the call to finditer in the implicitdict package. Somehow it hanged because in the docstring I had: """
Raises:
QueryError: if the parsing failed.
""" Changing to the following fixed the issue: """
Raises:
* QueryError: if the parsing failed.
""" I'm not sure what to do of this issue, this may happen again in the future, but maybe it is just a rare corner-case. Should we do something about it or just ignore it? |
ImplicitDict is far enough down the technology stack that it having a problem could cause some pretty serious headaches so I'd like to troubleshoot if we can. Unfortunately I couldn't reproduce the issue on this branch; it would probably be worth 15-30 minutes of you trying similarly to reproduce if you don't mind. I'm happy to fix if we can reproduce. |
I can reproduce locally by doing the following: git checkout 56d6643d7b7e8048842ca679336f7640a81e2480
make json-schema Then the following command will hang: + docker run --name type_schema_manager --rm -u 1000:1000 -v /home/misbach/Development/OrbitalizeIdeaProject/interuss/monitoring:/app interuss/monitoring python /app/schemas/manage_type_schemas.py --generate A Ctrl+C will output the following traceback:
I will spend 30 minutes trying to find a fix, if I don't I will open up an issue with all this info. |
This is a proposal for addressing #458 (but this PR does not actually address it).
The idea is to raise
QueryError
if anything in the calldelete_op_intent
fails, including the parsing. This allows passing the reason for errors up to the scenarios, while not having too much boilerplate in the scenarios.It does so by adding a
parse_json_result
function toQuery
that attempts parsing the response to a certain type and raisesQueryError
if the parsing fails. This exception is then caught in the scenario's implementation, also enabling flows where we expect some operations to fail.Actually that is probably the expected use of this
QueryError
, but I seem to have at least partially missed that until now.This PR implements this approach just for a single function (
delete_op_intent
), but if that is acceptable the idea is to generalize on all functions ofmonitoring/uss_qualifier/resources/astm/f3548/v21/dss.py
(which would fix normally #458).