-
Notifications
You must be signed in to change notification settings - Fork 32
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Add PostGIS support #634
Add PostGIS support #634
Conversation
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.
Mostly looks good. Feel free to address any of my comments that can addressed quickly, but simply adding a comment or a GH issue to come back to them later would also be an acceptable response.
(And obviously get the GHA checks passing)
pass | ||
|
||
|
||
class ExplorerAbstractIndex(ABC): |
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.
Eventually it would be nice to use this (and the equivalent abstract class in OWS) as a checklist of functionality to be brought into the core index API.
It would therefore be nice if all these methods could have clear docstrings. Probably not worth holding up the 1.9 release over though.
@@ -100,7 +102,9 @@ def odc_db(postgresql_server, tmp_path_factory, request): | |||
tmp_path_factory.mktemp("odc") / "test_datacube.conf" | |||
) | |||
config = configparser.ConfigParser() |
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.
With the new config API, it feels like there should be a simpler way to handle test config than dynamically generating a temporary ini file.
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.
Yeah, this whole setup is copypasta from odc-tools and needs to be revisited
Create the
ExplorerIndex
class to enable PostGIS index support.📚 Documentation preview 📚: https://datacube-explorer--634.org.readthedocs.build/en/634/