Skip to content

Fix backwards compatibility issue#105

Open
JordanLaserGit wants to merge 6 commits into
mainfrom
issue-103-docker-compat
Open

Fix backwards compatibility issue#105
JordanLaserGit wants to merge 6 commits into
mainfrom
issue-103-docker-compat

Conversation

@JordanLaserGit

Copy link
Copy Markdown
Collaborator

Restores backward compatibility with older datastream containers (issue #103): t-route restart/crosswalk args are now handed to datastream config gen, bmi-config-gen, and validation steps via environment variables instead of CLI flags, so older containers ignore them rather than erroring on unrecognized arguments. Also adds a manual workflow to test deployed container combinations.

Additions

  • Manual-dispatch workflow .github/workflows/test_datastream_backward_compatibility.yaml that runs the datastream against each deployed DS/FP/NGIAB combination, or a custom combo passed as inputs.

Removals

  • --troute_restart/--troute_crosswalk(_file) CLI flags from the configure_datastream.py, ngen_configs_gen.py, and run_validator.py docker invocations.
  • Redundant RESTART_BASE branching in the ngen config-gen step.

Changes

  • scripts/datastream passes t-route restart/crosswalk to those three steps via docker run -e env vars.
  • configure_datastream.py, ngen_configs_gen.py, and run_validator.py default those args from the corresponding env vars.

Testing

  1. Actions → "Test Datastream Backward Compatibility" → Run workflow.

Screenshots

Notes

The docker version combinations that are used in the NRDS are hardcoded into the workflow. It'll accept another combination as args, but that combinations structure will ideally be updated with NRDS releases.

Todos

  • Update CHANGELOG "Unreleased" section.

Checklist

  • PR has an informative and human-readable title
  • Changes are limited to a single goal (no scope creep)
  • Code can be automatically merged (no conflicts)
  • Code follows project standards (link if applicable)
  • Passes all existing automated tests
  • Any change in functionality is tested
  • New functions are documented (with a description, list of inputs, and expected output)
  • Placeholder code is flagged / future todos are captured in comments
  • Visually tested in supported browsers and devices (see checklist below 👇)
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

  • Windows
  • Linux
  • Browser

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

@JordanLaserGit JordanLaserGit self-assigned this Jul 22, 2026
@JordanLaserGit JordanLaserGit added bug Something isn't working P0 Priority level. P0: Critical, P1: High, P2: Medium, P3: Low labels Jul 22, 2026
@JordanLaserGit JordanLaserGit changed the title Fix backwards compatibility issue (#103) Fix backwards compatibility issue Jul 22, 2026
Comment thread scripts/datastream
echo "COMMAND: docker run --rm -v "$NGEN_RUN":"$DOCKER_MOUNT" \
echo "COMMAND: docker run --rm -v "$NGEN_RUN":"$DOCKER_MOUNT" \
-u $(id -u):$(id -g) \
-e TROUTE_RESTART_FILE="${RESTART_BASE:+$DOCKER_MOUNT/restart/$RESTART_BASE}" -e TROUTE_CROSSWALK_FILE="${CROSSWALK_BASE:+$DOCKER_MOUNT/restart/$CROSSWALK_BASE}" \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What a neat bash operator trick. I'm stealing this

Comment thread README.md Outdated

## Getting Started
* **Installation:** Follow the [Installation Guide](https://github.com/CIROH-UA/datastreamcli/blob/main/INSTALL.md) to prepare your environment for `DataStreamCLI`.
* **Tutorial:** New to `DataStreamCLI`? Work through the [Getting Started guide](https://github.com/CIROH-UA/datastreamcli/blob/main/docs/GETTING_STARTED.md) — progressive, explained examples that build from the minimal command up to reproducing a Research DataStream simulation.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This link doesn't point to anything. Was this file meant to be added to this PR?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is why I shouldn't dev at night. Meant to push this to a different branch... 🙃

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@quinnylee

Copy link
Copy Markdown
Collaborator

Overall looks like a good PR. All the pytests are passing locally, and the failing CI test seems to be unrelated to this change. Only change I would want is in the README addition that doesn't point to an existing markdown file.

@JordanLaserGit
JordanLaserGit requested a review from quinnylee July 23, 2026 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working P0 Priority level. P0: Critical, P1: High, P2: Medium, P3: Low

Projects

Development

Successfully merging this pull request may close these issues.

2 participants