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

schema caching needs xmlschema version #19

Closed
tlambert03 opened this issue Jul 22, 2020 · 5 comments · Fixed by #20
Closed

schema caching needs xmlschema version #19

tlambert03 opened this issue Jul 22, 2020 · 5 comments · Fixed by #20

Comments

@tlambert03
Copy link
Owner

just so I don't forget to fix it: when testing #17 locally a few tests failed that weren't testing on CI, so I updated xmlschema to check... and they all failed. I realized that it was ultimately due to the pickled schema cache being stale. So that cache key should minimally have the xmlschema version in it, but maybe we don't cache at all.

@jmuhlich
Copy link
Collaborator

  1. We should grab the .xsd and include it with the generated python code, then just load that. That thing isn't changing so no sense in fetching it from a remote server every run. (This usually takes less than a second but it also requires a network connection)
  2. The schema parse takes over 2 seconds on my machine which I agree isn't great, but the hassle of managing the cache may not be worth it as you're also realizing...

@jmuhlich
Copy link
Collaborator

Remember there are only 2 truly hard problems in computer science: Naming things, cache invalidation, and off-by-one errors.

@tlambert03
Copy link
Owner Author

that's hilarious.

@tlambert03
Copy link
Owner Author

sounds good. I'll add the xsd to source. I think i'll leave the caching in there for now, but include the xmlschema version string and ome-types version in the key. That should cover everything but development woes, and we can deal with that ourselves

@jmuhlich
Copy link
Collaborator

jmuhlich commented Jul 22, 2020

Actually the schema parse only takes 1 second. I was accidentally timing it loading from the URL rather than a local file. I don't know if 1 second is even worth all the hassles of caching. (e.g. if it's a zipped wheel install, that path doesn't even exist for writing a file to and you need to find somewhere else...)

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 a pull request may close this issue.

2 participants