Skip to content
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-3447 FSDS - Support allow-list for attribute partition scheme #3271

Merged
merged 6 commits into from
Feb 4, 2025

Conversation

dlgundry
Copy link
Contributor

@dlgundry dlgundry commented Jan 31, 2025

  • Added the ability to provided a comma separated list of allowed paths to AttributeScheme
  • Allows AttributeScheme to be configured to only partition specific pre-defined allowed paths in FSDS

@elahrvivaz
Copy link
Contributor

seems like we should also update getPartitionName (which isn't used in the converter case, but would in the regular case), so that we don't end up with values that can't be read. i think then we'd also need a default value (instead of the empty string like we have now), maybe as another option, and if it's not specified just use the first in the enumerated list.

@dlgundry dlgundry force-pushed the attribute-scheme-filtering branch from 78d00ea to 918b990 Compare February 3, 2025 14:22
@dlgundry dlgundry changed the title Added Enumeration Config and Logic to AttributeScheme Added Allowed List Config and Logic to AttributeScheme Feb 3, 2025
@dlgundry
Copy link
Contributor Author

dlgundry commented Feb 3, 2025

seems like we should also update getPartitionName (which isn't used in the converter case, but would in the regular case), so that we don't end up with values that can't be read. i think then we'd also need a default value (instead of the empty string like we have now), maybe as another option, and if it's not specified just use the first in the enumerated list.

I think I have captured your comment with the latest commit

@elahrvivaz
Copy link
Contributor

looks good, i think there's just a little bit of inconsistency in when we're dealing with type-encoded values and plan values. i'll try to capture that in another comment in a minute.

@elahrvivaz
Copy link
Contributor

elahrvivaz commented Feb 3, 2025

So for the attribute index, we run values through a lexicoder, which maps from a normal value to a sorted stringified value. i.e. if you're dealing with ints, 0 maps to something like 80000000 so that negative values sort correctly. there are 2 methods, AttributeIndexKey.typeEncode (which assumes a valid typed input value) and AttributeIndexKey.encodeForQuery (which will try to convert the input into the correct type). Since we're dealing with strings in the input here, we probably want to run allowedValues and defaultPartition through encodeForQuery in the factory set-up. then i think the only change is we have to run the input value through typeEncode before comparing it in getPartitionName

@elahrvivaz
Copy link
Contributor

@dlgundry you'll also need to sign the Eclipse CLA as outlined here: https://github.com/locationtech/geomesa/blob/main/CONTRIBUTING.md#eclipse-contributor-agreement

{"fundingSource": "41203", "team": "FED.ICGSA.APPS.FUSION.ARCH", "fshGit": "885a7c43", "fshDocker": "sha256:a8bfeb6e"}
@dlgundry dlgundry force-pushed the attribute-scheme-filtering branch from d3d0d3e to 4e628f7 Compare February 3, 2025 17:45
@elahrvivaz
Copy link
Contributor

generally, don't force-push, that makes it easier to track changes - then i'll squash down and clean up the messages on merge.

…in/scala/org/locationtech/geomesa/fs/storage/common/partitions/AttributeScheme.scala

Co-authored-by: Emilio <[email protected]>
{"fundingSource": "41203", "team": "FED.ICGSA.APPS.FUSION.ARCH", "fshGit": "a4f7446b", "fshDocker": "sha256:054bce20"}
@elahrvivaz
Copy link
Contributor

@fdfea any more comments?

dlgundry and others added 2 commits February 3, 2025 15:34
…in/scala/org/locationtech/geomesa/fs/storage/common/partitions/AttributeScheme.scala

Co-authored-by: Emilio <[email protected]>
{"fundingSource": "41203", "team": "FED.ICGSA.APPS.FUSION.ARCH", "fshGit": "a4f7446b", "fshDocker": "sha256:054bce20"}
@fdfea
Copy link
Contributor

fdfea commented Feb 3, 2025

lgtm 🚀

@elahrvivaz elahrvivaz changed the title Added Allowed List Config and Logic to AttributeScheme GEOMESA-3447 FSDS - Support allow-list for attribute partition scheme Feb 4, 2025
{"fundingSource": "41203", "team": "FED.ICGSA.APPS.FUSION.ARCH", "fshGit": "a4f7446b", "fshDocker": "sha256:054bce20"}
@elahrvivaz elahrvivaz merged commit 03ed860 into locationtech:main Feb 4, 2025
69 of 70 checks passed
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.

3 participants