Bugfix/session office not set#1757
Draft
msweier wants to merge 4 commits into
Draft
Conversation
…od() and ctx.url()
Author
|
I updated the ClobControllerTest because it is the only mock setup that is building the request by hand and it needed to match the new context. The integration tests didn't needed because it is testing for real. |
Collaborator
|
Seems worth trying. If we're always setting the office here should SessionOfficePreparer get removed from the AuthDao? |
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.
Summary
This is an AI assisted PR to help get the ball rolling on this bug fix.
Adds
SessionOfficePreparerto the connection preparer chain inJooqDao.getDslContext(Context)so that every pooled connection gets its Oracle session office set on checkout.I've seen intermittant 500 errors on develop CDA that traces to (
ORA-20047: SESSION_OFFICE_ID_NOT_SET) on authenticated requests to the timeseries endpoint when multithreading a POR timeseries pull, but AI suggests this applies to all office scoped endpoints.This may have been latent since 2022 but exposed with PR #1692 made it visible by switching timeseries reads from
cwms_ts.retrieve_ts_out_tab(which handles office context internally) to a direct SQL path that callsCWMS_UTIL.CONVERT_UNITSinline. Non-authenticated requests ppear unaffected becauseAuthDao.prepareGuestContext()already wires in aSessionOfficePreparer; the authenticated path viaprepareContextWithUser()doesn't.Related Issue
Closes #1753
Validation
This is difficult to test. The failure depends on connection pool state, which connections are warm, what office they were last used for, and whether the pool has recycled them. AI couldn't reliably reproduce it in a single-threaded integration test, and even under concurrent load it only fails a fraction of the time. I am able to reporduce using a cwms-python POR query in develop, but I couldn't validate on a local instance.
The new
JooqDaoTestITdoes what it can: it verifiesSessionOfficePrepareragainst the Oracle instance by checking that the session variable gets set on a cold connection and correctly overridden when switching offices, but it can't replicate the race conditions that causes the error. The tests also pass on the currentdevelopbranch since I couldn't replicate the error condition in the test enviorment.If anyone has ideas for a better way to exercise the pool-level behavior, I'm all ears.
Checklist