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

Avoid locking on failed operations #488

Merged
merged 7 commits into from
Aug 14, 2023
Merged

Avoid locking on failed operations #488

merged 7 commits into from
Aug 14, 2023

Conversation

khaeru
Copy link
Member

@khaeru khaeru commented Jul 27, 2023

The PR adds a new context manager .utils.discard_on_error() that can be used to avoid leaving scenarios in a locked state. This can occur when exceptions are raised and JDBCBackend is in use: an unclean exit means the database is left in a state that marks the scenario as "locked" for use by a particular user.

Originally developed for iiasa/message_data#446.

How to review

  • Read the diff and note that the CI checks all pass.
  • Read the additions to the docs including examples and ensure they are clear.
  • Optionally, try a particular code operation known to fail and leave scenario(s) in a locked state.

PR checklist

  • Continuous integration checks all ✅
  • Add or expand tests; coverage checks both ✅
  • Add, expand, or update documentation.
  • Update release notes.

@khaeru khaeru added the enh New features & functionality label Jul 27, 2023
@khaeru khaeru added this to the 3.8 milestone Jul 27, 2023
@khaeru khaeru self-assigned this Jul 27, 2023
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #488 (246be1b) into main (4274880) will increase coverage by 0.0%.
The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main    #488   +/-   ##
=====================================
  Coverage   98.5%   98.5%           
=====================================
  Files         42      42           
  Lines       4506    4557   +51     
=====================================
+ Hits        4442    4493   +51     
  Misses        64      64           
Files Changed Coverage Δ
ixmp/core/timeseries.py 98.7% <100.0%> (+<0.1%) ⬆️
ixmp/testing/__init__.py 95.9% <100.0%> (+<0.1%) ⬆️
ixmp/tests/core/test_timeseries.py 100.0% <100.0%> (ø)
ixmp/tests/test_utils.py 100.0% <100.0%> (ø)
ixmp/utils/__init__.py 98.6% <100.0%> (+<0.1%) ⬆️

@khaeru khaeru force-pushed the enh/avoid-lock branch 2 times, most recently from d3dcb4e to 4a0f835 Compare August 8, 2023 10:28
khaeru added a commit that referenced this pull request Aug 8, 2023
@khaeru khaeru requested a review from a team August 8, 2023 11:21
Copy link
Member

@gidden gidden left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @khaeru. Small change requested for future developers.

ixmp/core/timeseries.py Outdated Show resolved Hide resolved
@gidden
Copy link
Member

gidden commented Aug 14, 2023

I think the force push means I can't access my review to change to accept?

Copy link
Member

@gidden gidden left a comment

Choose a reason for hiding this comment

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

lgtm

@gidden
Copy link
Member

gidden commented Aug 14, 2023

Ok, was able to do a new review - thanks @khaeru !

@khaeru khaeru merged commit ddb50fc into main Aug 14, 2023
19 checks passed
@glatterf42 glatterf42 deleted the enh/avoid-lock branch August 14, 2023 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enh New features & functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants