Use compath.py to define COM paths instead of hard-coded COMROOT construction#220
Open
Copilot wants to merge 10 commits into
Open
Use compath.py to define COM paths instead of hard-coded COMROOT construction#220Copilot wants to merge 10 commits into
Copilot wants to merge 10 commits into
Conversation
Agent-Logs-Url: https://github.com/NOAA-EMC/obsForge/sessions/45c3ae7a-619d-4226-9de3-1730be6973aa Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NOAA-EMC/obsForge/sessions/45c3ae7a-619d-4226-9de3-1730be6973aa Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Refactor code to use compath instead of hard coded paths
Use compath.py to define COM paths instead of hard-coded COMROOT construction
May 6, 2026
Contributor
CoryMartin-NOAA
left a comment
CoryMartin-NOAA
left a comment
There was a problem hiding this comment.
@copilot this isn't quite right, for JOBSFORGE_GLOBAL_MARINE_DUMP COMIN_OBSPROC should look something more like this: https://github.com/NOAA-EMC/global-workflow/blob/ee5e04eaaba44aed87d2dd4bf559dd1ef360cb79/dev/jobs/JGLOBAL_OFFLINE_ATMOS_ANALYSIS.j2#L36-L37
and we need a COMOUT_OCEAN_OBS defined here (and modified in the python script in the output section ) that looks like this:
export COMOUT_OCEAN_OBS="${ROTDIR}/${RUN}.${PDY}/${cyc}/ocean/insitu"
Basically if you see OBSPROC_COMROOT, that should be replaced by COMIN_OBSPROC and COMIN_OBSPROC should be replaced by COMOUT_OCEAN_OBS
…EAN_OBS Agent-Logs-Url: https://github.com/NOAA-EMC/obsForge/sessions/d75cccdd-ac12-4c58-8dc6-71827e2b319c Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Contributor
Author
Done in commit
|
Agent-Logs-Url: https://github.com/NOAA-EMC/obsForge/sessions/97be0637-cb0c-4830-aa7d-86903a24d239 Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NOAA-EMC/obsForge/sessions/3ea1a7d7-534a-4dcc-b2a1-d750fb9640fa Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Agent-Logs-Url: https://github.com/NOAA-EMC/obsForge/sessions/0932c2a7-a100-48f4-ab86-2d3d260a79cb Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
… _STAGE_OBS Agent-Logs-Url: https://github.com/NOAA-EMC/obsForge/sessions/077ab63f-0718-4e07-a84c-366610a5c4f3 Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
The `script_env` fixture in `test_exobsforge_global_dump.py` only set 4
variables (`cyc`, `current_cycle`, `PDY`, `RUN`), causing `KeyError:
'COMOUT_OBSFORGE_MARINE_OBS'` when `MarineObsPrep.finalize()` ran. In
production these variables are injected by the job scripts; the test had
no equivalent setup.
## Changes
- **`script_env` fixture** — adds the three missing runtime path
variables, each rooted under pytest's `tmp_path`, with directories
pre-created:
```python
env["DCOMROOT"] = str(tmp_path / "dcom")
env["COMOUT_OBSFORGE_MARINE_OBS"] = str(tmp_path / "com" / "marine_obs")
env["COMOUT_OBSFORGE_CHEM_OBS"] = str(tmp_path / "com" / "chem_obs")
Path(env["DCOMROOT"]).mkdir(parents=True, exist_ok=True)
Path(env["COMOUT_OBSFORGE_MARINE_OBS"]).mkdir(parents=True,
exist_ok=True)
Path(env["COMOUT_OBSFORGE_CHEM_OBS"]).mkdir(parents=True, exist_ok=True)
```
`COMOUT_OBSFORGE_CHEM_OBS` covers the AOD script — on the target branch
`AerosolObsPrep.finalize()` reads that key (matching
`JOBSFORGE_GLOBAL_AOD_DUMP`).
- **`test_run_exobsforge_script`** — fixes `create_dcom` to use
`env["DCOMROOT"]` (fixture-controlled) instead of
`os.getenv("DCOMROOT")` (parent process, unset in CI).
<!-- START COPILOT ORIGINAL PROMPT -->
<details>
<summary>Original prompt</summary>
The failing GitHub Actions job for PR #220 in `NOAA-EMC/obsForge` is
caused by a pytest environment setup gap in
`scripts/tests/test_exobsforge_global_dump.py`.
Failure details:
- Workflow run: `25503656413`
- Job: `74864479258`
- Failing test:
`scripts/tests/test_exobsforge_global_dump.py::test_run_exobsforge_script`
- Logs show `KeyError: 'COMOUT_OBSFORGE_MARINE_OBS'` raised when running
`scripts/exobsforge_global_marine_dump.py`.
Relevant context from the failing job:
- The workflow `.github/workflows/build.yaml` runs `pytest scripts/tests
--disable-warnings -v` inside Docker.
- The test fixture `script_env` in
`scripts/tests/test_exobsforge_global_dump.py` currently sets only a
small subset of required environment variables (`cyc`, `current_cycle`,
`PDY`, `RUN`).
- The script `scripts/exobsforge_global_marine_dump.py` builds runtime
config from `os.environ` and invokes `MarineObsPrep.finalize()`, which
expects output path environment variables such as
`COMOUT_OBSFORGE_MARINE_OBS` to be present.
Requested fix:
1. Update `scripts/tests/test_exobsforge_global_dump.py` so the
`script_env` fixture provides the missing output directory environment
variables needed by the scripts under test.
2. Use temporary directories rooted under `tmp_path` for these variables
rather than relying on external environment state.
3. Create any required directories in the fixture before running the
scripts.
4. Keep the change minimal and focused on fixing the failing test.
5. If the AOD script under test requires a similar output variable,
include it as part of the fixture setup as well.
6. After making the change, open a pull request targeting branch
`copilot/use-compath-instead-of-hard-coded-paths`.
Please keep the PR title concise and descriptive.
The following is the prior conversation context from the user's chat
exploration (may be truncated):
User: Please find a solution for this failing job. Use the logs, job
definition, and any referenced files where the failure occurred. Keep
your response focused on the solution and include code suggestions when
appropriate.
Assistant: The failure to fix is the pytest error in
[`scripts/tests/test_exobsforge_global_dump.py`](https://github.com/NOAA-EMC/obsForge/blob/e52c83bb5289edf54655c2e3743ddce912dce664/scripts/tests/test_exobsforge_global_dump.py#L55-L87),
not the repeated `NetCDF: Unknown file format` messages. Those happen
because the test intentionally creates empty `.nc`/`.h5` placeholder
files at lines 47–50, but the job only fails when
`exobsforge_global_marine_dump.py` crashes with:
- `KeyError: 'COMOUT_OBSFORGE_MARINE_OBS'` in
[`scripts/exobsforge_global_marine_dump.py`](https://github.com/NOAA-EMC/obsForge/blob/e52c83bb5289edf54655c2e3743ddce912dce664/scripts/exobsforge_global_marine_dump.py#L15-L35)
## Root cause
The test fixture sets only:
- `cyc`
- `current_cycle`
- `PDY`
- `RUN`
in
[`script_env`](https://github.com/NOAA-EMC/obsForge/blob/e52c83bb5289edf54655c2e3743ddce912dce664/scripts/tests/test_exobsforge_global_dump.py#L7-L15)
but `MarineObsPrep.finalize()` expects `COMOUT_OBSFORGE_MARINE_OBS` to
exist in the environment/config. Since the script builds its config from
`os.environ` first, that missing variable propagates into a `KeyError`.
## Solution
Set the required output directory variables in the test environment
before running the script.
### Recommended patch
Update
[`scripts/tests/test_exobsforge_global_dump.py`](https://github.com/NOAA-EMC/obsForge/blob/e52c83bb5289edf54655c2e3743ddce912dce664/scripts/tests/test_exobsforge_global_dump.py)
so the fixture provides the missing paths:
```python
import pytest
import os
import subprocess
from pathlib import Path
@pytest.fixture
def script_env(tmp_path):
env = os.environ.copy()
env["cyc"] = "0"
env["current_cycle"] = "20250301600"
env["PDY"] = "20250316"
env["RUN"] = "gdas"
# Required runtime directories
env["DCOMROOT"] = str(tmp_path / "dcom")
env["COMROOT"] = str(tmp_path / "com")
env["COMOUT_OBSFORGE_MARINE_OBS"] = str(tmp_path / "com" / "marine_obs")
env["COMOUT_OBSFORGE_AOD_OBS"] = str(tmp_path / "com" / "aod_obs")
# Ensure directories exist
Path(env["DCOMROOT"]).mkdir(parents=True, exist_ok=True)
Path(env["COMOUT_OBSFORGE_MARINE_OBS"]).mkdir(parents=True, exist_ok=True)
Path(env["COMOUT_OBSFORGE_AOD_OBS"]).mkdir(parents=True, exist_ok=True)
return env
```
## Why this is the right fix
- The failing traceback points to missing runtime configuration, not bad
Python logic in the script.
- The workflow just runs `pytest scripts/tests` in the container, so the
test must fully mock the environment itself; nothing in
[`.github/workflows/build.yaml`](https://github.com/NOAA-EMC/obsForge/blob/e52c83bb5289edf54655c2e3743ddce912dce664/.github/workflows/build.yaml#L42-L65)
injects these variables.
- `exobsforge_global_marine_dump.py` depends on environment-driven
config assembly at lines 17–30, so the test must provide all required
k...
</details>
<!-- START COPILOT CODING AGENT SUFFIX -->
*This pull request was created from Copilot chat.*
>
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
CoryMartin-NOAA
approved these changes
May 7, 2026
Contributor
|
Added @givelberg and @apchoiCMD as reviewers because I haven't touched this in ages |
Contributor
|
@copilot resolve the merge conflicts in this pull request and ask me if you run into any issues |
…nstead-of-hard-coded-paths Co-authored-by: CoryMartin-NOAA <6354668+CoryMartin-NOAA@users.noreply.github.com>
Contributor
Author
Done in commit |
CoryMartin-NOAA
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
JOBSFORGE_GLOBAL_MARINE_BUFR_DUMP: updateCOMIN_OBSPROCto include/${RUN}.${PDY}/${cyc}parm/bufr2ioda_template.yaml.j2: simplifydump_filenameto use updatedCOMIN_OBSPROCOBSFORGEprefix for explicitness_OBSsuffix to ATMOS COM variables per reviewer requestorigin/main(commit1790f36): job scripts moved fromjobs/todev/jobs/;jobsis now a symlink todev/jobs/. All compath.py changes preserved in renamed locations.