Skip to content

Commit c779ca5

Browse files
rsonghusterrsonghuster
authored
Merge pull request #132 from devsapp/model-remove-error
fix: model NAS being inaccessible error skip and retry logic
2 parents 39c3d67 + 85dfbc9 commit c779ca5

9 files changed

Lines changed: 818 additions & 287 deletions

File tree

‎__tests__/e2e/nodejs/test-permission-code/index.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
1-
module.exports.handler = async function(event, context, callback) {
1+
module.exports.handler = async function (event, context, callback) {
22
return 'hello world';
3-
};
3+
};

‎__tests__/ut/commands/artModelService_test.ts‎

Lines changed: 178 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,20 @@ jest.mock('../../../src/logger', () => {
2929
jest.mock('@alicloud/devs20230714');
3030
jest.mock('@alicloud/openapi-client');
3131
jest.mock('../../../src/utils');
32-
jest.mock('../../../src/subCommands/model/utils');
32+
// Mock the utils module with specific functions
33+
jest.mock('../../../src/subCommands/model/utils', () => {
34+
const originalModule = jest.requireActual('../../../src/subCommands/model/utils');
35+
const mockRetryFileManagerRsyncAndCheckStatus = jest.fn();
36+
return {
37+
__esModule: true,
38+
...originalModule,
39+
retryWithFileManager: jest.fn((command, fn) => fn()),
40+
retryFileManagerRsyncAndCheckStatus: mockRetryFileManagerRsyncAndCheckStatus,
41+
initClient: jest.fn(),
42+
checkModelStatus: jest.fn(),
43+
extractOssMountDir: jest.fn(),
44+
};
45+
});
3346

3447
describe('ArtModelService', () => {
3548
let artModelService: ArtModelService;
@@ -91,11 +104,40 @@ describe('ArtModelService', () => {
91104
(sleep as jest.Mock).mockResolvedValue(undefined);
92105
});
93106

107+
it('should skip download if modelConfig.mode is "never"', async () => {
108+
const name = 'test-project$test-env$test-function';
109+
const params = {
110+
modelConfig: {
111+
mode: 'never',
112+
source: {
113+
uri: 'modelscope://test-model',
114+
},
115+
target: {
116+
uri: 'nas://auto',
117+
},
118+
files: [
119+
{
120+
source: { path: 'file1.txt' },
121+
target: { path: 'file1.txt' },
122+
},
123+
],
124+
},
125+
storage: 'nas',
126+
nasMountPoints: [{ mountDir: '/mnt/test' }],
127+
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
128+
region: 'cn-hangzhou',
129+
vpcConfig: {},
130+
};
131+
132+
await expect(artModelService.downloadModel(name, params)).resolves.toBeUndefined();
133+
expect(require('../../../src/subCommands/model/utils').initClient).toHaveBeenCalled();
134+
});
135+
94136
it('should skip download if all files already exist and are finished', async () => {
95137
const name = 'test-project$test-env$test-function';
96138
const params = {
97139
modelConfig: {
98-
mode: 'once', // 添加mode属性
140+
mode: 'once',
99141
source: {
100142
uri: 'modelscope://test-model',
101143
},
@@ -130,7 +172,7 @@ describe('ArtModelService', () => {
130172
},
131173
parameters: {
132174
// 确保这些参数与实际生成的路径匹配
133-
destination: 'file:///mnt/test/file1.txt',
175+
destination: 'file://mnt/test/file1.txt',
134176
source: 'modelscope://test-model/file1.txt',
135177
},
136178
},
@@ -140,14 +182,14 @@ describe('ArtModelService', () => {
140182
} as any);
141183

142184
// 当所有文件已经下载完成时，应该正常结束而不抛出异常
143-
await expect(artModelService.downloadModel(name, params)).rejects.toThrow();
185+
await expect(artModelService.downloadModel(name, params)).resolves.toBeUndefined();
144186
});
145187

146188
it('should successfully download files when no existing tasks', async () => {
147189
const name = 'test-project$test-env$test-function';
148190
const params = {
149191
modelConfig: {
150-
mode: 'always', // 添加mode属性
192+
mode: 'always',
151193
source: {
152194
uri: 'modelscope://test-model',
153195
},
@@ -186,36 +228,8 @@ describe('ArtModelService', () => {
186228
},
187229
} as any);
188230

189-
// 模拟合理的下载时间，避免超时
190-
const startTime = Date.now() - 5000; // 5秒前开始
191-
mockDevClient.getFileManagerTask
192-
.mockResolvedValueOnce({
193-
body: {
194-
data: {
195-
finished: false,
196-
startTime: startTime,
197-
progress: {
198-
currentBytes: 512,
199-
totalBytes: 1024,
200-
},
201-
},
202-
},
203-
} as any)
204-
.mockResolvedValueOnce({
205-
body: {
206-
data: {
207-
finished: true,
208-
success: true,
209-
startTime: startTime,
210-
finishedTime: Date.now(),
211-
progress: {
212-
currentBytes: 1024,
213-
totalBytes: 1024,
214-
total: true,
215-
},
216-
},
217-
},
218-
} as any);
231+
// ArtModelService内部会调用checkModelStatus，所以我们需要模拟它
232+
(checkModelStatus as jest.Mock).mockResolvedValue(undefined);
219233

220234
// 成功下载应该正常完成而不抛出异常
221235
await expect(artModelService.downloadModel(name, params)).resolves.toBeUndefined();
@@ -225,7 +239,7 @@ describe('ArtModelService', () => {
225239
const name = 'test-project$test-env$test-function';
226240
const params = {
227241
modelConfig: {
228-
mode: 'always', // 添加mode属性
242+
mode: 'always',
229243
source: {
230244
uri: 'modelscope://test-model',
231245
},
@@ -254,13 +268,11 @@ describe('ArtModelService', () => {
254268
},
255269
} as any);
256270

257-
mockDevClient.fileManagerRsync.mockResolvedValue({
258-
body: {
259-
success: false,
260-
data: {},
261-
requestId: 'req-123',
262-
},
263-
} as any);
271+
// 现在使用重试函数，我们需要模拟重试函数抛出错误
272+
(
273+
require('../../../src/subCommands/model/utils')
274+
.retryFileManagerRsyncAndCheckStatus as jest.Mock
275+
).mockRejectedValue(new Error('Download failed'));
264276

265277
// 下载失败应该抛出异常
266278
await expect(artModelService.downloadModel(name, params)).rejects.toThrow();
@@ -270,7 +282,7 @@ describe('ArtModelService', () => {
270282
const name = 'test-project$test-env$test-function';
271283
const params = {
272284
modelConfig: {
273-
mode: 'always', // 添加mode属性
285+
mode: 'always',
274286
source: {
275287
uri: 'modelscope://test-model',
276288
},
@@ -324,8 +336,11 @@ describe('ArtModelService', () => {
324336
},
325337
} as any);
326338

327-
// 模拟 checkModelStatus 抛出超时错误
328-
(checkModelStatus as jest.Mock).mockRejectedValue(new Error('Timeout'));
339+
// 现在使用重试函数，我们需要模拟重试函数抛出超时错误
340+
(
341+
require('../../../src/subCommands/model/utils')
342+
.retryFileManagerRsyncAndCheckStatus as jest.Mock
343+
).mockRejectedValue(new Error('Timeout'));
329344

330345
// 超时应该抛出异常
331346
await expect(artModelService.downloadModel(name, params)).rejects.toThrow();
@@ -335,7 +350,7 @@ describe('ArtModelService', () => {
335350
const name = 'test-project$test-env$test-function';
336351
const params = {
337352
modelConfig: {
338-
mode: 'always', // 添加mode属性
353+
mode: 'always',
339354
source: {
340355
uri: 'modelscope://test-model',
341356
},
@@ -374,8 +389,11 @@ describe('ArtModelService', () => {
374389
},
375390
} as any);
376391

377-
// 模拟 checkModelStatus 抛出错误
378-
(checkModelStatus as jest.Mock).mockRejectedValue(new Error('Download failed'));
392+
// 现在使用重试函数，我们需要模拟重试函数抛出错误
393+
(
394+
require('../../../src/subCommands/model/utils')
395+
.retryFileManagerRsyncAndCheckStatus as jest.Mock
396+
).mockRejectedValue(new Error('Download failed'));
379397

380398
// 下载失败应该抛出异常
381399
await expect(artModelService.downloadModel(name, params)).rejects.toThrow();
@@ -385,7 +403,7 @@ describe('ArtModelService', () => {
385403
const name = 'test-project$test-env$test-function';
386404
const params = {
387405
modelConfig: {
388-
mode: 'always', // 添加mode属性
406+
mode: 'always',
389407
source: {
390408
uri: 'modelscope://test-model',
391409
},
@@ -404,6 +422,72 @@ describe('ArtModelService', () => {
404422
// 空文件列表应该正常完成
405423
await expect(artModelService.downloadModel(name, params)).resolves.toBeUndefined();
406424
});
425+
426+
it('should use MODEL_CONFLIC_HANDLING environment variable for conflict handling', async () => {
427+
const name = 'test-project$test-env$test-function';
428+
const params = {
429+
modelConfig: {
430+
mode: 'always',
431+
source: {
432+
uri: 'modelscope://test-model',
433+
},
434+
target: {
435+
uri: 'nas://auto',
436+
},
437+
files: [
438+
{
439+
source: { path: 'file1.txt' },
440+
target: { path: 'file1.txt' },
441+
},
442+
],
443+
conflictResolution: 'overwrite', // 这个值应该被环境变量覆盖
444+
},
445+
storage: 'nas',
446+
nasMountPoints: [{ mountDir: '/mnt/test' }],
447+
role: 'acs:ram::123456789:role/aliyundevsdefaultrole',
448+
region: 'cn-hangzhou',
449+
vpcConfig: {},
450+
};
451+
452+
// 设置环境变量
453+
const originalValue = process.env.MODEL_CONFLIC_HANDLING;
454+
process.env.MODEL_CONFLIC_HANDLING = 'skip';
455+
456+
mockDevClient.listFileManagerTasks.mockResolvedValue({
457+
body: {
458+
data: {
459+
tasks: [],
460+
},
461+
},
462+
} as any);
463+
464+
mockDevClient.fileManagerRsync.mockResolvedValue({
465+
body: {
466+
success: true,
467+
data: {
468+
taskID: 'task-123',
469+
},
470+
requestId: 'req-123',
471+
},
472+
} as any);
473+
474+
// 现在使用重试函数，我们需要模拟重试函数成功执行
475+
(
476+
require('../../../src/subCommands/model/utils')
477+
.retryFileManagerRsyncAndCheckStatus as jest.Mock
478+
).mockResolvedValue(undefined);
479+
480+
// 成功下载应该正常完成而不抛出异常
481+
await expect(artModelService.downloadModel(name, params)).resolves.toBeUndefined();
482+
483+
// 由于现在使用了重试函数，我们验证重试函数被调用
484+
expect(
485+
require('../../../src/subCommands/model/utils').retryFileManagerRsyncAndCheckStatus,
486+
).toHaveBeenCalled();
487+
488+
// 恢复环境变量
489+
process.env.MODEL_CONFLIC_HANDLING = originalValue;
490+
});
407491
});
408492

409493
describe('removeModel', () => {
@@ -536,6 +620,39 @@ describe('ArtModelService', () => {
536620
});
537621
});
538622

623+
it('should handle source URI with reversion', () => {
624+
const result = artModelService.getSourceAndDestination(
625+
'modelscope://test-model',
626+
{
627+
source: { path: 'file1.txt', uri: 'modelscope://different-model' },
628+
target: { path: 'file1.txt' },
629+
},
630+
[{ mountDir: '/mnt/nas' }],
631+
[{ mountDir: '/mnt/oss' }],
632+
'nas://auto',
633+
);
634+
635+
expect(result).toEqual({
636+
source: 'modelscope://different-model/file1.txt',
637+
destination: 'file://mnt/nas/file1.txt',
638+
});
639+
});
640+
641+
it('should handle target URI with reversion', () => {
642+
const result = artModelService.getSourceAndDestination(
643+
'modelscope://test-model',
644+
{ source: { path: 'file1.txt' }, target: { path: 'file1.txt', uri: 'oss://auto' } },
645+
[{ mountDir: '/mnt/nas' }],
646+
[{ mountDir: '/mnt/oss' }],
647+
'nas://auto',
648+
);
649+
650+
expect(result).toEqual({
651+
source: 'modelscope://test-model/file1.txt',
652+
destination: 'file://mnt/oss/file1.txt',
653+
});
654+
});
655+
539656
it('should handle invalid source URI', () => {
540657
expect(() => {
541658
artModelService.getSourceAndDestination(
@@ -615,5 +732,16 @@ describe('ArtModelService', () => {
615732

616733
expect(result).toBe('file://mnt/custom/file1.txt');
617734
});
735+
736+
it('should handle target path starting with slash', () => {
737+
const result = (artModelService as any)._getDestinationPath(
738+
'nas://mnt/custom',
739+
{ target: { path: '/file1.txt' } }, // Path starting with slash
740+
[{ mountDir: '/mnt/nas' }],
741+
[{ mountDir: '/mnt/oss' }],
742+
);
743+
744+
expect(result).toBe('file://mnt/custom/file1.txt'); // Should remove leading slash
745+
});
618746
});
619747
});

0 commit comments

Comments
 (0)