Skip to content

[Feature Request] Allow custom dehydration functions (and maybe hydration functions?) #576

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

Closed
havardthom opened this issue Aug 30, 2021 · 5 comments

Comments

@havardthom
Copy link

Would love a way to pass custom dehydration functions through config when starting the driver, this would give more flexibility for passing different python data types as parameters without having to convert each case separately. We currently monkeypatched your DataDehydrator as a workaround, to support UUID's and ObjectId's:

self.dehydration_functions.update({
    UUID: lambda x: str(x),
    ObjectId: lambda x: str(x)
})

Having this as a built-in feature would of course be preferable over monkeypatching.

I've also considered hydration functions but I see that this could affect performance, since the types are not known beforehand and every string property would have to be checked. Currently we convert returned ID's back to UUID's for each query separately.

@robsdedude
Copy link
Member

Thanks for the input.

What's the advantage of this over a thin wrapper around the driver's functions to dehydrate the objects before passing them to the driver. Something along the lines of

def custom_dehydration(obj):
    if isinstance(obj, UUID):
        return str(obj)
    elif isinstance(obj, ObjectId):
        return str(obj)
    return obj

def tx_run(tx, query, **parameters):
    parameters = {k: custom_dehydration(v) for k, v in parameters.items()}
    tx.run(query, **parameters)

@havardthom
Copy link
Author

A wrapper function would also have to handle deep objects so it would have to be recursive. It would basically be a smaller version of the driver's dehydrate_ function:

def dehydrate_(obj):

A wrapper feels unnecessary when the logic is already in the driver and it is easily scalable, also it would mean passing through all parameters twice instead of once in the driver.

@robsdedude
Copy link
Member

Thanks again for the input. We've put the idea up on our internal Trello wall for future work https://trello.com/c/NajZu4P0. We'll come back to this issue as soon as we've made a decision of either rejecting it or putting it on the road-map for a future release.

@havardthom
Copy link
Author

With #881 including third party type serialization and not returning the same type in response, have you considered adding the following types?

  • set
  • uuid.UUID
  • bson.ObjectId

I can make a seperate feature request if you'd like

@robsdedude
Copy link
Member

robsdedude commented Jan 5, 2023

Thanks for the feedback!

I considered set and figured it was a bad idea as there is nothing like that on the DBMS side. So I'd have to turn the set into a list and then it would behave quite differently on the server side. That felt odd. Similarly, the other types don't have anything that matches them on the server side. Best I could do is turn them into strings. But I'd argue that's a very opinionated mapping. The custom hydration/dehydration hooks still come up in internal discussions every now and then. Should they make it into the API, users would be able to define these opinionated transformations as they see fit for their application. I think that'd be a much better solution than guessing what could suit the user's needs.

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

No branches or pull requests

2 participants