TimedCertificateReloader should use NIOFileSystem and not blocking I/O from Foundation

[weissi]

swift-nio-extras/Sources/NIOCertificateReloading/TimedCertificateReloader.swift

Line 365 in 72a23ea

guard let bytes = FileManager.default.contents(atPath: path) else {

This should use the non-blocking NIOFileSystem APIs. They're both more easy to use and work correctly even if the file system blocks (Foundation.FileManager blocks the calling thread and uses different implementations on Linux & Darwin).

[glbrntt]

I think we've gotta stabilise the NIOFS API first but we should make the change after we've done that.

[weissi]

I think we've gotta stabilise the NIOFS API first but we should make the change after we've done that.

NIOExtas and NIOFS have the same development team and therefore I think it should be absolutely possible to adopt this sooner than that. But besides: What's holding NIOFS back?

[glbrntt]

NIOExtas and NIOFS have the same development team and therefore I think it should be absolutely possible to adopt this sooner than that.

With the proposed file path changes I think it's a very real possibility that NIOFS breaks its API.

What's holding NIOFS back?

Resources, sadly

[weissi]

NIOExtas and NIOFS have the same development team and therefore I think it should be absolutely possible to adopt this sooner than that.

With the proposed file path changes I think it's a very real possibility that NIOFS breaks its API.

I think that'll have to be staged anyway at this point, NIOFS is already pretty widely adopted so I'm pretty sure that enough compatibility could be provided for NIOFS to use it as an implementation detail.

Sure, NIOExtras shouldn't expose SystemPackage.FilePath in its API but I don't think that'd be necessary. For example, where ever a FilePath (as in any of System.FilePath, NIO.NIOFilePath or SystemPackage.FilePath) is required, it should be possible to do .init(filePathString) for example. Also, NIO could still keep the _NIOFileSystem package alive for a few months with strongly worded deprecation warnings that it should be adoptable by NIOExtras.

[glbrntt]

I think that'll have to be staged anyway at this point, NIOFS is already pretty widely adopted so I'm pretty sure that enough compatibility could be provided for NIOFS to use it as an implementation detail.

FilePath is used in protocol requirements so the API will break (technically) but you're right that we can probably shim this in a way that most users won't notice. There are definitely user that implement the underlying protocol as well who will be broken.

Sure, NIOExtras shouldn't expose SystemPackage.FilePath in its API but I don't think that'd be necessary. For example, where ever a FilePath (as in any of System.FilePath, NIO.NIOFilePath or SystemPackage.FilePath) is required, it should be possible to do .init(filePathString) for example. Also, NIO could still keep the _NIOFileSystem package alive for a few months with strongly worded deprecation warnings that it should be adoptable by NIOExtras.

I'm not worried about the API being used here and that breaking. I don't want its use here to be given as an implicit signal that the API is stable and won't change.

IMO this is just more evidence we need to raise the priority of getting it over the line.

[weissi]

IMO this is just more evidence we need to raise the priority of getting it over the line.

Agreed! The ecosystem deserves to have a modern, non-blocking file system API. We're all into asynchronous I/O so we all need it.