search

ngageoint / hootenanny-ui

Hootenanny UI is a submodule of the Hootennany vector conflation project.
ISC License
29 stars 7 forks source link

Clipped layer does not support space in output name #326

Closed mikejeffe closed 8 years ago

mikejeffe commented 8 years ago

When you clip using bounding box or map extent, there is the option to enter a name for the layer. If the name has a space in between it causes the clip job to fail without reporting the reason why (example error below). Since providing a space in the name is supported when renaming a layer it seems that we should support it with all functions that involve output name.

2016-03-16 13:56:12,675 ERROR JobExecutionManager:225 - Job with ID: a0b658d9-6382-4bd1-882d-fe4015f0d732 failed: {"chainjobstatus":"a0b658d9-6382-4bd1-882d-fe4015f0d732","childrencount":"1","children":[{"id":"834e8d78-4d4b-4cdb-90c2-bd6c7864c342","detail":"Failed to execute.Error running crop-map:\ncrop-map takes three parameters.\nmake: *** [step1] Error 255\n","status":"failed"}]}

mikejeffe commented 8 years ago

This also could be something that @sisskind might have some insight into since he worked on the implementation of clipping layers.

sisskind commented 8 years ago

This may also be causing issues on ngageoint/hootenanny#480. Need to confirm space issue for ingesting datasets or creating through conflation.

sisskind commented 8 years ago

@mjeffe98 Ready for test. Should not be able to use a space in dataset name when importing/clipping/conflating.

sisskind commented 8 years ago

https://github.com/ngageoint/hootenanny-ui/commit/d8e284dfe24fa30dabb4a034e34f33d4a5c3aacf#diff-23fb12fc4edecd0c5add7cbdc3dcca59

mikejeffe commented 8 years ago

Just adding a comment for a mental note. Revisiting this to see if removing support for spaces altogether is the best approach or not.

sisskind commented 8 years ago

Appears that importing with spaces is also causing errors.

sisskind commented 8 years ago

This is a renderdb issue -- it appears it will not accept layer names with spaces. @brianhatchl how do you want to proceed?

sisskind commented 8 years ago

@brianhatchl @mattjdnv changing hoot core code to replace map name with map id in issue-326 branch (will commit shortly).

brianhatchl commented 8 years ago

@sisskind replacing map name with id is the proper fix. There should be tests that break at this point.

sisskind commented 8 years ago

@brianhatchl ok -- will let you know once it is ready for a review. may miss some things along the way, but if i can import/clip/delete without warnings, I think I'll be on the right track?

brianhatchl commented 8 years ago

I'm still a bit uncertain because I thought there was a ticket for adding a export render db trigger to the clipping job, but maybe not.

This fix will settle this https://github.com/ngageoint/hootenanny-ui/issues/269

brianhatchl commented 8 years ago

See scripts/exportrenderdb.sh and then uses of RasterToTilesService job in ConflationResource and FileUploadResource. Currently $INPUT=mapname and those resources would have to add the mapid to the available env vars for the exportrenderdb script.

brianhatchl commented 8 years ago

_createCommandObj in RasterToTilesService would need to accept the map id as a param.

As you can see, I just commandeered the old density map service infrastructure to run the exportrenderdb script instead of the gdal2tiles stuff.

sisskind commented 8 years ago

So that is the only change that would need to be made? I also found map name being used instead of map id in a few places and made the change (see https://github.com/ngageoint/hootenanny/commit/467de2892c83ce8561eecd07d521abb525e248ab)

sisskind commented 8 years ago

Working on updating _createCommandObj now

brianhatchl commented 8 years ago

There is newish code in the core to delete render dbs within hoot delete-map so that's the first update in the commit you reference. And the node-mapnik-server needs to know the render db naming convention as well.

sisskind commented 8 years ago

where in node-mapnik-server does that get defined?

brianhatchl commented 8 years ago

I think you already found it in your initial commit, I was describing what those commit changes affected in comment above.

sisskind commented 8 years ago

@brianhatchl do you have bandwidth to review? think I am missing something, since mapnik is running but not rendering. Will create a pull request for easier viewing.

sisskind commented 8 years ago

https://github.com/ngageoint/hootenanny/pull/598

brianhatchl commented 8 years ago

The pull request above (replacing map name with map id in the render database name) is ready to review. But I don't think the clip tool in the UI generates a render db based on my testing (see #262). This pull request should address https://github.com/ngageoint/hootenanny-ui/issues/269 however.