- Issue created by @mstrelan
- Merge request !8620Issue #3458403: Conditionally disable access to update manager routes → (Closed) created by mstrelan
- Status changed to Needs work
2 months ago 11:13pm 1 July 2024 - 🇦🇺Australia mstrelan
This is effectively the same as removing the
setCacheMaxAge(0)call inUpdateManagerAccessCheck::access. Should we just remove that instead? - Status changed to Needs review
2 months ago 10:58pm 2 July 2024 - 🇦🇺Australia mstrelan
Deprecated the access check and added a CR. Not sure if versions are correct.
- Status changed to Needs work
2 months ago 2:04pm 5 July 2024 - 🇺🇸United States smustgrave
Hey sorry but can the issue summary be updated also.
Just reading the summary I wouldn't of known we are deprecating UpdateManagerAccessCheck for example
- Status changed to Needs review
2 months ago 3:54pm 5 July 2024 - Status changed to RTBC
2 months ago 3:59pm 5 July 2024 - 🇳🇿New Zealand quietone New Zealand
All questions here are answered, I read the MR and CR and they are clear. The only thing left would be changes for a 10.4 backport, but that doesn't need to be done now.
- Status changed to Fixed
2 months ago 8:50am 8 July 2024 - 🇬🇧United Kingdom catch
Committed/pushed to 11.x, thanks!
I was about to ask for a backport MR, then I realised it would just be the same change with the first hunk of the diff removed, so went ahead and made that change locally for both 11.0.x and 10.4.x. Reclassifying as a bug report since this fixes performance issues.
- 🇦🇺Australia mstrelan
Any chance of a back port to 10.3 where the regression was introduced?
- 🇬🇧United Kingdom catch
Was a bit hesitant because of the new class, but I think it's fine and better than shipping the bug for 11 months - backported to 10.3.x too.
Automatically closed - issue fixed for 2 weeks with no activity.