API key auth optional feature flag - #1909
labkey-adam wants to merge 8 commits into
Conversation
|
We need to make sure that this gets enabled as part of the upgrade, either via a user education handoff or possibly via an upgrade script. |
labkey-martyp
left a comment
labkey-martyp
left a comment
There was a problem hiding this comment.
We need to make sure that this gets enabled as part of the upgrade, either via a user education handoff or possibly via an upgrade script.
Yes I think an upgrade script would be preferred as this will not be in production likely until well into 2027. Handling it now in a script ensures this is taken care of far down the road.
Due to security concerns, we want this off by default. I think a core upgrade script would be ill-advised. I could accept ONPRC- and SNPRC-specific upgrade scripts, but ONPRC (at least) will need to take another step: migrating to use the |
…eset optional feature. This way, it's not affected by signOut in the test.
|
@labkey-jeckels not sure how I "dismissed" your review, but I guess I did |
| } | ||
| finally | ||
| { | ||
| OptionalFeatureHelper.resetOptionalFeature(cn, API_KEY_OPTIONAL_FEATURE_FLAG); |
There was a problem hiding this comment.
I've been warned in other scenarios about doing work in finally blocks. @labkey-tchad is this OK?
labkey-jeckels
left a comment
labkey-jeckels
left a comment
There was a problem hiding this comment.
Worth a confirmation on the try/finally approach before merging
3 participants
Related Pull Requests