Skip to content

Fix #382 - deprecate ignore_errors and fix errorneous skip_duplicates behavior#383

Merged
dimitri-yatsenko merged 4 commits intodatajoint:masterfrom
eywalker:master
Oct 24, 2017
Merged

Fix #382 - deprecate ignore_errors and fix errorneous skip_duplicates behavior#383
dimitri-yatsenko merged 4 commits intodatajoint:masterfrom
eywalker:master

Conversation

@eywalker
Copy link
Contributor

@eywalker eywalker commented Oct 24, 2017

Fix #382

  • ignore_errors in insert is now deprecated - errors are no longer suppressed with this option and warnings are returned notifying users of this deprecation
  • skip_duplicates=True no longer suppresses other errors
  • Add simple test case to detect side effect due to skip_duplicates=True
  • Known MySQL errors are re-represented as DataJoint errors with helpful error messages

@eywalker eywalker changed the title Fix #382 - remove ignore_errors Fix #382 - deprecate ignore_errors and fix errorneous skip_duplicates behavior Oct 24, 2017
@coveralls
Copy link

coveralls commented Oct 24, 2017

Coverage Status

Coverage decreased (-0.1%) to 91.937% when pulling 5038f61 on eywalker:master into 54cd18f on datajoint:master.

@ixcat
Copy link

ixcat commented Oct 24, 2017

looks good to me;
not comfortable enough w/codebase yet to OK unless we are going in 'sprint' mode..

warnings.simplefilter('always')
self.connection.query(*args, suppress_warnings=False, **kwargs)
for w in ws:
# 1062 is MySQL's error code for duplicated entry
Copy link
Member

Choose a reason for hiding this comment

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

This comment is redundant (should be with the server_error_codes)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, that's redundant - forgot to remove it after I switched to using server_error_codes

@dimitri-yatsenko dimitri-yatsenko merged commit 2da8a8c into datajoint:master Oct 24, 2017
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