Skip to content

Conversation

p-mongo
Copy link
Contributor

@p-mongo p-mongo commented Jul 19, 2019

No description provided.

@p-mongo p-mongo changed the title RUBY-1884 Always wait for background threads to finish when closing resources RUBY-1884 time out background thread waits after about 10 seconds Jul 19, 2019
@p-mongo p-mongo requested review from HanaPearlman and saghm July 19, 2019 21:52
# @param [ Hash ] options The options.
#
# @option options [ Logger ] :logger A custom logger to use.
def initialize(pool, options = {})
@pool = pool
@options = @pool.options
Copy link
Contributor

Choose a reason for hiding this comment

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

This line can be deleted

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I seem to have missed this, what options did the populator use before?

Copy link
Contributor

Choose a reason for hiding this comment

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

The populator includes Loggable, so that’s what the options were used for before. Now that the Background Thread includes Loggable, I think the “includes Loggable” line also can be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay removed that too.

end

let(:populator) do
described_class.new(pool, pool.options)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be doing some kind of clean up for the populator after the test is run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is probably a good idea.

@p-mongo
Copy link
Contributor Author

p-mongo commented Jul 23, 2019

Rebased to fix the test suite.

@p-mongo p-mongo merged commit 6e4d542 into mongodb:master Jul 24, 2019
@p-mongo p-mongo deleted the 1884 branch July 24, 2019 17:18
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.

4 participants