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

[FIX] Select Rows: Fix crash on changed variable type #4254

Merged
merged 3 commits into from
Dec 14, 2019

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Dec 6, 2019

Issue

Fixes #3952. Widget's settings stored variable names without type, and retrieved settings that did not really match.

Description of changes

Add variable types to settings.

Previous settings are not migrated because we cannot guess variable types in migration. A possible workaround would be to skip migration and instead add specific code for handling old settings in decode_setting and match.

Settings are not migrated; what crashed will still crash, but the only way to prevent this would be to clear settings entirely, which would harm more users than letting the widget crash for the few users who have existing settings and will change variables types.

Includes
  • Code changes
  • Tests

@codecov
Copy link

codecov bot commented Dec 6, 2019

Codecov Report

Merging #4254 into master will increase coverage by <.01%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##           master    #4254      +/-   ##
==========================================
+ Coverage   86.23%   86.24%   +<.01%     
==========================================
  Files         396      396              
  Lines       70666    70714      +48     
==========================================
+ Hits        60940    60988      +48     
  Misses       9726     9726

@@ -105,6 +124,8 @@ class Outputs:
purge_classes = Setting(False, schema_only=True)
auto_commit = Setting(True)

settings_version = 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Should'n this be set to 2?

#
# @classmethod
# def migrate_context(cls, context, version):
# if not version:
Copy link
Contributor

Choose a reason for hiding this comment

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

if version is None or version < 2:

@janezd janezd force-pushed the select-rows-change-vartype branch 4 times, most recently from 0ecff3e to fa537b1 Compare December 13, 2019 13:04
@VesnaT VesnaT merged commit 9f3a965 into biolab:master Dec 14, 2019
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.

Select rows crashes on changing input var type
2 participants