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

Expose timeout for lock via environment variable configuration #786

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pmm4654
Copy link

@pmm4654 pmm4654 commented Dec 19, 2023

Expose the timeout configuration option used in Resque::Scheduler::Locking::Base

Related issue: #700

@pmm4654 pmm4654 force-pushed the master branch 3 times, most recently from df7ef73 to 1f9d5cf Compare December 19, 2023 20:02
@pmm4654 pmm4654 marked this pull request as ready for review December 19, 2023 20:02
@learoyklinginsmith
Copy link

@PatrickTulskie any chance we could get some eyes on this change? 🙏

@PatrickTulskie PatrickTulskie self-requested a review April 26, 2024 13:11
Copy link
Contributor

@PatrickTulskie PatrickTulskie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall good. I'd just like to set the env stuff the way it was instead of the meta programming approach. Calling send usually winds up causing a lot more scrutiny.


c.logfile = options[:logfile] if options.key?(:logfile)

c.logformat = options[:logformat] if options.key?(:logformat)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just leave this and add the new timeout option?

@pmm4654 pmm4654 force-pushed the master branch 3 times, most recently from 2b12b0e to 3f2d65c Compare November 18, 2024 18:05
@pmm4654
Copy link
Author

pmm4654 commented Nov 27, 2024

@PatrickTulskie Hey there! I'm just nudging this PR after I made the changes you requested.

Sign up for free to 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.

3 participants