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

Add the ability for pynamo to read + write map and list attributes #146

Closed
wants to merge 88 commits into from
Closed

Add the ability for pynamo to read + write map and list attributes #146

wants to merge 88 commits into from

Conversation

jmphilli
Copy link
Contributor

@jmphilli jmphilli commented Jul 29, 2016

Context
This allows pynamo to serialize + deserialize map and list types.

SHORT_ATTR_TYPES = [STRING_SHORT, STRING_SET_SHORT, NUMBER_SHORT, NUMBER_SET_SHORT, BINARY_SHORT, BINARY_SET_SHORT]
MAP = 'Map'
LIST = 'List'
SHORT_ATTR_TYPES = [STRING_SHORT, STRING_SET_SHORT, NUMBER_SHORT, NUMBER_SET_SHORT, BINARY_SHORT, BINARY_SET_SHORT, MAP_SHORT]
Copy link

Choose a reason for hiding this comment

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

LIST_SHORT missing here

"""
rval = []
for v in values:
if v is None:
Copy link
Member

Choose a reason for hiding this comment

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

I'm sensing a pattern with all of these.. surely we can write this in a generic way such that we don't need to do the type checks every time?

@@ -257,3 +258,181 @@ def deserialize(self, value):
Takes a UTC datetime string and returns a datetime object
"""
return parse(value, dayfirst=False).datetime


class MapAttribute(Attribute):
Copy link

Choose a reason for hiding this comment

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

so when I want to define an MapAttribute that requires verification, I need to create a subclass, otherwise I can use MapAttribute directly, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bingo. for our user case we would just want to do

class CouponTemplate(MapAttribute):
    code = UnicodeAttribute()
    ...

@coveralls
Copy link

coveralls commented Sep 12, 2016

Coverage Status

Changes Unknown when pulling da0246a on jmphilli:nested-validation into * on jlafon:devel*.

return all(self.is_type_safe(k, v) for k, v in six.iteritems(self._get_attributes()))

def serialize(self, values):
rval = dict()
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: consistency use literal {}

@coveralls
Copy link

coveralls commented Sep 12, 2016

Coverage Status

Coverage increased (+0.2%) to 97.683% when pulling d12a1ce on jmphilli:nested-validation into 5229759 on jlafon:devel.

@coveralls
Copy link

coveralls commented Sep 12, 2016

Coverage Status

Coverage increased (+0.2%) to 97.683% when pulling d12a1ce on jmphilli:nested-validation into 5229759 on jlafon:devel.

@coveralls
Copy link

coveralls commented Sep 13, 2016

Coverage Status

Coverage decreased (-0.02%) to 97.477% when pulling 8bd8601 on jmphilli:nested-validation into 5229759 on jlafon:devel.

@coveralls
Copy link

coveralls commented Sep 13, 2016

Coverage Status

Coverage increased (+0.2%) to 97.666% when pulling 9759dcd on jmphilli:nested-validation into 5229759 on jlafon:devel.

@jlafon
Copy link
Contributor

jlafon commented Sep 14, 2016

LGTM!

@coveralls
Copy link

coveralls commented Sep 14, 2016

Coverage Status

Coverage increased (+0.2%) to 97.666% when pulling 81cf2e6 on jmphilli:nested-validation into 5229759 on jlafon:devel.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 97.681% when pulling 61275c7 on jmphilli:nested-validation into 5229759 on jlafon:devel.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.2%) to 97.681% when pulling 61275c7 on jmphilli:nested-validation into 5229759 on jlafon:devel.

@coveralls
Copy link

coveralls commented Sep 14, 2016

Coverage Status

Coverage increased (+0.2%) to 97.681% when pulling 61275c7 on jmphilli:nested-validation into 5229759 on jlafon:devel.

@jmphilli
Copy link
Contributor Author

Merged into lyft going to do an RC and then merge into jlafon from there

@jmphilli jmphilli closed this Sep 14, 2016
@jmphilli jmphilli deleted the nested-validation branch September 14, 2016 23:08
@jmphilli jmphilli restored the nested-validation branch October 24, 2016 17:06
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.

None yet

7 participants