Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [geomesa-dev] datastore api contract
Timeout as a separate connection parameter sounds like the absolute right call. 

In terms of branches, for some of it, we need to do better about cleaning up after ourselves.;)

Other than that, occasionally a change in a version (especially for Accumulo/Hadoop) can require a code change.  We haven't gotten slick with integrating those issues with profiles quite yet.  Generally though, we've got room for improvement with our build.  Maybe we can chat about it at the LT Incubation sprint?

Cheers,

Jim

On 12/13/2015 05:55 PM, Jody Garnett wrote:
I would recommend making the timeout a seperate connection parameter :)

And since you drive the bus you can make any style changes you desire when merging. Indeed since we have so many divergent branches for geomesa you are unlikely to get a clean pull request.

(aside: So why so many branches, that is what maven profiles are for...)

--
Jody Garnett

On 13 December 2015 at 12:14, Jim Hughes <jnh5y@xxxxxxxx> wrote:
Hi Jody,

Thanks for bringing these up again.  The changes look reasonable; there are only two things worth discussing briefly:

1.  For the Zookeeper future timeout, would 5 seconds be ok?  It looks like this is designed to avoid either a longer 30-second timeout or a situation where things just hang. 

2.  Since we don't have a published style-guide, would it be acceptable to make style changes while merging PRs for these changes? 

Thanks in advance,

Jim


On 12/11/2015 10:40 PM, Jody Garnett wrote:

So we kind of owe you a pull request :) We packaged up GeoMesa for our last suite release and fixed a couple datastore api bugs that prevented other datastores from working correctly when GeoMesa was installed.

Here are the three fixes:

The above commits focus on:

  • AccumuloDataStoreFactory.scala
  • GeoMesaCoverageFormat.scala
  • CoverageFormat.scala

The key take away is checking the URL that is passed in is appropriate (so we do not pretend to accept shapefile or geotiff references):

override def accepts(input: AnyRef) = testURL(input)
override def accepts(source: AnyRef, hints: Hints) = testURL(source)
private def testURL(source: AnyRef) = source match{
  case str: String => str.startsWith("accumulo://")
  case _ => false    
  }

While Tom has signed the correct contributor agreement (for the above snippet) - I expect we come up with a more aesthetically pleasing solution on the dev list.

Tyler (if you are on here) you may have a cleaner branch to use as a starting point?



_______________________________________________
geomesa-dev mailing list
geomesa-dev@xxxxxxxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
http://locationtech.org/mailman/listinfo/geomesa-dev


_______________________________________________
geomesa-dev mailing list
geomesa-dev@xxxxxxxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
http://locationtech.org/mailman/listinfo/geomesa-dev



_______________________________________________
geomesa-dev mailing list
geomesa-dev@xxxxxxxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
http://locationtech.org/mailman/listinfo/geomesa-dev

Back to the top