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-3426 FSDS - Support path filtering for converter queries #3245

Merged
merged 5 commits into from
Dec 16, 2024

Conversation

fdfea
Copy link
Contributor

@fdfea fdfea commented Dec 12, 2024

No description provided.

@fdfea
Copy link
Contributor Author

fdfea commented Dec 12, 2024

Reminding myself to update the docs

@fdfea fdfea force-pushed the ffeaser_fsds_path_filter branch 2 times, most recently from fb05e71 to a530c4c Compare December 13, 2024 20:35
@fdfea fdfea force-pushed the ffeaser_fsds_path_filter branch from a530c4c to d2e3424 Compare December 13, 2024 20:39
@fdfea fdfea changed the title FSDS - Support path filtering for queries FSDS - Support path filtering for converter queries Dec 13, 2024
val pathFiltering = Option(context.conf.get(PathFilterName)).flatMap { name =>
val factory = PathFilteringFactory.load(NamedOptions(name, pathFilteringOpts.toMap))
if (factory.isEmpty) {
logger.warn(s"Failed to load ${classOf[PathFiltering].getName} for config '$name'")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you think a warning is ok here, or should we throw an error? i worry people might not notice the warning, and then get worse performance but since things still work correctly they might not realize the mistake

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm otherwise!

@elahrvivaz elahrvivaz changed the title FSDS - Support path filtering for converter queries GEOMESA-3426 FSDS - Support path filtering for converter queries Dec 16, 2024
@fdfea
Copy link
Contributor Author

fdfea commented Dec 16, 2024

@elahrvivaz I've tested with real data on my end and it seems to be working expected. Could we also get this in a 5.0.x release?

@elahrvivaz
Copy link
Contributor

thanks for remembering the docs!

@elahrvivaz elahrvivaz merged commit d6d45c8 into locationtech:main Dec 16, 2024
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.

2 participants