-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
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
Fixed #29484 -- Removed the need to specify SPATIALITE_LIBRARY_PATH with Spatialite 4.2+. #10017
Conversation
It looks like an improvement (no need to specify 'mod_spatialite' on Ubuntu 16.04) but I don't think it'll work on 18.04 because the exception comes from |
It works as I said in my comment. I wonder if we should instead drop support for SpatiaLite 4.1 (released June 2013) and replace |
I don't think it hurts so much to keep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're not worried about silencing a failure to load from settings.SPATIALITE_LIBRARY_PATH
in the case where one of the fallbacks works, then? If the performance penalty from calling load extension several times isn't a concern, I guess we can go with this. I would add mod_spatialite.so
to the list since that fixes the issue on Ubuntu 18.04 and prioritize mod_spatialite variants over find_library()
.
'Unable to load the SpatiaLite library extension "%s"' % self.spatialite_lib | ||
) from exc | ||
'Unable to load the SpatiaLite library extension. ' | ||
'Library names tried: %s' % ", ".join(self.lib_spatialite_paths) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
', '
(since quotes)
for path in self.lib_spatialite_paths: | ||
try: | ||
conn.load_extension(path) | ||
except Exception as exc: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove unused as exc
And now? |
Looks good. Worth trying try to add some tests? |
I'm not very motivated to add tests here, this makes coverage happy, but I don't see a real value for it. |
…ith Spatialite 4.2+. Thanks Tim Graham for the review.
https://code.djangoproject.com/ticket/29484