(develop) BackupSystemTask is not limiting the backups as folders

[claudius108]

Fix for issue #2387.

[adamretter]

@claudius108 the history for the commits here seems a bit muddled?

[claudius108]

Indeed, but they are working fine. I tested them a lot.

…

On Wed, Mar 13, 2019, 06:07 Adam Retter ***@***.*** wrote: @claudius108 <https://github.com/claudius108> the history for the commits here seems a bit muddled? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABqjnmkVtYUM_rilMcYNEeN3ExflFJtuks5vWHmAgaJpZM4bDGqa> .

[adamretter]

@claudius108 Okay, could you rebase please, and drop the incidental changes, e.g. xquery/parser/*?

[claudius108]

Rebased and picked up only the needed commits, I hope now it is good.

…

On Wed, Mar 13, 2019 at 6:33 AM Adam Retter ***@***.***> wrote: @claudius108 <https://github.com/claudius108> Okay, could you rebase please, and drop the incidental changes, e.g. xquery/parser/*? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABqjnv6VfIPxKLoaQJgw4_plNab1seyoks5vWH-VgaJpZM4bDGqa> .

-- http://kuberam.ro

[adamretter]

I think you should either put entriesPaths in a try-with-resources or you need to finally close it... otherwise file handles may be leaked!

[adamretter]

@claudius108 apart from the one comment it LGTM, can you squash the history to a single commit please?

[claudius108]

You are right, thanks. I will use if (numberOfEntriesToBeDeleted > 0) { try (Stream<Path> list = entriesPaths.stream();) { list.sorted(timestampComparator).limit(numberOfEntriesToBeDeleted).forEach(path -> { if (LOG.isDebugEnabled()) { LOG.debug("Purging backup : " + FileUtils.fileName(path)); } FileUtils.deleteQuietly(path); }); } }

…

On Wed, Mar 13, 2019 at 8:56 AM Adam Retter ***@***.***> wrote: @claudius108 <https://github.com/claudius108> apart from the one comment it LGTM, can you squash the history to a single commit please? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2496 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABqjnmuUHVVJbHvOULeblt-xMUOPxaaDks5vWKEXgaJpZM4bDGqa> .

-- http://kuberam.ro

[claudius108]

But in FileUtils.java you already use try() /** * A list of the entries in the directory. The listing is not recursive. * * @param directory The directory to list the entries for * @param filter A filter to be applied to the list * @return The list of entries */ public static List<Path> list(final Path directory, final Predicate<Path> filter) throws IOException { try(final Stream<Path> entries = Files.list(directory).filter(filter)) { return entries.collect(Collectors.toList()); } } On Wed, Mar 13, 2019 at 9:23 AM Claudius Teodorescu < claudius.teodorescu@gmail.com> wrote:

…

You are right, thanks. I will use if (numberOfEntriesToBeDeleted > 0) { try (Stream<Path> list = entriesPaths.stream();) { list.sorted(timestampComparator).limit(numberOfEntriesToBeDeleted).forEach(path -> { if (LOG.isDebugEnabled()) { LOG.debug("Purging backup : " + FileUtils.fileName(path)); } FileUtils.deleteQuietly(path); }); } } On Wed, Mar 13, 2019 at 8:56 AM Adam Retter ***@***.***> wrote: > @claudius108 <https://github.com/claudius108> apart from the one comment > it LGTM, can you squash the history to a single commit please? > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#2496 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ABqjnmuUHVVJbHvOULeblt-xMUOPxaaDks5vWKEXgaJpZM4bDGqa> > . > -- http://kuberam.ro

-- http://kuberam.ro