Skip to content

Correct Location Delete with new Geometry table. - #201

Open
MikeNeilson wants to merge 6 commits into
mainfrom
bugfix/200-delete-loc-with-geometry
Open

Correct Location Delete with new Geometry table.#201
MikeNeilson wants to merge 6 commits into
mainfrom
bugfix/200-delete-loc-with-geometry

Conversation

@MikeNeilson

Copy link
Copy Markdown
Contributor

Will implement fix later. Commentary certainly welcome.

Pretty sure it just needs the addition of "delete_loc" in the check for delete at_location_geometry.

But also need to expand test to include sub locations.

@MikeNeilson
MikeNeilson requested review from a team and katfeingold and removed request for a team July 17, 2026 20:58
@MikeNeilson MikeNeilson changed the title Initial test to prove issue. Correct Location Delete with new Geometry table. Jul 20, 2026
@MikeNeilson

Copy link
Copy Markdown
Contributor Author

NOTE: still need to add test for deleting sublocations, but otherwise gives the sense of what needs to happen. E.G. at_location_geometry should be treated as "part of the location" for delete and not need _all or _cascade to be specified. If the location is deleted, the geometry just dies.

@MikeNeilson

Copy link
Copy Markdown
Contributor Author

NOTE: still need to add test for deleting sublocations, but otherwise gives the sense of what needs to happen. E.G. at_location_geometry should be treated as "part of the location" for delete and not need _all or _cascade to be specified. If the location is deleted, the geometry just dies.

Tests and fixes were added.

@ktarbet ktarbet left a comment

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.

Looks OK to me.

Comment thread schema/build.xml
<exec executable="utplsql" resultproperty="utreturn">
<arg value="run"/>
<arg value="-p=${oracle.cwms.user}.test_cwms_rating,${oracle.cwms.user}.test_cwms_text,${oracle.cwms.user}.test_lrts_updates,${oracle.cwms.user}.test_probability_parameter,${oracle.cwms.user}.test_cwms_loc,${oracle.cwms.user}.test_cwms_ts,${oracle.cwms.user}.test_cwms_util,${oracle.cwms.user}.test_cwms_err,${oracle.cwms.user}.test_versioned_time_series,${oracle.cwms.user}.test_update_ts_extents,${oracle.cwms.user}.test_cwms_pool,${oracle.cwms.user}.test_timeseries_snapping,${oracle.cwms.user}.test_cwms_cat,${oracle.cwms.user}.test_cwms_msg,${oracle.cwms.user}.test_cwms_level,${oracle.cwms.user}.test_cwms_water_supply,${oracle.cwms.user}.test_cwms_display,${oracle.cwms.user}.test_cwms_data_dissem,${oracle.cwms.user}.test_cwms_stream,${oracle.cwms.user}.test_cwms_measurements,${oracle.cwms.user}.test_cwms_fcst,${oracle.cwms.user}.test_cwms_forecast,${oracle.cwms.user}.test_cwms_xchg,${oracle.cwms.user}.test_cwms_cache"/>
<!-- <arg value="-p=${oracle.cwms.user}.test_cwms_rating,${oracle.cwms.user}.test_cwms_text,${oracle.cwms.user}.test_lrts_updates,${oracle.cwms.user}.test_probability_parameter,${oracle.cwms.user}.test_cwms_loc,${oracle.cwms.user}.test_cwms_ts,${oracle.cwms.user}.test_cwms_util,${oracle.cwms.user}.test_cwms_err,${oracle.cwms.user}.test_versioned_time_series,${oracle.cwms.user}.test_update_ts_extents,${oracle.cwms.user}.test_cwms_pool,${oracle.cwms.user}.test_timeseries_snapping,${oracle.cwms.user}.test_cwms_cat,${oracle.cwms.user}.test_cwms_msg,${oracle.cwms.user}.test_cwms_level,${oracle.cwms.user}.test_cwms_water_supply,${oracle.cwms.user}.test_cwms_display,${oracle.cwms.user}.test_cwms_data_dissem,${oracle.cwms.user}.test_cwms_stream,${oracle.cwms.user}.test_cwms_measurements,${oracle.cwms.user}.test_cwms_fcst,${oracle.cwms.user}.test_cwms_forecast,${oracle.cwms.user}.test_cwms_xchg,${oracle.cwms.user}.test_cwms_cache"/> -->

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.

consider deleting if not needed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

... oops, that change wasn't supposed to get in, that was to make the tests run faster locally.

Comment thread run_full_sequence.sh
if [ $? -ne 0 ]; then echo "Failed to stop db?"; exit 6; fi

# ant docker.push -Dteamcity.branch=${BRANCH}_${AGENT} -Denv.USER=$AGENT -Denv.BUILD_NUMBER=1 -Ddryrun=true -Ddocker.registry=testregistry $*
# ant docker.push -Dteamcity.branch=${BRANCH}_${AGENT} -Denv.USER=$AGENT -Denv.BUILD_NUMBER=1 -Ddryrun=true -Ddocker.registry=testregistry -Doracle.image=$ORACLE_IMAGE $*

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.

consider removing.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This one is intentionally for when that needs to be tested, but since it's an external operation I didn't want it active by default.

@MikeNeilson MikeNeilson left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Someone feel free to take this over, at least for the build.xml reset anyways, while I'm gone, otherwise I'll get back to it Friday.

Comment thread schema/build.xml
<exec executable="utplsql" resultproperty="utreturn">
<arg value="run"/>
<arg value="-p=${oracle.cwms.user}.test_cwms_rating,${oracle.cwms.user}.test_cwms_text,${oracle.cwms.user}.test_lrts_updates,${oracle.cwms.user}.test_probability_parameter,${oracle.cwms.user}.test_cwms_loc,${oracle.cwms.user}.test_cwms_ts,${oracle.cwms.user}.test_cwms_util,${oracle.cwms.user}.test_cwms_err,${oracle.cwms.user}.test_versioned_time_series,${oracle.cwms.user}.test_update_ts_extents,${oracle.cwms.user}.test_cwms_pool,${oracle.cwms.user}.test_timeseries_snapping,${oracle.cwms.user}.test_cwms_cat,${oracle.cwms.user}.test_cwms_msg,${oracle.cwms.user}.test_cwms_level,${oracle.cwms.user}.test_cwms_water_supply,${oracle.cwms.user}.test_cwms_display,${oracle.cwms.user}.test_cwms_data_dissem,${oracle.cwms.user}.test_cwms_stream,${oracle.cwms.user}.test_cwms_measurements,${oracle.cwms.user}.test_cwms_fcst,${oracle.cwms.user}.test_cwms_forecast,${oracle.cwms.user}.test_cwms_xchg,${oracle.cwms.user}.test_cwms_cache"/>
<!-- <arg value="-p=${oracle.cwms.user}.test_cwms_rating,${oracle.cwms.user}.test_cwms_text,${oracle.cwms.user}.test_lrts_updates,${oracle.cwms.user}.test_probability_parameter,${oracle.cwms.user}.test_cwms_loc,${oracle.cwms.user}.test_cwms_ts,${oracle.cwms.user}.test_cwms_util,${oracle.cwms.user}.test_cwms_err,${oracle.cwms.user}.test_versioned_time_series,${oracle.cwms.user}.test_update_ts_extents,${oracle.cwms.user}.test_cwms_pool,${oracle.cwms.user}.test_timeseries_snapping,${oracle.cwms.user}.test_cwms_cat,${oracle.cwms.user}.test_cwms_msg,${oracle.cwms.user}.test_cwms_level,${oracle.cwms.user}.test_cwms_water_supply,${oracle.cwms.user}.test_cwms_display,${oracle.cwms.user}.test_cwms_data_dissem,${oracle.cwms.user}.test_cwms_stream,${oracle.cwms.user}.test_cwms_measurements,${oracle.cwms.user}.test_cwms_fcst,${oracle.cwms.user}.test_cwms_forecast,${oracle.cwms.user}.test_cwms_xchg,${oracle.cwms.user}.test_cwms_cache"/> -->

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

... oops, that change wasn't supposed to get in, that was to make the tests run faster locally.

Comment thread run_full_sequence.sh
if [ $? -ne 0 ]; then echo "Failed to stop db?"; exit 6; fi

# ant docker.push -Dteamcity.branch=${BRANCH}_${AGENT} -Denv.USER=$AGENT -Denv.BUILD_NUMBER=1 -Ddryrun=true -Ddocker.registry=testregistry $*
# ant docker.push -Dteamcity.branch=${BRANCH}_${AGENT} -Denv.USER=$AGENT -Denv.BUILD_NUMBER=1 -Ddryrun=true -Ddocker.registry=testregistry -Doracle.image=$ORACLE_IMAGE $*

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This one is intentionally for when that needs to be tested, but since it's an external operation I didn't want it active by default.

@MikeNeilson
MikeNeilson requested a review from ktarbet July 24, 2026 21:30
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.

2 participants