-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
GH-8898: method to clear queue in AbstractRemoteFileStreamingMessageS… #8899
Conversation
…reamingMessageSource
@kkosiacki Please sign the Contributor License Agreement! Click here to manually synchronize the status of this Pull Request. See the FAQ for frequently asked questions. |
@kkosiacki Thank you for signing the Contributor License Agreement! |
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.
This new API has also to be mentioned in the whats-new.adoc
.
You also can talk about it in the rotating-server-advice.adoc
.
Thanks
/** | ||
* Clear internal queue of listed files.. | ||
*/ | ||
public void clearUnprocessedFilesQueue() { |
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.
OK. I cannot imaging the scenario when it is necessary, but let's give it a more meaningful name like clearFetchedCache()
and some good Javadoc, plus @since 6.3
and your name to the @author
list of the affected classes.
I think another fix (separate issue/PR) must be done in the StandardRotationPolicy
to enforce setMaxFetchSize(1)
when mode is fair
. Otherwise that fetched cache really does not make sense since we have just rotated to another dir and (possible) session factory. So, we would expect fresh file for that condition.
|
||
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.assertThatExceptionOfType; | ||
import static org.mockito.ArgumentMatchers.anyInt; | ||
import static org.mockito.ArgumentMatchers.anyString; | ||
import static org.mockito.Mockito.mock; | ||
import static org.mockito.Mockito.when; | ||
import static org.mockito.Mockito.*; |
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.
No asterisk imports.
See if spring-framework.xml
in the src/idea
or similar in eclipse
can help you to re-align your IDE with our code style.
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.
See the fix for this issue: #8967.
Might be that is enough for your use-case and we don't need to do anything else?
Thanks
Merged as 753916c after some cleanup and docs. Thank you for contribution anyway; looking forward for more! |
This Pull Request is fixing problem describe in GH-8898