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

CAMEL-11656 - add preSort option #2108

Merged
merged 1 commit into from Nov 23, 2017
Merged

Conversation

onderson
Copy link
Contributor

even though i believe this issue is not related to sorting file list beforehand, i am just opening the PR to discuss further and revise...
@davsclaus could you review?

@oscerd
Copy link
Contributor

oscerd commented Nov 22, 2017

In my opinion it can be good to have this, but we need to avoid the options number explosion in this component. Lets wait for @davsclaus

Copy link
Contributor

@davsclaus davsclaus left a comment

Choose a reason for hiding this comment

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

As much as I love the java 8 stream api, then this would create a 2nd list for sorting, where as Collections.sort would work on existing ArrayList and not use a 2nd list. We had some examples where users had 100.000+ files in a dir that would eat up memory (more in particular with the ftp, where there is a FTP remote file object instance)

I wonder if we can sort using non java 8 streaming

@onderson
Copy link
Contributor Author

yes sure.updated.

@davsclaus
Copy link
Contributor

Maybe we can improved the documentation a bit, something like:

When pre-sort is enabled then the consumer will sort the file and directory names during polling, that was retrieved from the file system. You may want to do this in case you need to operate on the files in a sorted order. The pre-sort is executed before the consumer starts to filter, and accept files to process by Camel. This option is default disabled.

@onderson onderson merged commit 20936b1 into apache:master Nov 23, 2017
@oscerd
Copy link
Contributor

oscerd commented Nov 23, 2017

@onders86 is there a specific reason to backport to 2.20.x? This is a new option, so usually we avoid new options in minor releases

@onderson
Copy link
Contributor Author

@oscerd No there is not. I thought we would need to backport because this was a wish since 2.19.0. I dont mind reverting. Let me know.

@onderson
Copy link
Contributor Author

@oscerd , reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants