update etl configuration input to yml instead of environment variables by adamkorynta · Pull Request #1795 · USACE/cwms-data-api

adamkorynta · 2026-06-22T17:56:03Z

Summary

file system staging update for projects, locations, and timeseries
add in REGI files to VCS
add in docker environment for running python tests
move etl calls to parent docker-compose file
let the source URL be optional, if not set only use staged filesystem resources

Related Issue

Validation

Unit tests and docker compose in local dev environment

Checklist

AI tools used - GH Copilot for README.md generation, logger messages, yml parsing, and code review

adamkorynta · 2026-06-22T17:57:29Z

@dezidizon please review revisions to @RyanM-RMA's initial implementation

dezidizon

Performed code review and only found minor changes to capitalize several print out statements. This code is ready for merging even without implementing the requested updates.

MikeNeilson

Seems reasonable. Mostly commenting to track what I've already looked at.

MikeNeilson · 2026-06-29T15:04:20Z

+
+## Configuration Overview
+
+The main runtime configuration is stored in a YAML file, defaulting to `regi.yml` in the working directory.


Might want to name this something other than REGI. Not a major issue, but we might get less, I'll go with "side eye", if we use a more generic name.

krowvin

Not sure on a few items

krowvin · 2026-07-30T19:36:25Z

    for future in as_completed(futures_to_items):
        item = futures_to_items[future]
        if future.exception():
-            logger.warning(f"Exception occurred for {item}: {future.exception()}")
+            logger.warning(_friendly_exception_message(item, future.exception()))


The failures looked to be logged within the thread. This looks like an ETL run would be partial and exit with status 0 within the thread.

Should we instead collect the failures and raise after the futures finish? This way compose/CI/operators can see that the migration was not fully successful.

good catch, updated to consolidate all the failures

krowvin · 2026-07-30T19:37:38Z

+      - ./cda-etl/data/regi:/data/regi
+    environment:
+      - REGI_CONFIG_PATH=/app/regi.yml
+      - SOURCE_CDA_URL=${SOURCE_CDA_URL:-https://cwms-data-test.cwbi.us/cwms-data/}


Should we leave this empty here (no default) so we can use the staged files only mode?

works for me. removed

krowvin · 2026-07-30T19:38:40Z

+### Run the ETL container
+
+```bash
+./gradlew runEtl


I'm not seeing runEtl in the build.gradle

Should this be etlEnvFile or are we missing a task in the build.gradle?

There is a task registered in cda-etl/build.gradle

krowvin · 2026-07-30T19:39:48Z

+      - id: EUFA
+        enabled: true
+        locations:
+          - id: SWT.EUFA-Dam


Should this just be EUFA-DAM without the office prefixed in a location?

yup, fixed that

- file system staging update for projects, locations, and timeseries - add in REGI files to VCS - add in docker environment for running python tests - move etl calls to parent docker-compose file - let the source URL be optional, if not set only use staged filesystem resources

Co-authored-by: dezidizon <[email protected]>

MikeNeilson · 2026-08-17T19:49:18Z

+
+### Other Runtime Settings
+
+- `REGI_CONFIG_PATH`: Path to the YAML config file. Defaults to `regi.yml`.


Suggested change

- `REGI_CONFIG_PATH`: Path to the YAML config file. Defaults to `regi.yml`.

- `ETL_CONFIG_PATH`: Path to the YAML config file. Defaults to `etl.yml`.

That seems like a reasonably generic name.

based on our previous discussion I renamed to sample-app in: #1856 just haven't had a chance to bring that change into this PR yet

MikeNeilson · 2026-08-17T19:49:44Z

+### Run the ETL container
+
+```bash
+./gradlew runEtl


There is a task registered in cda-etl/build.gradle

MikeNeilson

Just my one nitpick about the variable name.

MikeNeilson · 2026-08-17T20:17:12Z

+The main runtime configuration is stored in a YAML file, defaulting to `sample-app.yml` in the working directory.

-The application reads the YAML path from the `REGI_CONFIG_PATH` environment variable. If the variable is not set, it looks for `regi.yml` next to where the process starts.
+The application reads the YAML path from the `SAMPLE_APP_CONFIG_PATH` environment variable. If the variable is not set, it looks for `sample-app.yml` next to where the process starts.


I'm going to suggest "ETL_CONFIG_PATH" again. Calling the sample "sample-app" seems reasonable but it doesn't really make sense the variable pointing to the configuration would be called sample app. I would think we'd want the same program to target different configurations, not have to make adjustments to the program to point to different variables to make sense in the given usage.

"SAMPLE_DATA" would also work in that context.

yeah I like ETL_CONFIG_PATH

so what do you think about:

ETL_CONFIG_PATH for the env var name
sample-data for the data directory
sample-app.yml for the yml file

Yeah, that's works.

ETL_CONFIG_PATH for env variable sample-data for data directory sample-app.yml to domain the application

MikeNeilson · 2026-08-17T20:30:37Z

+      - ./cda-etl/data/regi/regi.yml:/app/regi.yml
+      - ./cda-etl/data/regi:/data/regi
+    environment:
+      - REGI_CONFIG_PATH=/app/regi.yml


Suggested change

- REGI_CONFIG_PATH=/app/regi.yml

- ETL_CONFIG_PATH=/app/sample-app.yml

... and the volumes above.

shoulda text searched outside subdirectory...

adamkorynta requested a review from krowvin June 22, 2026 17:56

adamkorynta changed the title ~~update configuration input to yml instead of environment variables~~ update etl configuration input to yml instead of environment variables Jun 22, 2026

dezidizon approved these changes Jun 23, 2026

View reviewed changes

Comment thread cda-etl/src/cda_etl/config.py Outdated

Comment thread cda-etl/src/cda_etl/config.py Outdated

Comment thread cda-etl/src/cda_etl/config.py Outdated

Comment thread cda-etl/src/cda_etl/config.py Outdated

Comment thread cda-etl/src/cda_etl/config.py Outdated

MikeNeilson reviewed Jun 29, 2026

View reviewed changes

krowvin reviewed Jul 30, 2026

View reviewed changes

adamkorynta and others added 4 commits August 17, 2026 10:59

remove tests that reference old methods

fc8688b

Apply suggestions from code review

d5c3d80

Co-authored-by: dezidizon <[email protected]>

fix typo

e56f37d

adamkorynta force-pushed the feature/etl_follow_up branch from d6a4ac7 to e56f37d Compare August 17, 2026 17:59

adamkorynta added 2 commits August 17, 2026 12:26

updates from PR feedback

b3b520b

cleanup build error

f560e95

MikeNeilson requested changes Aug 17, 2026

View reviewed changes

adamkorynta added 2 commits August 17, 2026 13:01

simplify docker build command

ca3d352

rename REGI references to sample-app

0a36ba4

MikeNeilson requested changes Aug 17, 2026

View reviewed changes

change variable name

9bcff95

ETL_CONFIG_PATH for env variable sample-data for data directory sample-app.yml to domain the application

MikeNeilson requested changes Aug 17, 2026

View reviewed changes

cleanup compose names

7d3b581

MikeNeilson approved these changes Aug 17, 2026

View reviewed changes

adamkorynta merged commit d0f9391 into develop Aug 17, 2026
10 checks passed

adamkorynta deleted the feature/etl_follow_up branch August 17, 2026 20:51


		## Configuration Overview

		The main runtime configuration is stored in a YAML file, defaulting to `regi.yml` in the working directory.


		### Other Runtime Settings

		- `REGI_CONFIG_PATH`: Path to the YAML config file. Defaults to `regi.yml`.

	- `REGI_CONFIG_PATH`: Path to the YAML config file. Defaults to `regi.yml`.
	- `ETL_CONFIG_PATH`: Path to the YAML config file. Defaults to `etl.yml`.

	- REGI_CONFIG_PATH=/app/regi.yml
	- ETL_CONFIG_PATH=/app/sample-app.yml

Uh oh!

Conversation

adamkorynta commented Jun 22, 2026

Summary

Related Issue

Validation

Checklist

Uh oh!

adamkorynta commented Jun 22, 2026

Uh oh!

dezidizon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MikeNeilson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

krowvin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeNeilson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants