GEOWAVE-1812: updated major dependencies to current versions#1813
GEOWAVE-1812: updated major dependencies to current versions#1813
Conversation
1d1afd7 to
42861a5
Compare
| file = "win-x64-gdal204.zip"; | ||
| url = new URL(gdalEnv + "/windows/MSVC2017/" + file); | ||
| } else if (isMac()) { | ||
| file = "gdal-1.9.2_macOSX.zip"; |
There was a problem hiding this comment.
Shouldn't the mac version also be updated?
There was a problem hiding this comment.
it may be best to stick with homebrew on the mac, although I spent awhile making sure this worked at least under windows (for ease with our standalone installer) and ubuntu 20.04 (for CI tests), although I don't think it works across the board on linux even. I don't have any options for testing on a mac so I'd not be the best person to try to get it working for mac osx. The older version seemed very specific with an older GDAL install which really pushed us toward integrating the install with our CLI for ease of use, but this should be good for GDAL 2.2+ (any recent version) so at some point we could just decide to ditch the install gdal CLI. Also, technically, GeoTools actually picks up on the older GDAL version, but our tests don't pass with some method not found exceptions (so its probably not very functional, might work for some things though for GeoTools to even attempt to use it?).
| } | ||
|
|
||
| @Override | ||
| public void close() { |
There was a problem hiding this comment.
If AccumuloOperations nees to be closeable, we may need to propagate that to the base DataStoreOperations interface so that we don't leak resources when using the operations from within GeoWave. Unless this is being handled in some other way that I'm not seeing?
There was a problem hiding this comment.
its similar to how RocksDB works, the DataStore is closeable as well and additionally we do call it from GeoTools' DataStore's dispose() method. In both cases we use a static cache for connections so there is no sense in opening and closing anything on a per instance basis, just at the end of the JVMs usage (but when the JVM exits I'd bet resources are released as well)
| // nothing to do | ||
| } | ||
|
|
||
| public static void main(final String[] args) throws Exception { |
There was a problem hiding this comment.
This looks like left over test code that should probably be removed.
| """ | ||
| return self._base_options.getReplicationFactor() | ||
|
|
||
| def set_gc_grace_seconds(self, gc_grace_seconds): |
There was a problem hiding this comment.
These gc_grace_seconds methods aren't being covered in the test.
| public static void startTimer() { | ||
|
|
||
| // GDAL version >= 2.3.0 | ||
| System.loadLibrary("gdalalljni"); |
There was a problem hiding this comment.
I am curious as to why gdal needs to be explicitly loaded here. Does that mean that third party developers need to explicitly load gdal when using those capabilities within GeoWave?
There was a problem hiding this comment.
nope, another left-over from fighting the tests, I'll delete
No description provided.