binarykitchen / find-remove

recursively finds files by filter options from a start directory onwards and deletes these. useful if you want to clean up a directory in your node.js app.
https://npmjs.org/package/find-remove
61 stars 19 forks source link

Promise/Async support #13

Open d0b1010r opened 6 years ago

d0b1010r commented 6 years ago

Hi, tried my take at adding async support. This requires node 8 with async/await because this allows the code to be really close. The only test I needed to change (apart from the .then) was adding setTimeout with 1 ms for the 0.0005 tests.

What do you think?

binarykitchen commented 6 years ago

thanks for your efforts - a couple of problems with your commits:

d0b1010r commented 6 years ago

I see. Well I can't think of any maintainable way to do these complicated checks with callbacks (as you mentioned) - at the same time I can't envision how it should work with streams / and or events. Perhaps you can share some ideas? I work mostly nowadays with async/await and really like it, it's just so much more readable and logic then all these callbacks (or even promises).

The other points: 1) I thought you might want to keep both versions - but changed, so now the history is visible 3) I "unchanged" the 0.0005 seconds test and they still failed on my system - but on travis it's not a problem. I rechecked and even before my change these tests fail on my system (mac os x).This seems to be about the speed of the system? I added some logging:

path | now - expiration time | now | mtime | now > expirationTime
path .../directory1/directory1_2/directory1_2_1/something.jpg ageSeconds 0.5 1518528404355 1518528404354  _  true
path .../find-remove/directory1/directory1_2/directory1_2_1/something.png ageSeconds -0.5 1518528404355 1518528404355  _  false

Seems to be that the files are not old enough in these cases and thus not deleted. It would help to change the check from return now > expirationTime to >=.

4) Thanks for the info, now the build runs fine.

binarykitchen commented 6 years ago

great, looking much better. we're close.

about the 0.0005 ... can you change it to a constant at top of test and try to increment it slowly until all test pass. very curious here. dont change to >= as this isn't correct.

binarykitchen commented 6 years ago

and yes, don't forget documentation ;)

d0b1010r commented 6 years ago

I think the problem with the 0.0005s check is that it's already higher than the available precision. Or am I overlooking something? From my example:

current time:       1518528404355
modification time:  1518528404355
expiration time is  1518528404355.5
-> difference is    -0.5

current and modification time is already exactly the same number. Might work to switch to process.hrtime. But currently out of time for the week, so more next week.

binarykitchen commented 6 years ago

or it is because we have different CPU speeds? what's your CPU?

d0b1010r commented 6 years ago

It's a 2,6 GHz Intel Core i5 / MBP from 2014 with 10.13.3

binarykitchen commented 6 years ago

okay your machine is faster than mine, although tests pass here. are you on mac os?

d0b1010r commented 6 years ago

Yes, I am on mac os x. I think it's something with the speed as well. My machine "is too" fast in a way, because the files are not old enough to be older than 0.0005s. Thus a setTimeout of 1ms in the test would ensure that the files are old enough to be watched by the module.

binarykitchen commented 6 years ago

ok, understand. in that case change increase slightly, maybe to 1ms or more until all tests pass.

binarykitchen commented 6 years ago

@davidlanger you still alive?

d0b1010r commented 6 years ago

Yepp, but was on vacation the last two weeks. Will continue as fast as possible.

But you wrote to "increase slightly, maybe to 1ms or more". But this won't help the problem, as the problem is that the files are not old enough. The only way to make this check work, is to add timeout, as the problem is that on my computer the files are never old enough to be deleted if the code is run right after the initialization.

Illustration: Currently:

  1. Initialization, creating files
  2. Test

The time between 1 and 2 is less than the precision available (which is only in milliseconds). 0.0005s is less than a millisecond.

With set Timeout:

  1. Initialization
  2. Wait 1ms
  3. Test

Now all the newly created files are old enough.

binarykitchen commented 6 years ago

oh i see, sorry - in that case add the timeout then

binarykitchen commented 6 years ago

@davidlanger hello?

binarykitchen commented 6 years ago

@davidlanger you still alive?

Hibrix-net commented 3 years ago

Hello, Any news of this?

Cheers,

binarykitchen commented 3 years ago

@Hibrix-net sorry no time and needs rebasing

binarykitchen commented 1 year ago

@mgerylrm thanks for your approval, yet merge conflicts should be resolved first.