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

Custom JSON encoders and decoders #131

Merged
merged 1 commit into from
Jan 31, 2020
Merged

Custom JSON encoders and decoders #131

merged 1 commit into from
Jan 31, 2020

Conversation

elemoine
Copy link
Contributor

@elemoine elemoine commented Jan 24, 2020

Cf. #99 (and #108 for the preliminary work on this).

This PR makes it possible for the user to provide custom JSON encoders and decoders.

The custom encoder and decoder are provided as JSON dumps and loads functions on the App object.

Here's an example dealing with datetime objects:

import functools
import json

from procrastinate import App, PostgresJobStore

# Function used for encoding datetime objects
def encode(obj):
    if isinstance(obj, datetime.datetime):
        return obj.isoformat()
    raise TypeError()

dumps = functools.partial(json.dumps, default=encode)

# Function used for decoding datetime objects
def decode(dict_):
    if "dt" in dict_:
        dict_["dt"] = datetime.datetime.fromisoformat(dict_["dt"])
    return dict_

loads = functools.partial(json.loads, object_hook=decode)

app = App(job_store=PostgresJobStore(), dumps=dumps, loads=loads)

I think the approach provides a maximum of flexibility to the users. They can use the json module the way they want. They can even use a different JSON implementation than json, such as UltraJSON, if they want to.

I am creating this PR for discussion. The code in my branch (attached to this PR) basically works and is tested, although there's still work to do (typing, documentation, more tests maybe, etc.).

Any feedback?

Successful PR Checklist:

  • Tests
  • Documentation (optionally: run spell checking)
  • Had a good time contributing? (if not, feel free to give some feedback)

@codecov-io
Copy link

codecov-io commented Jan 24, 2020

Codecov Report

Merging #131 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #131   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          19     19           
  Lines         736    743    +7     
  Branches       70     71    +1     
=====================================
+ Hits          736    743    +7
Impacted Files Coverage Δ
procrastinate/store.py 100% <100%> (ø) ⬆️
procrastinate/cli.py 100% <100%> (ø) ⬆️
procrastinate/aiopg_connector.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d3601e...793ff64. Read the comment docs.

@DainDwarf
Copy link
Contributor

DainDwarf commented Jan 24, 2020

I was wondering, would it be better to store the functions in the job_store rather than the app? I think it would reduce passing around those functions through all the wrapping, and keep the code cleaner. And imo it makes sense that "encode/decoding functions" are configured to the instance of what is dealing with matching jobs on postgresql.

Taking your example, for the user point of view, you would only need to change the last line to

app = App(job_store=PostgresJobStore(dumps=dumps, loads=loads))

@elemoine
Copy link
Contributor Author

I was wondering, would it be better to store the functions in the job_store rather than the app?

Great suggestion! I'll look into it…

@elemoine elemoine force-pushed the ele_json branch 5 times, most recently from 9660bc9 to edf18f2 Compare January 31, 2020 15:47
@elemoine elemoine requested a review from CorBott as a code owner January 31, 2020 15:47
@elemoine elemoine force-pushed the ele_json branch 2 times, most recently from 2ae6b4a to 0e24f8e Compare January 31, 2020 15:55
@elemoine elemoine changed the title [WIP] Custom JSON encoders and decoders Custom JSON encoders and decoders Jan 31, 2020
@elemoine elemoine merged commit 4e05069 into master Jan 31, 2020
@elemoine elemoine deleted the ele_json branch January 31, 2020 16:15
@ewjoachim ewjoachim mentioned this pull request Feb 14, 2020
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