GEOMESA-1422 Allow GeoMesaInputFormat to use Accumulo mock connection #1077
Conversation
|
just wondering...what was the underlying problem here? I thought we regularly used the |
jahhulbert-ccri
changed the title
jahhulbert-ccri
changed the title
|
@jahhulbert-ccri there would be first errors. To use mock object we have to pass Accumulo mock connector in params, but params accept type |
| @@ -151,7 +167,23 @@ class GeoMesaInputFormat extends InputFormat[Text, SimpleFeature] with LazyLoggi | |||
| var table: GeoMesaTable = null | |||
|
|
|||
| private def init(conf: Configuration) = if (sft == null) { | |||
| val params = GeoMesaConfigurator.getDataStoreInParams(conf) | |||
| val dsParams = GeoMesaConfigurator.getDataStoreInParams(conf) | |||
There was a problem hiding this comment.
I don't think you need to make any changes to this method - the mock parameters should get passed through and the data store should create the mock connection for us. It might be b/c you are hard-coding the instance name - I think we can avoid that above though.
|
@pomadchin so did you try adding ("useMock", "true") to the dsParams and if so did it fail? |
| "connector" -> connector, | ||
| "caching" -> false, | ||
| "tableName" -> AccumuloDataStoreParams.tableNameParam.lookUp(dsParams).asInstanceOf[String] | ||
| ) |
There was a problem hiding this comment.
I don't think you need to make these changes either - just checking for the isMock parameter and calling the appropriate AbstractInputFormat/InputFormatBaseAdapter method should be enough...
if you keep the data store parameters the same as get passed in, it should sync up with the InputFormat class below and the AccumuloDataStoreFactory method
|
@jahhulbert-ccri there's a different AccumuloInputFormat configuration for mock connectors - we actually do it properly in spark, but not here |
|
@elahrvivaz ah ok good to know |
|
@jahhulbert-ccri works, for some reason not worked ._. // updated pr |
|
@pomadchin can you sign a CLA as outlined here: |
|
@pomadchin could you copy the fix into the |
elahrvivaz
changed the title
|
whoever merges should add the ticket # to the squashed commit message |
|
@elahrvivaz I'll add it. this all look good to you? |
| @@ -71,7 +71,10 @@ object GeoMesaInputFormat extends LazyLogging { | |||
|
|
|||
| val instance = AccumuloDataStoreParams.instanceIdParam.lookUp(dsParams).asInstanceOf[String] | |||
| val zookeepers = AccumuloDataStoreParams.zookeepersParam.lookUp(dsParams).asInstanceOf[String] | |||
| InputFormatBaseAdapter.setZooKeeperInstance(job, instance, zookeepers) | |||
| if(AccumuloDataStoreParams.mockParam.lookUp(dsParams).asInstanceOf[String] == "true") | |||
There was a problem hiding this comment.
@pomadchin picky - we usually add a space after the `if`` for our scala style
|
@jahhulbert-ccri yeah, but I think we need to wait for @pomadchin to sign the CLA before merging. |
|
with CLA we need to wait a bit // changed email address, it takes time to invalidate it and to validate again. |
Signed-off-by: Grigory Pomadchin <gr.pomadchin@gmail.com>
|
@pomadchin thanks, I'll merge today. The eclipse CLA site is down, but once I can verify that we should be good. |
|
merging |
|
merged as 3f952bf |
This pr allows
GeoMesaInputFormatto work with Accumulo mock connection.Signed-off-by: Grigory Pomadchin gr.pomadchin@gmail.com