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

Support for MongoDB Decimal type #1045

Closed
theothermatt opened this Issue Aug 11, 2017 · 10 comments

Comments

Projects
None yet
4 participants
@theothermatt
Copy link

theothermatt commented Aug 11, 2017

This is a feature request for support for the newly added MongoDB Decimal type:

https://docs.mongodb.com/master/release-notes/3.4/#decimal-type

I'm sure you've got plenty of things to do, I just wanted to make sure it was on the radar!

Thanks,

Matt

@Amedeo91

This comment has been minimized.

Copy link

Amedeo91 commented Aug 11, 2017

Hi,

how do you need this feature to be supported? By default or with some schema set-up?

Regards,
Amedeo

@Amedeo91

This comment has been minimized.

Copy link

Amedeo91 commented Aug 11, 2017

@nicolaiarocci should it be a default behavior or should it be a set-up in same places of the schema?

This is how to support it: https://api.mongodb.com/python/3.4.0/api/bson/decimal128.html

@theothermatt

This comment has been minimized.

Copy link
Author

theothermatt commented Aug 14, 2017

@Amedeo91 not quite sure what you mean by 'default or with some schema set-up', but I guess I had pictured it as being done by a 'type': 'decimal' line in the schema.

A nice extra would be the ability to validate based on the number of decimal places. For example, reject a value with more than 2 decimal places. Of course, this could be done with custom validation rules.

@Amedeo91

This comment has been minimized.

Copy link

Amedeo91 commented Aug 14, 2017

For the second point, I think it is more a specific need, hence it might be customized by the framework user.

For the first point, I was referring that Eve might store every number as decimal type in Mongo or either to specify a new type in the schema. @nicolaiarocci what would you suggest?

@Amedeo91

This comment has been minimized.

Copy link

Amedeo91 commented Aug 14, 2017

Ok, I just did a small analysis: it would be enough to add the lamda function in the serializers dictionary inside the Mongo class in the file eve\io\mongo\mongo.py
However, then we have to update Cerberus aswell to validate the data type. For it, I think it should be enough to update the file validator.py adding the method def _validate_type_decimal(self, value). @nicolaiarocci : do you think there should be anything else to do? Maybe we should start to add this feature in Cerberus and after in Eve?

@nicolaiarocci

This comment has been minimized.

Copy link
Member

nicolaiarocci commented Aug 19, 2017

@Amedeo91 On second thought (see you PR on Cerberus), couldn't you simply update Eve's custom validator instead of updating Cerberus core? It is true that Decimal support out of Eve's scope is nice but, in fact, there have been no requests for that in Cerberus yet.

WIth a single Eve PR we could add validation and serialization for Decimals. Thoughts?

@Amedeo91

This comment has been minimized.

Copy link

Amedeo91 commented Aug 19, 2017

@nicolaiarocci, I was thinking the same thing this morning! :)

@Amedeo91

This comment has been minimized.

Copy link

Amedeo91 commented Aug 19, 2017

I will try to do the PR this days! :)

@funkyfuture

This comment has been minimized.

Copy link
Member

funkyfuture commented Aug 20, 2017

and i didn't dare to propose. :-)

@nicolaiarocci nicolaiarocci added this to the 0.8 milestone Aug 28, 2017

@nicolaiarocci

This comment has been minimized.

Copy link
Member

nicolaiarocci commented Aug 29, 2017

It's merged to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.