fix(cli): handle missing target artifact dir #4112
Conversation
bajtos
force-pushed
the
fix/lb4-artifact-discovery-missing-dir
branch
from
a922255
to
0993d0f
Compare
| ); | ||
| }); | ||
| } catch (err) { | ||
| if (err.code === 'ENOENT') { |
There was a problem hiding this comment.
I would prefer to check if the directory exists in advance instead of using the err code.
There was a problem hiding this comment.
I find that approach problematic, because it introduces a race condition - between the time we check if the directory exists, and the time we read the entries, the directory can be deleted or removed.
What are your concerns about my proposed approach?
There was a problem hiding this comment.
I don't think we need to deal with the edge case where the directory is removed when the command in running. If so, we'll have to use sync fs apis.
My main concern is to rely on the error code. It would be more authoritative by calling an api to check existence of the directory.
It's not a blocker for the PR, anyway.
| @@ -119,6 +117,21 @@ describe('lb4 model integration', () => { | |||
| basicModelFileChecks(expectedModelFile, expectedIndexFile); | |||
| }); | |||
|
|
|||
| it('creates "src/models" directory if it does no exist', async () => { | |||
There was a problem hiding this comment.
typo: does no exist -> does not exist
There was a problem hiding this comment.
I fixed it as part of rebase.
| } catch (err) { | ||
| if (err.code === 'ENOENT') { | ||
| // Target directory was not found (e.g. "src/models" does not exist yet). | ||
| return []; |
There was a problem hiding this comment.
Question: I assume if the target directory does not exist, it will get created when creating the artifact?
When creating a new artifact (e.g. Model or Repository) while the target directory does not exist yet (e.g. `src/models` or `src/repositories`), we used to abort CLI with a ENOENT error. This commit is fixing our CLI to treat the missing directory as if it was empty and continue the operation if possible. Signed-off-by: Miroslav Bajtoš <[email protected]>
raymondfeng
force-pushed
the
fix/lb4-artifact-discovery-missing-dir
branch
from
0993d0f
to
b86111d
Compare
When creating a new artifact (e.g. Model or Repository) while the target directory does not exist yet (e.g.
src/modelsorsrc/repositories), we used to abort CLI with a ENOENT error.This commit is fixing our CLI to treat the missing directory as if it was empty and continue the operation if possible.
Checklist
👉 Read and sign the CLA (Contributor License Agreement) 👈
npm testpasses on your machinepackages/cliwere updatedexamples/*were updated👉 Check out how to submit a PR 👈