Add ability to use Decimal for DynamoDB numeric types #1183

Merged
merged 10 commits into from Jan 8, 2013

Projects

None yet

3 participants

@jamesls
the boto project member

This expands on the work of @disruptek based on the comments in #1060.

The gist of this change is to refactor the
serialization/deserialization of python types to the format
that the dynamodb expects into a single "Dynamizer" interface.

This interface is then plumbed into Layer2 and can be passed in
via the __init__. This also allows users to entirely override
this process if they need.

I implemented two types of Dynamizers:

  • Dynamizer - Uses Decimals to handle numeric types.
  • LossyFloatDynamizer - Uses int/float to handle numeric types.

I made the default dynamizer the LossyFloatDynamizer which will
maintain backwards compatibility (all of the dynamodb tests pass).
I imagine (hope) after some time we can default to Dynamizer
instead.

If a user wants to use decimals, they can either pass in the
appropriate dynamizer:

  dynamodb = boto.connect_dynamodb(dynamizer=Dynamizer)

or use the use_decimals convenience method::

  dynamodb = boto.connect_dynamodb()
  dynamodb.use_decimals()

If a user wants to customize this process, they can subclass
Dynamizer and either override encode or decode, or they
can override a specific conversion. For example:

  class UnicodeDynamizer(Dynamizer):
    def decode_s(self, attr):
        return unicode(attr)

  dynamodb = boto.connect_dynamodb(dynamizer=UnicodeDynamizer)

I've added unittests/integration tests for the new functionality.

If this approach looks reasonable I can update the user docs, API docstrings,
etc. for the new changes. Just looking for feedback first.

@garnaat, @disruptek thoughts?

disruptek and others added some commits Oct 15, 2012
@disruptek disruptek use Decimal for numeric validate/store/retrieve 2e6936c
@disruptek disruptek raise an exception on NaN and Infinity decimal values 6158a69
@jamesls jamesls Minor pep8 formatting 20808a1
@jamesls jamesls Add toggle to use Decimals with DynamoDB Layer2
The gist of this change is to refactor the
serialization/deserialization of python types to the format
that the dynamodb expects into a single "Dynamizer" interface.

This interface is then plumbed into Layer2 and can be passed in
via the __init__.  This also allows users to entirely override
this process if they need.

I implemented two types of Dynamizers:

* Dynamizer - Uses Decimals to handle numeric types.
* LossyFloatDynamizer - Uses int/float to handle numeric types.

I made the default dynamizer the LossyFloatDynamizer which will
maintain backwards compatibility (all of the dynamodb tests pass).

If a user wants to use decimals, they can either pass in the
appropriate dynamizer:

  dynamodb = boto.connect_dynamodb(dynamizer=Dynamizer)

or use the `use_decimals` convenience method::

  dynamodb = boto.connect_dynamodb()
  dynamodb.use_decimals()

If a user wants to customize this process, they can subclass
Dynamizer and either override `encode` or `decode`, or they
can override a specific conversion.  For example:

  class UnicodeDynamizer(Dynamizer):
    def decode_s(self, attr):
        return unicode(attr)

  dynamodb = boto.connect_dynamodb(dynamizer=UnicodeDynamizer)

I've added unittests/integration tests for the new functionality.
6767cec
@jamesls jamesls Fix deprecated warning in python2.6 2195b8b
@jamesls
the boto project member

Well, it turns out you can't directly convert floats to decimals in python2.6 (this is the failing test). There's a recipe in the FAQ section for the decimal module that I'll try out.

@jamesls jamesls Fix python2.6 bug converting floats to Decimals
tox is now happy with all the unittests.
5211685
@disruptek

Thanks for working on this; this issue has been gnawing at me for awhile now. Since your implementation matches my original proposal from 4 months ago, of course I'm in favor of it. :-)

I have only a couple comments on the code:

  • You suggested in #1060 that in Dynamize.encode_n() we narrow the exception handling to the TypeError we raise therein or to an appropriate Decimal exception. That change makes sense to me, did you simply overlook it?
  • It seems to me that Dynamize._get_dynamodb_type() shouldn't be _private, as it will probably need to be reimplemented in most subclasses and as such represents an interface for the user. Maybe I'm overlooking a key detail, but it's hard to think of a way to avoid reimplementing this method in any Real World subclass. If the point of the _private designation is to suggest that it will not be called outside the class, then I wonder if all the encode_X/decode_X methods should be similarly _private.
@jamesls jamesls Incorporate review feedback from disruptek
* Don't catch Exception, catch more specific exceptions
  when converting to Decimal.
* Mark the encode_X/decode_X methods as internal (client should
  use encode/decode which will delegate to the proper
  encode_X/decode_X method.
2ef10ba
@jamesls
the boto project member
  • I overlooked that, I've updated the code accordingly.
  • My intent with marking the methods _internal with the leading underscore is that it's not intended to be called outside the class. I wanted a simple public API with just two methods encode and decode. Subclasses are free to call these methods in any way they need. So, as per your suggestion, I've changed the encode_X/decode_X methods as internal.

Great feedback, thanks for reviewing the PR.

@jamesls jamesls Add an integration test for large integers
Noticed #1176 mentioned large integers so I figured
added an explicit integration test for this wouldn't be
a bad idea.
6f9ff24
@jamesls
the boto project member

Ok, so it sounds like I'll go ahead and flesh the rest of this change out (which I believe is mostly just docstring/comments and possibly an update to the dynamodb tutorial).

@garnaat Any thoughts?

@jamesls jamesls Add docs for dynamodb decimal changes
* Added docstrings
* Updated tutorial with section on using decimals
* Added boto.dynamodb.types to the list of modules
  for which to generate API docs.
7972dda
@jamesls jamesls Merge branch 'develop' into dynamodb-decimal
Conflicts:
	docs/source/dynamodb_tut.rst
d6f853a
@jamesls jamesls merged commit d6f853a into boto:develop Jan 8, 2013
@jamesls jamesls deleted the jamesls:dynamodb-decimal branch Jan 9, 2013
@mwaterfall

Is there a reason that Decimal is being used for both whole and fractional numbers? As opposed to using int for integers and only replacing floats with Decimal?

Currently, when pulling items from DynamoDB using Decimals entirely, it's not possible for me to determine which fields/columns are integers and which ones are fractional.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment