New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GEOMESA-1198 Changing attribute table suffix #913
Conversation
Signed-off-by: Emilio Lahr-Vivaz <elahrvivaz@ccri.com>
I tested out with old data and with old enabled indices and it still works. |
@@ -51,7 +51,7 @@ object AttributeTable extends GeoMesaTable with LazyLogging { | |||
override def supports(sft: SimpleFeatureType) = | |||
sft.getSchemaVersion > 5 && sft.getAttributeDescriptors.exists(_.isIndexed) | |||
|
|||
override val suffix: String = "attr_idx" | |||
override val suffix: String = "attr" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this affect 'existing' installations? I'm guessing 'no' since the table name would be configured in the metadata in Accumulo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's back-compatible, yeah. the main bugaboo was if they used the 'enabled.indices' options.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do have a couple of places where we used that... @jnh5y see Alec's email 😢 We should probably at least try to do real release notes like accumulo where we can mention things like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just to clarify it will work with enabled.indices - that just was where the complexity in code came in.
Ok, looks good to me. |
so far looks good. i'll try it with an old one just to make sure. |
@jahhulbert-ccri ok, great; then I'm assigning the PR to you. |
Signed-off-by: Emilio Lahr-Vivaz elahrvivaz@ccri.com