Skip to content

Adds Date retyping for GeoTools Vector ingest (#468)#470

Merged
rfecher merged 1 commit intomasterfrom
GEOWAVE-468
Jul 21, 2015
Merged

Adds Date retyping for GeoTools Vector ingest (#468)#470
rfecher merged 1 commit intomasterfrom
GEOWAVE-468

Conversation

@ewilson-radblue
Copy link
Copy Markdown
Contributor

Adds Date retyping for GeoTools Vector ingest (#468). To support it, adds CompoundIngestFormatOptionProvider to enable multiple providers for a single ingest plugin.

@chrisbennight
Copy link
Copy Markdown
Contributor

So the failure is valid - it's coming from changes in geotools between 12.x and 13.x+; the SimpleFeatureTypeBuilder class had a bunch of set(...) methods added here: geotools/geotools@33e9de5#diff-69cbda95a5fcc79676d0c12b73dd3f21
the diff makes use of one of those methods here:
https://github.com/ngageoint/geowave/blob/GEOWAVE-468/extensions/formats/geotools-vector/src/main/java/mil/nga/giat/geowave/format/geotools/vector/retyping/date/DateFieldRetypingSource.java#L66

and the first profile uses 12.4 of geotools, so the method isn't found.

Now what to do about this; typically we have been looking to stick with N and N-1. 14.x isn't out yet. I looking at what got added in the commit above though, and what was supported in 12.x I don't see a super clean way to do this (since attributes is protected, and there's no accessor) - but I just glanced it it real quick.

Dropping 12.x support a bit early isn't out of the question - just want to get a double check and make sure there's not an easy way to maintain it. @rfecher

@rfecher
Copy link
Copy Markdown
Contributor

rfecher commented Jul 18, 2015

that is kinda annoying - I think there should be a much less elegant way to set an attribute descriptor in 12.x that will also work in 13+ (I think looping over the attributes and just adding the attribute descriptor with SimpleFeatureTypeBuilder.add() if its not the date field and adding the new descriptor if it is the date field). But they are close to releasing v14.0 so my vote is that we drop support for 12.x in our next minor release (0.8.9). We are releasing 0.8.8.1 on Monday with some bug fixes and Eric's other commit can make it into that but we'll hold off on this commit (so 0.8.8.1 will be the last one with geotoools 12.x support).

@ewilson-radblue
Copy link
Copy Markdown
Contributor Author

I actually had it at first where it would loop through the AttributeDescriptors of the initial type, re-add them if they weren't the ones marked to replace, and replace them if need be. It worked but I thought this solution was more concise.

I can switch back to that if it would be preferable to also maintain GeoTools 12.x support.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would trim() the substring to be safe from whitespace

…adds CompoundIngestFormatOptionProvider to enable multiple providers for a single ingest plugin.
@ewilson-radblue
Copy link
Copy Markdown
Contributor Author

When convenient, I think it's ready to be merged. I squashed my changes into one commit, switched back to support GeoTools 12.x, and addressed the code comments. Thanks!

rfecher added a commit that referenced this pull request Jul 21, 2015
Adds Date retyping for GeoTools Vector ingest (#468)
@rfecher rfecher merged commit 8cc3c96 into master Jul 21, 2015
@rfecher rfecher deleted the GEOWAVE-468 branch July 21, 2015 12:34
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.

4 participants