Improve SSRS session ID handoff #1294
Conversation
|
@brentlogan @jryurkanin Assuming these changes pass automated tests, I'd like to see if ONPRC can test my feature branch with the real SSRS server. I think the new code is generating equivalent URLs but I can't test in any depth because I don't have your SSRS setup and its associated reports. |
|
@labkey-jeckels I see build #159 from this morning. If that is the correct build, I can install that on a test VM and check it against the production SSRS server. |
Please wait for build #178 which will include my last set of changes. It's in the queue now: |
|
@labkey-jeckels I attempted to install the build you linked in your prior comment. It failed because there are two modules with the name ldk. I'll copy the 10 changed files over to my dev environment and test that way. |
|
@labkey-jeckels I won't be able to check this on my dev machine: there's no session id and the SSRS VM won't be able to connect to "localhost." I suspect the reason why the build doesn't show on our standard ONPRC Installer page in TeamCity is what causes the multiple modules to have the name "ldk." Once you get that issue resolved, I'll be able to test this for you. |
I'm taking a look at what went wrong with the installer build. There are duplicates of a handful of modules, including LDK. There's a matching feature branch in the Git repo where they live which must be tripping things up |
|
@brentlogan I checked that the new build doesn't have the duplicate modules. Please give it a try at your convenience. |
|
@brentlogan I wanted to check in and make sure you have what you need to test this out. |
|
@labkey-jeckels He replied here about it, sorry for not pointing this out earlier: https://www.labkey.org/ONPRC/Support%20Tickets/issues-details.view?issueId=52120 |
|
@labkey-martyp ready for you to review. I won't merge until I get full confirmation from ONPRC but any changes should be minor at this point. |
|
@labkey-jeckels, thanks for your work on this. I just talked with @Ohsudev this morning about standardizing. He will change one of the reports so we can test standardizing the capitalization of sessionid. We'll let you know when that testing is complete, which I expect by the end of the week. |
|
Marty, or Mr Patel. Can you please review, try to resolve all conflicts, and "merged" onto release |
I pushed a fix. It was just a missing import. |
|
Josh. I pushed a release early this morning that sets the last script within the onprc_ehrModule.java as
@OverRide
public @nullable Double getSchemaVersion()
{
return 24.009;
}
Hopefully, you did use that version number for you PR.
Thanks,
Raymond
From: Josh Eckels ***@***.***>
Sent: Monday, May 19, 2025 9:51 AM
To: LabKey/onprcEHRModules ***@***.***>
Cc: Raymond Blasa ***@***.***>; Mention ***@***.***>
Subject: [EXTERNAL] Re: [LabKey/onprcEHRModules] Improve SSRS session ID handoff (PR #1294)
[Image removed by sender.]labkey-jeckels left a comment (LabKey/onprcEHRModules#1294)<https://urldefense.com/v3/__https:/github.com/LabKey/onprcEHRModules/pull/1294*issuecomment-2891675024__;Iw!!Mi0JBg!MQ4YlxSMXz0bIuzGL9MBcILQgOSodLCj5uYU7JKAluOzVAMNd04z1du9BNyvmOOs1-8g-LGqYfL5jTVbkV9DrXI$>
Marty, or Mr Patel. Can you please review, try to resolve all conflicts, and "merged" onto release
I pushed a fix. It was just a missing import.
—
Reply to this email directly, view it on GitHub<https://urldefense.com/v3/__https:/github.com/LabKey/onprcEHRModules/pull/1294*issuecomment-2891675024__;Iw!!Mi0JBg!MQ4YlxSMXz0bIuzGL9MBcILQgOSodLCj5uYU7JKAluOzVAMNd04z1du9BNyvmOOs1-8g-LGqYfL5jTVbkV9DrXI$>, or unsubscribe<https://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/ASHU2YANIB75XKNRTQBSJK327IDXDAVCNFSM6AAAAABZJ4LESSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQOJRGY3TKMBSGQ__;!!Mi0JBg!MQ4YlxSMXz0bIuzGL9MBcILQgOSodLCj5uYU7JKAluOzVAMNd04z1du9BNyvmOOs1-8g-LGqYfL5jTVb6S3agSo$>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
I hadn't done a merge from the main 24.11 branch yet - I missed that was part of the request. I've done that now. My work didn't have any schema version changes so there was nothing to manually merge there. |
|
@brentlogan @Ohsudev checking in on this PR. Anything else you need from me to help get it into production? |
|
@labkey-jeckels, could you please resolve the conflicts in ONPRC_EHRController.java? Thanks! |
# Conflicts: # onprc_ehr/src/org/labkey/onprc_ehr/ONPRC_EHRController.java
The conflicts were all in the As I'm sure you know, the longer this feature branch stays open the more conflicts it'll accumulate. I hope it can be merged soon but know that it requires some coordination on the SSRS side. |
|
Thanks, @labkey-jeckels! @Ohsudev has a different deploy ready for tonight, but I hope to get this one out this coming Monday. |
brentlogan
left a comment
brentlogan
left a comment
There was a problem hiding this comment.
Josh, this looks good and we're finally ready to deploy. I was able to test and get no errors, so we're squashing and merging and deploying tonight.
Rationale
Switch from pulling session ID from cookie to an API call for handoff to SSRS. Consolidate many duplicate chunks of code.
Related Pull Requests
Changes
onprc_ehr-getSessionIdAPI to get session ID for SSRS usageLABKEY.Utils.getSessionID()