Skip to content

GEOWAVE-684: export vector data commandline tools; also seems to have…#685

Merged
rwgdrummer merged 6 commits into0.9.1from
GEOWAVE-684
Apr 4, 2016
Merged

GEOWAVE-684: export vector data commandline tools; also seems to have…#685
rwgdrummer merged 6 commits into0.9.1from
GEOWAVE-684

Conversation

@rfecher
Copy link
Copy Markdown
Contributor

@rfecher rfecher commented Mar 29, 2016

… formatted a few files that were in a conflicting format

@rfecher rfecher added this to the 0.9.1 milestone Mar 29, 2016
… formatted a few files that were in a conflicting format
@rfecher
Copy link
Copy Markdown
Contributor Author

rfecher commented Mar 30, 2016

this relates to PR #695

return -1;
}
if (index instanceof PrimaryIndex) {
options.setIndex((PrimaryIndex) index);
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.

Looking at the GeoWaveInputFormat, I think we have an issue.
I like this approach. But, it looks like GeoWaveInputFormat overrides this selection.
I think we need a separate ticket to make sure that QueryOptions is not overridden by GeoWaveInputFormat.setIndex and setAdapters. Basically, the proposal is to have getAdapters and getIndex on GeoWaveInputFormat used ONLY if the QueryOptions adapters and index are not set. Agreed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good catch - that is an odd one...your proposed approach would be an improvement, or we could just rely on QueryOptions only...do you see a reason to setIndex or setAdapters on GeoWaveInputFormat separately? My assumption is that the methods in GeoWaveInputFormat pre-date QueryOptions and that is the reason they exist separately right now. Perhaps we could leave the setter methods around and just have it set the queryoptions? Or I would consider removing those methods altogether and using queryoptions only unless there's something I'm missing?

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.

Lets remove them. I will merge this request in today. I create a separate ticket for this additional adjustment.

@rwgdrummer
Copy link
Copy Markdown
Contributor

Once you see the comment...I am happy with this PR. All good.

GEOWAVE-686: added an ingest format for avro encoded using the geowav…
@rwgdrummer rwgdrummer merged commit 4e98ac3 into 0.9.1 Apr 4, 2016
@dcy2003 dcy2003 deleted the GEOWAVE-684 branch April 5, 2016 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants