Skip to content

Ticket 45210: Fall back quietly when failing to add demographics source - #124

Closed
labkey-jeckels wants to merge 1 commit into
release22.3-SNAPSHOTfrom
22.3_fb_quietPermissionErrors
Closed

Ticket 45210: Fall back quietly when failing to add demographics source#124
labkey-jeckels wants to merge 1 commit into
release22.3-SNAPSHOTfrom
22.3_fb_quietPermissionErrors

Conversation

@labkey-jeckels

Copy link
Copy Markdown

Rationale

The previous attempt to quiet logging about failing to resolve demographics sources (in this case, because of linked schema permission changes), #114, either focused on the wrong spot or there's another codepath that we didn't catch.

Example logging from production that's filling the log files:

java.lang.IllegalArgumentException: Unable to create table for query: Demographics
at org.labkey.laboratory.DemographicsSource.validateKey(DemographicsSource.java:146) ~[laboratory-22.3-SNAPSHOT.jar:?]
at org.labkey.laboratory.DemographicsSource.getFromPropertyManager(DemographicsSource.java:63) ~[laboratory-22.3-SNAPSHOT.jar:?]
at org.labkey.laboratory.LaboratoryServiceImpl.getDemographicsSources(LaboratoryServiceImpl.java:384) [laboratory-22.3-SNAPSHOT.jar:?]
at org.labkey.laboratory.query.LaboratoryTableCustomizer.appendDemographicsCols(LaboratoryTableCustomizer.java:265) [laboratory-22.3-SNAPSHOT.jar:?]
at org.labkey.laboratory.query.LaboratoryTableCustomizer.appendCalculatedCols(LaboratoryTableCustomizer.java:257) [laboratory-22.3-SNAPSHOT.jar:?]
at org.labkey.laboratory.query.DefaultAssayCustomizer.customizeDataTable(DefaultAssayCustomizer.java:203) [laboratory-22.3-SNAPSHOT.jar:?]
at org.labkey.laboratory.query.DefaultAssayCustomizer.customizeAssayTable(DefaultAssayCustomizer.java:54) [laboratory-22.3-SNAPSHOT.jar:?]
at org.labkey.laboratory.query.DefaultAssayCustomizer.customize(DefaultAssayCustomizer.java:36) [laboratory-22.3-SNAPSHOT.jar:?]

Changes

  • Reduce already caught exception logging to debug level. This will keep it from spamming the logs but still let admins opt-in by changing the log level for LaboratoryServiceImpl via the Admin Console for troubleshooting.

@bbimber

Copy link
Copy Markdown
Collaborator

@labkey-jeckels do you know more about what sort of cases induce this issue? While I appreciate your point about opt-in to errors and appreciate the point about not wanting to spam the logs with tons of errors, I still dont think I have a 100% personal handle on what causes this disconnect and what's the right behavior within the customizer. I'd rather see whether we can alter the code within LaboratoryTableCustomizer or DemographicsSource to differentiate "real" bad configuration from the permissions case, which is a different condition.

Looking quickly, I think validateKey() could test permissions, return false, and avoid this logging. I can open a PR on that.

@bbimber

Copy link
Copy Markdown
Collaborator

@labkey-jeckels what about this? bimberlabinternal#26

@labkey-jeckels

Copy link
Copy Markdown
Author

Thanks for the review, Ben. I commented in the alternative PR with more info.

@labkey-jeckels
labkey-jeckels deleted the 22.3_fb_quietPermissionErrors branch May 26, 2022 16:58
Sign up for freeto 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

@labkey-jeckels@bbimber