opera-adt / COMPASS

COregistered Multi-temPorAl Sar Slc
Apache License 2.0
38 stars 18 forks source link

Journal `error_channel` raises errors, which mask subsequent raised errors #129

Open scottstanie opened 1 year ago

scottstanie commented 1 year ago

Right now we have a number of places that do something like

msg = "Some error message"
error_channel.log(msg)
raise ValueError(msg)

but that way journal is set up, anything logged to the error channel or higher (the fatal channel) will raise it's own python error:

( in testjournal.py)

import journal
error_channel = journal.error('test')
error_channel.log("This is a test error message")
raise ValueError("This will never be raised")
$ python testjournal.py
journal: This is a test error message
Traceback (most recent call last):
  File "/Users/staniewi/repos/testjournal.py", line 3, in <module>
    error_channel.log("This is a test error message")
journal.ext.journal.ApplicationError: test: application error

I think we have two options

  1. simply remove the raise .... statements after all our logging with journal to the error channel
  2. if we want the specific python error (like ValueError, OSError, PermisionsError) to be raised, we'll need to change which channel we're logging two

I only bring up 2 as an option because the error raised will always be a journal.ext.journal.ApplicationError

scottstanie commented 1 year ago

Related to this: @gracebato and I noticed that when you run older YAMLs with an updated COMPASS, the workflow fails since the schema has changed (as expected), but the error message is buried in the middle of a long traceback:

$ s1_cslc.py geo_runconfig_20180902_t087_185679_iw1.yaml
journal: Validation fail for s1_cslc_geo runconfig yaml stack-no-lut/runconfigs/geo_runconfig_20180902_t087_185679_iw1.yaml.
Traceback (most recent call last):
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/utils/runconfig.py", line 64, in load_validate_yaml
    yamale.validate(schema, data)
  File "/u/aurora-r0/staniewi/miniconda3/envs/mapping/lib/python3.10/site-packages/yamale/yamale.py", line 43, in validate
    raise YamaleError(results)
yamale.yamale_error.YamaleError: Error validating data 'stack-no-lut/runconfigs/geo_runconfig_20180902_t087_185679_iw1.yaml' with schema '/u/aurora-r0/staniewi/repos/COMPASS/src/compass/schemas/s1_cslc_geo.yaml'
        runconfig.groups.processing.geocoding.output_format: Unexpected element

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/u/aurora-r0/staniewi/miniconda3/envs/mapping/bin/s1_cslc.py", line 8, in <module>
    sys.exit(main())
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/s1_cslc.py", line 49, in main
    run(parser.run_config_path, parser.args.grid_type)
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/s1_cslc.py", line 40, in run
    cfg = GeoRunConfig.load_from_yaml(run_config_path, 's1_cslc_geo')
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/utils/geo_runconfig.py", line 65, in load_from_yaml
    cfg = load_validate_yaml(yaml_path, workflow_name)
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/utils/runconfig.py", line 67, in load_validate_yaml
    error_channel.log(err_str)
journal.ext.journal.ApplicationError: runconfig.load_validate_yaml: application error

The final line, where the most relevant error usually is, doesn't say anything useful in this case, and the first error line from journal only starts to give the hint journal: Validation fail for ...

If we were to change this section where there's a lot of try/except/log/raise: https://github.com/opera-adt/COMPASS/blob/a8252f591babd1878a3c445c80111edabd3df261/src/compass/utils/runconfig.py#L38-L80

to just something like

schema = yamale.make_schema(f'{helpers.WORKFLOW_SCRIPTS_DIR}/schemas/{schema_name}.yaml', parser='ruamel')
data = yamale.make_data(yaml_runconfig, parser='ruamel')
yamale.validate(schema, data) 

Then the error becomes more clear at the bottom:

$ s1_cslc.py stack-no-lut/runconfigs/geo_runconfig_20180902_t087_185679_iw1.yaml
Traceback (most recent call last):
  File "/u/aurora-r0/staniewi/miniconda3/envs/mapping/bin/s1_cslc.py", line 8, in <module>
    sys.exit(main())
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/s1_cslc.py", line 49, in main
    run(parser.run_config_path, parser.args.grid_type)
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/s1_cslc.py", line 40, in run
    cfg = GeoRunConfig.load_from_yaml(run_config_path, 's1_cslc_geo')
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/utils/geo_runconfig.py", line 65, in load_from_yaml
    cfg = load_validate_yaml(yaml_path, workflow_name)
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/utils/runconfig.py", line 51, in load_validate_yaml
    yamale.validate(schema, data)
  File "/u/aurora-r0/staniewi/miniconda3/envs/mapping/lib/python3.10/site-packages/yamale/yamale.py", line 43, in validate
    raise YamaleError(results)
yamale.yamale_error.YamaleError: Error validating data 'stack-no-lut/runconfigs/geo_runconfig_20180902_t087_185679_iw1.yaml' with schema '/u/aurora-r0/staniewi/repos/COMPASS/src/compass/schemas/s1_cslc_geo.yaml'
        runconfig.groups.processing.geocoding.output_format: Unexpected element
scottstanie commented 7 months ago

This is fixable with some config setting for journal where you don't raise an ApplicationError; we should turn this on.

Additionally (requiring an issue in s1reader), this error message is very hard to decipher:

$ s1_cslc.py resorb_runconfig_S1A_IW_SLC__1SDV_20231209T130729_20231209T130756_051578_063A12_FB7C.yaml
/u/aurora-r0/staniewi/repos/s1-reader/src/s1reader/s1_orbit.py:425: UserWarning: No single orbit file was found in the file list provided. Attempting to find set of RESORB files that covers the time period.
  warnings.warn(msg)
Found RESOEB file covering the S1 SAFE frame.
Cannot find a single RESORB file that meets the time frame criteria.
/u/aurora-r0/staniewi/repos/s1-reader/src/s1reader/s1_orbit.py:432: UserWarning: No single orbit file was found in the file list provided.
  warnings.warn(msg)
/u/aurora-r0/staniewi/repos/s1-reader/src/s1reader/s1_orbit.py:348: UserWarning: No orbit file was found for S1A_IW_SLC__1SDV_20231209T130729_20231209T130756_051578_063A12_FB7C.zip from the directory provided: /u/aurora-r0/staniewi/dev/cslc-burst-discont/grace-example/orbits/resorb
  warnings.warn(msg)
journal: No orbit file correlates to safe file: S1A_IW_SLC__1SDV_20231209T130729_20231209T130756_051578_063A12_FB7C.zip
Traceback (most recent call last):
  File "/u/aurora-r0/staniewi/miniconda3/envs/mapping-311/bin/s1_cslc.py", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/s1_cslc.py", line 53, in main
    run(parser.run_config_path, parser.args.grid_type)
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/s1_cslc.py", line 41, in run
    cfg = GeoRunConfig.load_from_yaml(run_config_path, 's1_cslc_geo')
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/utils/geo_runconfig.py", line 97, in load_from_yaml
    bursts = runconfig_to_bursts(sns)
             ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/u/aurora-r0/staniewi/repos/COMPASS/src/compass/utils/runconfig.py", line 193, in runconfig_to_bursts
    error_channel.log(err_str)
journal.ext.journal.ApplicationError: runconfig.correlate_burst_to_orbit: application error
  1. we are giving warnings in s1reader instead of Exceptions- if we really cannot continue processing, the function that finds an orbit file should raise something instead of warning and moving on (since a warning implies "you did something wrong, but i was able to figure out the right thing and recover)
  2. there are 3-4 messages with conflicting info that would be hard to decipher if you haven't worked on the codebase-
    • it says "Found RESOEB file covering the S1 SAFE frame."
    • but then it says "Cannot find a single RESORB file that meets the time frame criteria." or other error-ish messages so we need to make a note to people trying to process with RESORBs that the probably have to provide multiple files to make compass happy and continue on