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

Key pickling 2.0: reverse unused #31

Closed
wants to merge 11 commits into from

Conversation

jquast
Copy link
Contributor

@jquast jquast commented Apr 26, 2015

This builds on PR #30, retuning coverage to 100% and removing unused traceback handlers.

  • spellfix compatibile => compatible
  • test_sqlite_migration.py was writing files to ./test/db, not ./tests/db.
  • use sys.version_info < (3, 0) rather than tracking _major_version.
  • Introduce 'unittest2' as development dependency, removing the TestCaseBackport class.
  • No longer necessary to use 'reraise', track inner or outer stack, as the next release of sqlitedict always pickles and there is no known exception to report.
  • SqliteDict.str depended on self.conn.filename always being set, but this is not necessarily True, (esp. in test case teardown due to del).
  • add missing test coverage for throwing KeyError when any key or value cannot be unpickled.

ziky90 and others added 11 commits April 17, 2015 18:49
- spellfix compatibile => compatible
- test_sqlite_migration.py was writing files to ./test/db, not ./tests/db.
- use sys.version_info < (3, 0) rather than tracking _major_version.
- Introduce 'unittest2' as development dependency, so that
  we can remove the TestCaseBackport class.
- No longer necessary to use 'reraise', track inner or outer
  stack, as the next release of sqlitedict always pickles
  and there is no exception to handle
- SqliteDict.__str__ depended on self.conn.filename always being set,
  but this is not necessarily True, esp. in test case teardown
  due to __del__.
- add missing test coverage for throwing KeyError when any key (or
  value?!) cannot be unpickled.
@piskvorky
Copy link
Owner

@jquast I see the exception handling code has been removed in this PR. How vital was the code? Is it needed or not?

I'd obviously prefer to have it removed, it obfuscates the code greatly. But I don't remember how critical it is, so I'm afraid to touch it. Can you comment?

(I'm only talking about the inner/out stacks, not key encoding here)

@menshikh-iv
Copy link
Contributor

Hi @jquast @ziky90, are you planning to continue this work?

@menshikh-iv menshikh-iv mentioned this pull request Oct 30, 2017
@menshikh-iv
Copy link
Contributor

This PR looks abandoned, for this reason, I close it.

@piskvorky piskvorky mentioned this pull request Mar 8, 2018
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