Move new files filtering to earlier

[pivnicek]

fixes #28

[pivnicek]

Hi @eddycek

This is a little PR to apply the new file filtering during the preparation step.

I was wondering if it wouldn't make sense also to apply this filter before the glob filter (since that is what I think takes the most time https://github.com/keboola/ex-ftp/blob/master/src/FtpExtractor.php#L129). I was looking at the job log and it took 26 sec for the prepare 10 files step and I think the glob validator may be the culprit, so reducing the amount of files we send to that loop could be helpful. Anyhow, just a couple thoughts. cheers.

[eddycek]

Hi @pivnicek

The idea of applying this filter before the global comes to me as a good idea.
@yustme, what do you think about that?

[eddycek]

lgtm

[pivnicek]

Thanks @eddycek , I'll add it and we can always take it or leave it :-)

[pivnicek]

huh, I think I've found a bug. I'm going to add a newFilesOnly test or two.

[pivnicek]

heh, not a bug in the original, but I think I created one :-). almost got it sorted.

[eddycek]

@pivnicek What's the problem?

[pivnicek]

ah, I seem to have forgotten about this :-) The main issue is that the shouldBeFileUploaded method https://github.com/keboola/ex-ftp/blob/master/src/FileStateRegistry.php#L40 doesn't just check whether the file should be uploaded, but prepares the output state also. So applying this before the glob means the glob filtered files may still end in the output state.

I would like to change it into 2 methods: isNewFile and prepareOutputState. I'll get to it shortly :-)

[eddycek]

@pivnicek oh, it really looks it must be splitted 👍

[pivnicek]

I added new tests, and they're passing locally, but the test where I add a file on the fly is not working on travis :-/ https://travis-ci.com/keboola/ex-ftp

[eddycek]

I added new tests, and they're passing locally, but the test where I add a file on the fly is not working on travis :-/ https://travis-ci.com/keboola/ex-ftp

@pivnicek strange error, did you try to restart the build?

[pivnicek]

it fails consistently, but I see why now. docker-compose run --rm app composer ci fails (like on travis), but docker-compose run --rm dev composer ci passes. So I should resolve it shortly.

[pivnicek]

I added the volume mapping for the testfile dir to the app service and it passes now. but nejsem jistej

[eddycek]

just a few things

[eddycek]

👍

[pivnicek]

Thanks @eddycek 🍻