Skip to content
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

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

mickmis
Copy link
Contributor

@mickmis mickmis commented Jan 19, 2024

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 call delete_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 to Query that attempts parsing the response to a certain type and raises QueryError 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 of monitoring/uss_qualifier/resources/astm/f3548/v21/dss.py (which would fix normally #458).

@mickmis
Copy link
Contributor Author

mickmis commented Jan 19, 2024

@BenjaminPelletier I'm requesting review as the code implemented seems to be working fine, however it seems to introduce an issue with the make format command that hangs forever (executing something in the schemas folder). For this reason I'm leaving it in draft until fixed. I will investigate it more when I'm back, but if you have any idea I welcome them.

@mickmis mickmis force-pushed the error-handling-dss-f3548-delete branch from cf66a60 to 56d6643 Compare January 30, 2024 13:06
@mickmis
Copy link
Contributor Author

mickmis commented Jan 30, 2024

@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?

@mickmis mickmis marked this pull request as ready for review January 30, 2024 13:57
@BenjaminPelletier
Copy link
Member

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.

@mickmis
Copy link
Contributor Author

mickmis commented Feb 2, 2024

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:

Traceback (most recent call last):
  File "/app/schemas/manage_type_schemas.py", line 243, in <module>
    sys.exit(main())
             ^^^^^^
  File "/app/schemas/manage_type_schemas.py", line 158, in main
    _make_type_schemas(TestRunReport, schema_vars_resolver, schemas)
  File "/app/schemas/manage_type_schemas.py", line 60, in _make_type_schemas
    implicitdict.jsonschema.make_json_schema(parent, reference_resolver, repo)
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 69, in make_json_schema
    properties[field], is_optional = _schema_for(value_type, schema_vars_resolver, schema_repository, schema_type)
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 162, in _schema_for
    make_json_schema(value_type, schema_vars_resolver, schema_repository)
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 69, in make_json_schema
    properties[field], is_optional = _schema_for(value_type, schema_vars_resolver, schema_repository, schema_type)
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 143, in _schema_for
    subschema, _ = _schema_for(arg_types[0], schema_vars_resolver, schema_repository, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 162, in _schema_for
    make_json_schema(value_type, schema_vars_resolver, schema_repository)
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 69, in make_json_schema
    properties[field], is_optional = _schema_for(value_type, schema_vars_resolver, schema_repository, schema_type)
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 143, in _schema_for
    subschema, _ = _schema_for(arg_types[0], schema_vars_resolver, schema_repository, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 162, in _schema_for
    make_json_schema(value_type, schema_vars_resolver, schema_repository)
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 69, in make_json_schema
    properties[field], is_optional = _schema_for(value_type, schema_vars_resolver, schema_repository, schema_type)
                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 143, in _schema_for
    subschema, _ = _schema_for(arg_types[0], schema_vars_resolver, schema_repository, context)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 126, in _schema_for
    items_schema, _ = _schema_for(arg_types[0], schema_vars_resolver, schema_repository, context)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 162, in _schema_for
    make_json_schema(value_type, schema_vars_resolver, schema_repository)
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 57, in make_json_schema
    field_docs = _field_docs_for(schema_type)
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/implicitdict/jsonschema.py", line 201, in _field_docs_for
    for m in re.finditer(doc_pattern, src):
KeyboardInterrupt
make[1]: *** [Makefile:3: format] Error 130
make: *** [Makefile:45: json-schema] Interrupt

I will spend 30 minutes trying to find a fix, if I don't I will open up an issue with all this info.
Edit: see interuss/implicitdict#13

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