Repository navigation
Landsat CheckSolarZenith treats a crashed check_solar_zenith_landsat as a pass (check=False) #66
Description
The Landsat CheckSolarZenith task treats a crash of check_solar_zenith_landsat as a pass. It runs the CLI with check=False (landsat_ac/tasks.py:234) and stops only when stdout is exactly invalid (:237), so a non-zero exit with empty stdout returns SOLAR_VALID: True and the pipeline carries on.
The Sentinel task uses check=True (sentinel/mapped_tasks.py:191), and the bash step #12 replaced ran this CLI under set -o errexit (src/scripts/landsat.sh:4 and :59-63 before #12). Both stop on a crash.
Checked at c385746 through PipelineBuilder and cli._run, with hls-utilities v1.11.4's real checkers on PATH and a stand-in downstream task that requires SOLAR_VALID. As a control, an MTL with SUN_ELEVATION = 10 (SZA 80) stops with exit 3, and the Sentinel equivalent does the same. When the checker exits 1 (missing SUN_ELEVATION, truncated or empty MTL, package not importable), Landsat exits 0 and the downstream task runs; Sentinel exits 1 on the equivalent failures. With HLS_LOG_OUTPUT at its default 0, the log shows the command and nothing about the failure; with 1 the traceback appears. Changing only tasks.py:234 to check=True makes every Landsat crash case exit 1 and leaves valid and invalid unchanged. The existing Landsat test (tests/landsat_ac/test_tasks.py:101) covers only the valid case.
I think the likelier trigger is a broken checker reading a good MTL, not a bad MTL; #10 fixed one. hls-utilities v1.11.2, pinned from #7 until #10, opens the MTL with mode 'rU', which raises ValueError on Python 3.13 (what pixi.lock resolved then). The bash step stopped on it. That was before #12, but with v1.11.2 on PATH, today's Landsat pipeline exits 0 on an SZA-80 MTL and the downstream task runs.
Scope: from reading the code, a missing MTL can't reach this task; DownloadGranule (tasks.py:144) and LocalGranule (tasks.py:174) check for it first. I didn't run the container, LaSRC or anything on AWS, so what a high-SZA granule does downstream isn't measured.
Happy to be told check=False was deliberate. Otherwise I can open a PR with the one-line fix and Landsat tests for the invalid and crash cases; it also covers the LaSRC pipeline (lasrc/workflow.py:168 uses the same task). Open PR #63 changes DownloadGranule in landsat_ac/tasks.py but not CheckSolarZenith.
Activity
ceholden commentedon Oct 2, 2026
Hey @Santoshkumarpuppala -- thanks for the detailed report! I think this is correct, and this is a latent bug hiding that would trigger if that CLI ever failed. If the CLI failed because of something like a corrupted MTL file I'd expect downstream steps to fail, but I'll open a PR to fix this as a correctness issue.
In our development queue we have some tickets to modernize
hls-utilitiesby "lift and shift"-ing the code into this repository as its own package. It's very silly to be calling Python code from the command line now that the pipeline itself is running in Python, but we wanted to get parity with the Bash based workflow scripts as a first step. That work will greatly simplify the handling of outcomes from these steps.Out of curiosity, are you a user of HLS data or interested in some of the workflow? We're happy to be developing the HLS data product in the open especially if it can be helpful to others, so wondering if that is related to you finding this.
Best,
Chris
Santoshkumarpuppala commentedon Oct 4, 2026
Thanks for the quick fix in #67, and for adding the crash test.
I'm not an HLS user and haven't worked on the pipeline. I came across it while reviewing public scientific and government repositories for checks that can pass without running what they claim, and
check=Falseon the Landsat solar zenith step was one of them.ceholden commentedon Oct 5, 2026
Thanks for the reply, even if you aren't a user of the data or algorithms I appreciate the extra context. Also thanks again for the issue -- it was a good push to merge the
hls-utilitiesPython code into this repository so we can call the functions directly as part of the modernization and tech debt reduction efforts we're doing 🚀.