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

Update cache id when object changes #72

Closed
wants to merge 1 commit into from

Conversation

geographika
Copy link

Fix for #61 and #71. Uses the uuid to ensure unique keys are used in the _walk_refs _processed dictionary.

@gazpachoking
Copy link
Owner

How do we generate the same uuid for the same object each time? I think this essentially will disable the cache.

@geographika
Copy link
Author

Sorry - I got carried away thinking I had a fix..! Yes this bypasses the cache completely.
Maybe a wrapper class with a uuid, obj, and id could be used, I'll experiment further.

@gazpachoking
Copy link
Owner

Hmm. I forget exactly how this code is working, but perhaps we only need to remember the result when it's a JsonRef object. In that case possibly the full_uri is good for the cache key.

@geographika
Copy link
Author

@gazpachoking I've tried a few approaches but still sometimes get inconsistent outputs. I thought updating the id when the object changes might help, but when running multiple pytest cases there were failures. It can be hard to recreate as adding a print statement can make tests pass successfully.

I did try without any caching with a simple timeit function and there was very little difference. I'm not sure if that would just be for my specific use case (loading https://github.com/geographika/mappyfile/blob/master/mappyfile/schemas/map.json).

"""
1000 loops

With cache:

0.002243063199974131 (each run)
2.243063199974131 (total time)

Without cache:

0.0023388927999767476
2.3388927999767475
"""
import timeit

N = 1000

t = timeit.timeit(
    stmt="json.dumps(validator.get_versioned_schema(8.2, 'map'))",
    setup="from mappyfile.validator import Validator; import json; validator = Validator()",
    number=N,
)

@gazpachoking
Copy link
Owner

The cache isn't actually to speed things up, it's to prevent infinite recursion. If there is a recursive structure created by a json reference, we need to create the recursive structure in python, rather than recurse infinitely.

@geographika geographika changed the title Use uuid as cache key instead of id Update cache id when object changes Nov 6, 2024
@geographika
Copy link
Author

@gazpachoking the latest changes fix my issues, and pass the test suite (test_recursive_extra was failing in previous attempts).
I think the issue was id was used as a key for an object that was then changed to a dict. Creating a new key for the replaced object, and also adding this pair to the cache seems to produce the correct outputs for the test suite, my test case, and the test suite of the project I'm using jsonref in.

@geographika
Copy link
Author

Fixed with e827f23 - closing this.

@geographika geographika closed this Feb 2, 2025
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.

2 participants