Skip to content

Adding initial commit for the blocking functionality in the get func…#535

Open
JoJo10Smith wants to merge 4 commits intopinterest:masterfrom
JoJo10Smith:blocking
Open

Adding initial commit for the blocking functionality in the get func…#535
JoJo10Smith wants to merge 4 commits intopinterest:masterfrom
JoJo10Smith:blocking

Conversation

@JoJo10Smith
Copy link

@JoJo10Smith JoJo10Smith commented Aug 16, 2023

This will add the blocking functionality to the get function with the following optional parameters:

  • max_attempts: this will determine how many attempts to try and find a free object in the specified time
  • max_timeout: the maximum time in seconds to try and connect after which the normal functionality of the method will occur.

This is in response to issue #519

… instead of max time and max attempts. Also cleaned up the function to try removing any that have idles for too long
…t we are checking agains if an object has been idling for too long
@JoJo10Smith
Copy link
Author

JoJo10Smith commented Aug 19, 2023

I realized that there was a better implementation which I will explain below:

  • Get the current_time and set an end_time to be the current_time + optional timeout, if timeout is not given then set the end_time = current_time.
  • Start the function as usual but now while the current_time <= end_time keep trying to remove any free objects that have idled for too long.
  • If a free object is then return obj, if not then get the current_time again, if it is before the end_time try removing free objects again.
  • Once the time limit is reached if there are no free objects (because the while loop has not been broken) we create one, if there are too many raise a RuntimeError. else return obj

I look forward to hearing any feedback/ responding to questions
Thanks
Jordan

In response to #519

@JoJo10Smith JoJo10Smith changed the title Adding initial commit for the bloacking functionality in the get func… Adding initial commit for the blocking functionality in the get func… Aug 30, 2023
Copy link
Contributor

@jogo jogo left a comment

Choose a reason for hiding this comment

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

thanks for the PR, a few inline comments and once those are sorted out better inline documentation, unit tests and updating the changelog would be great.

self.release(obj)

def get(self):
def get(self, **options):
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not use keyword arguments with defaults here? That and a docstring will make this feature easier to find and documented in our code docs.


if self._after_remove is not None:
self._after_remove(obj)
while current_time <= end_time:
Copy link
Contributor

Choose a reason for hiding this comment

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

if I understand this correctly this results in busy waiting all while holding a lock. Which seems like a behavior we don't want to introduce.

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.

2 participants