Skip to content

CWMS-2172: Enabled LRTS tests for Forecast Spec - #1187

Merged
MikeNeilson merged 12 commits into
USACE:developfrom
zack-rma:feature/forecast_lrts
Aug 1, 2025
Merged

CWMS-2172: Enabled LRTS tests for Forecast Spec#1187
MikeNeilson merged 12 commits into
USACE:developfrom
zack-rma:feature/forecast_lrts

Conversation

@zack-rma

Copy link
Copy Markdown
Collaborator

Fixes HydrologicEngineeringCenter/cwms-database#24.

Enables LRTS interval identifier integration tests for Forecast spec retrieval.

@zack-rma
zack-rma requested a review from adamkorynta July 15, 2025 17:29
adamkorynta
adamkorynta previously approved these changes Jul 24, 2025
.withVolumeName(TeamCityUtilities.cleanupBranchName(ORACLE_VOLUME))
.withSchemaImage(CWMS_DB_IMAGE);
cwmsDb.withImagePullPolicy(PullPolicy.defaultPolicy());
cwmsDb.withImagePullPolicy(PullPolicy.alwaysPull());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should be "if not present"

If not present will still check if it's up-to-date or not.

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.

I didn't see an available "if not present" policy, but I've reverted to the default policy. The db image link change resolved the build error, so the pull policy shouldn't be an issue.

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.

@MikeNeilson Zack reverted back to defaultPolicy. Can you resolve your requested change so that the PR can be merged?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, we need to figure out how to override that when I randomly go on vacation and forget to use "comment" instead of "request changes"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I do have the ability to dismiss your review, but figured there was no pressing timeline on this simple change

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

image

@MikeNeilson
MikeNeilson merged commit 7e18418 into USACE:develop Aug 1, 2025
5 checks passed
@zack-rma
zack-rma deleted the feature/forecast_lrts branch August 1, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

New LRTS interval identifier not being returned properly

4 participants