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

remove superfluous engine calls #50

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

jrknezha
Copy link
Collaborator

This PR removes the code that was creating multiple engines and instead uses the same engine (the one already created within the score table models file) for generating multiple sessions which is the correct desired behavior. This should address problems occurring due to connection limits being hit since the pool size of the engine was not able to be used in the old configuration.

All tests are passing. This does not change any codebase behavior, other files were just removing already unused calls to prevent any future calls to the engine instead of session.

Copy link
Collaborator

@amschne amschne 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 addressing this, @jrknezha - I'll work with Larry to smoke test our monitoring workflow against this branch to see if it improves performance

@jrknezha
Copy link
Collaborator Author

before merging this PR, we need to also make sure session.close() is being appropriately called for every possible code path where a session is opened.

@jrknezha jrknezha marked this pull request as draft September 13, 2024 00:12
@jrknezha jrknezha marked this pull request as ready for review October 17, 2024 17:27
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