So we added the finalizer to ensure that multiple downloads on the same connection don't fail, but there was no timeout on that completePending call. In my real-world tests, I've seen that freeze with no connection error so that the program eventually dies (as in all connections freeze on that call and nothing continues running, not that the program crashes).
I wrote this code so that if a data timeout is provided in the ftp settings, it will also be applied for the complete pending portion of downloads. In my real-world tests it's been running non-stop for 6 days with no problems (my program would freeze at least daily before), and I wrote a unit test to ensure this code works.
Unfortunately, I'm running ZIO2 with a locally-built zio-ftp library, so I wrote this code and the unit test against that. When I tried to backport everything to ZIO1, the unit test fails every time. I'm presuming this would work in the real world on ZIO1, the code isn't that complicated and I didn't have to change any of it for the backport (just the stuff dealing with layers that has changed).
I'll leave this for you to decide what to do with it, I think the code is good and it works for me on ZIO2, but it definitely would be nice to have a unit test covering it. maybe the existing tests using docker will give you enough assurance. (on that note, I'd add a test that tries multiple downloads on the same connection that would have broken before the finalization call was added)
regis-leray
commented
2 years ago
So we added the finalizer to ensure that multiple downloads on the same connection don't fail, but there was no timeout on that completePending call. In my real-world tests, I've seen that freeze with no connection error so that the program eventually dies (as in all connections freeze on that call and nothing continues running, not that the program crashes).
I wrote this code so that if a data timeout is provided in the ftp settings, it will also be applied for the complete pending portion of downloads. In my real-world tests it's been running non-stop for 6 days with no problems (my program would freeze at least daily before), and I wrote a unit test to ensure this code works.
Unfortunately, I'm running ZIO2 with a locally-built zio-ftp library, so I wrote this code and the unit test against that. When I tried to backport everything to ZIO1, the unit test fails every time. I'm presuming this would work in the real world on ZIO1, the code isn't that complicated and I didn't have to change any of it for the backport (just the stuff dealing with layers that has changed).
I opened an issue with ZIO about not being able to test this properly: https://github.com/zio/zio/issues/6255
I'll leave this for you to decide what to do with it, I think the code is good and it works for me on ZIO2, but it definitely would be nice to have a unit test covering it. maybe the existing tests using docker will give you enough assurance. (on that note, I'd add a test that tries multiple downloads on the same connection that would have broken before the finalization call was added)