Skip to content

delete pyramid levels of raster data#1631

Merged
jdgarrett merged 2 commits intolocationtech:masterfrom
rfecher:pyramid-delete
Jan 8, 2020
Merged

delete pyramid levels of raster data#1631
jdgarrett merged 2 commits intolocationtech:masterfrom
rfecher:pyramid-delete

Conversation

@rfecher
Copy link
Copy Markdown
Contributor

@rfecher rfecher commented Oct 10, 2019

No description provided.

if (isRaster(type)
&& ((coverageName == null) || coverageName.equals(adapter.getTypeName()))) {
if (adapter != null) {
LOGGER.error(
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.

There is some inconsistency with the way status/errors are being displayed to the user. In other CLI commands errors are typically reported via ParameterException, with information type text being reported via JCommander.getConsole().println. I think the error logging should be updated and perhaps the LOGGER.info parts as well, but as I said before, there seems to be some inconsistency amongst existing commands.

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.

I looked at this and I think there's a couple ways to look at it - always throw an exception on errors would be an option and let GeoWaveMain take care of handling exceptional cases, or always use JCommander.getConsole().println() although because a lot of logic in geowave commands is potentially shared with REST and gRPC services this is probably not the right approach. It seems actually we should be moving more to some generalized logging that just happens to be to JCommander console from the CLI but is logging to file for the services. Regardless, I think whatever is done, it may as well be done consistently outside of the scope of this.

statsStore.setStatistics(ovStats);
statsStore.setStatistics(pStats);

for (final ByteArray p : partitions) {
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.

Is it possible that there could be no statistics for the given coverage, but still have data that could be removed?

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.

yeah, I think the only somewhat legit case may be is statistics are disabled by the options, and I added a check for that (although with raster data functionality would be greatly diminished if stats are disabled so its probably something that just shouldn't happen, the overview stat is used to pick a resolution if you want to composite an image based on geobounds and pixel height/width, without that you would primarily be needing to query for tiles and composite them into an image externally).

Otherwise if there stats aren't found, something is wrong and it stands to reason you don't want to delete the data without properly maintaining the stats and get in a more chaotic situation. The only thing I can think of is to add a --force flag that does it anyways, but probably outside the scope here.

}

public void setParameters(final String inputStore, final String outputStore) {
parameters = new ArrayList<>();
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.

This command only expects a single parameter.

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

@jdgarrett jdgarrett merged commit 53354d5 into locationtech:master Jan 8, 2020
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.

2 participants