Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ModLog] Optimise get_next_case_number() #2908

Merged

Conversation

Tobotimus
Copy link
Member

@Tobotimus Tobotimus commented Jul 30, 2019

We've noticed that this is taking an obscenely long time on very large servers.

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
@Tobotimus Tobotimus added the Type: Optimisation label Jul 30, 2019
@Tobotimus Tobotimus requested review from Kowlin and mikeshardmind Jul 30, 2019
@Tobotimus Tobotimus requested a review from palmtree5 as a code owner Jul 30, 2019
redbot/core/modlog.py Show resolved Hide resolved
@mikeshardmind mikeshardmind added the Changelog Entry: Pending label Jul 30, 2019
mikeshardmind
mikeshardmind previously requested changes Jul 30, 2019
Copy link
Contributor

@mikeshardmind mikeshardmind left a comment

found something else...

redbot/core/modlog.py Outdated Show resolved Hide resolved
Tobotimus added 2 commits Jul 31, 2019
Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
@Tobotimus Tobotimus requested a review from Twentysix26 as a code owner Jul 31, 2019
@mikeshardmind mikeshardmind self-requested a review Jul 31, 2019
Copy link
Contributor

@mikeshardmind mikeshardmind left a comment

one last change from me after some consideration.

Let's go ahead and make get_next_case_number private and remove it from __all__

There's not a reason to expose this function outside of modlog, and the next release is one which allows for removing this from the public API.

Signed-off-by: Toby Harradine <tobyharradine@gmail.com>
@Tobotimus
Copy link
Member Author

@Tobotimus Tobotimus commented Jul 31, 2019

@mikeshardmind I've actually gone ahead and removed the function all together. It was only used in one place, and that place is kind of easier to understand (w.r.t. lock usage) if it just uses config directly.

mikeshardmind
mikeshardmind previously approved these changes Jul 31, 2019
@mikeshardmind mikeshardmind dismissed their stale review Jul 31, 2019

Apparently it *isn't the only place (discussion: discord)

@mikeshardmind mikeshardmind removed the Changelog Entry: Pending label Aug 2, 2019
@mikeshardmind mikeshardmind merged commit ef8b9b8 into Cog-Creators:V3/develop Aug 2, 2019
1 check passed
@Tobotimus Tobotimus deleted the V3/modlog_optimise_casenumber branch Aug 3, 2019
@mikeshardmind mikeshardmind added this to the 3.2.0 milestone Aug 30, 2019
@jack1142 jack1142 added the Breaking Change label May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Type: Optimisation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants