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

Put sessionmaker code in a function #227

Merged

Conversation

stveit
Copy link
Contributor

@stveit stveit commented Nov 18, 2021

Fix for #226

The downside of this code is that the sessionmaker will be created every time sqla_session is called. If wanted, I can remedy this by setting Session as a global variable and only allowing it to be set once or something

@codecov
Copy link

codecov bot commented Nov 18, 2021

Codecov Report

Merging #227 (8c8f90e) into develop (739e933) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #227      +/-   ##
===========================================
+ Coverage    59.41%   59.44%   +0.02%     
===========================================
  Files           61       61              
  Lines         6180     6184       +4     
===========================================
+ Hits          3672     3676       +4     
  Misses        2508     2508              
Impacted Files Coverage Δ
src/cnaas_nms/db/session.py 76.92% <100.00%> (+1.92%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 739e933...8c8f90e. Read the comment docs.

@hmpf
Copy link

hmpf commented Nov 23, 2021

Fix for #226

The downside of this code is that the sessionmaker will be created every time sqla_session is called. If wanted, I can remedy this by setting Session as a global variable and only allowing it to be set once or something

It's so little code why not just do it. I assume you mean a global in the same module? (I wonder if this has a name besides "Singleton".) Though, might be useful to have a way to check that the session has not disconnected and then reconnect.. but that is probably out of scope.

@indy-independence indy-independence merged commit 51adce6 into SUNET:develop Jan 12, 2022
pboers1988 added a commit to workfloworchestrator/cnaas-nms that referenced this pull request Feb 4, 2022
pboers1988 added a commit to workfloworchestrator/cnaas-nms that referenced this pull request Feb 4, 2022
pboers1988 added a commit to workfloworchestrator/cnaas-nms that referenced this pull request Feb 24, 2022
@lunkwill42 lunkwill42 deleted the uninett.fix_session_sideeffect branch September 19, 2022 13:30
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