Re-order loadFiles() so that logging occurs prior to renaming failed converters

[rhysfred]

When LoadFiles() in externalJS encounters an invalid converter, it attempts to rename this to converter.invalid prior to performing any logging. In the case where renaming the file is not possible (e.g. because the fs is read-only), the function never logs what was actually wrong with the converter (the error is swallowed).

[rhysfred]

Yet to confirm which, but seems that either the external converter file itself or the external_converters directory needs to be writable (why?). external converter works fine when copied into the directory as a writeable file, but not when mounted as the external_converters directory in kubernetes from a configMap. Would still suggest re-ordering the code so that the logging occurs first.

[Nerivec]

There are multiple reasons for the writable need, symlinking node_modules, cache busting, rename on invalid....
Anything under data dir is expected to be writable in general though.

[rhysfred]

OK, thanks for explaining why the external_converters directory needs to be writable. I would still though respectfully advocate for changing the ordering of the method. The current ordering logs an error that the unhappy path has failed (i.e. renaming the file to *.invalid due to ROFS), but it doesn't log the error in the happy path (presumably a similar write failure) that led to the unhappy path. Changing the order will log both, making it obvious that the happy path, not just the unhappy path, depend upon the directory being writable. This is useful as kubernetes users such as myself may make the incorrect assumption that the converter can be mounted as a simple configMap.

[Koenkk]

Thanks!