Skip to content

Commit

Permalink
refactor download command use golang error style insted of try catch (#…
Browse files Browse the repository at this point in the history
…31)

* refactored download command to use golang error style insted of try catch

* Improved PR
  • Loading branch information
Abse2001 authored Feb 5, 2025
1 parent cae9347 commit 03a5f38
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 24 deletions.
116 changes: 104 additions & 12 deletions cli/download/download-dataset.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,63 @@ export async function downloadDatasetToDirectory({
datasetNameWithOwner,
outputDirectory,
ky = defaultKy,
}: DownloadDatasetOptions) {
}: DownloadDatasetOptions): Promise<string> {
const [author, datasetName] = datasetNameWithOwner.split("/")
const outputPath = join(outputDirectory, `${author}.${datasetName}`)

// Get dataset info
const dataset = await ky
const [datasetResponse, datasetError] = await ky
.get("datasets/get", {
searchParams: {
dataset_name_with_owner: datasetNameWithOwner,
},
})
.json<{ dataset: { dataset_id: string; sample_count: number } }>()
.then((data) => [data, null])
.catch((err) => [null, err])
if (datasetError) {
throw new Error(
`Failed to fetch dataset "${datasetNameWithOwner}". Please check if the dataset exists and you have access to it. Original error: ${datasetError.message}`,
)
}

// Create the main dataset directory
await mkdir(outputPath, { recursive: true })
const [mkdirResult, mkdirError] = await mkdir(outputPath, { recursive: true })
.then(() => [true, null])
.catch((err) => [null, err])

if (mkdirError) {
throw new Error(
`Failed to create dataset directory "${outputPath}". Original error: ${mkdirError.message}`,
)
}

if (author && datasetName) {
await createPackageJson(outputPath, { author, datasetName })
const [packageJsonResult, packageJsonError] = await createPackageJson(
outputPath,
{ author, datasetName },
)
.then(() => [true, null])
.catch((err) => [null, err])

if (packageJsonError) {
throw new Error(
`Failed to create package.json in "${outputPath}". Original error: ${packageJsonError.message}`,
)
}
}

// Download each sample
for (
let sampleNumber = 1;
sampleNumber <= dataset.dataset.sample_count;
sampleNumber <= datasetResponse.dataset.sample_count;
sampleNumber++
) {
// Get sample info with available files
const sampleResponse = await ky
const [sampleResponse, sampleError] = await ky
.get("samples/get", {
searchParams: {
dataset_id: dataset.dataset.dataset_id,
dataset_id: datasetResponse.dataset.dataset_id,
sample_number: sampleNumber,
},
})
Expand All @@ -52,26 +79,91 @@ export async function downloadDatasetToDirectory({
available_file_paths: string[]
}
}>()
.then((data) => [data, null])
.catch((err) => [null, err])

if (sampleError) {
throw new Error(
`Failed to fetch sample ${sampleNumber} for dataset "${datasetNameWithOwner}". Original error: ${sampleError.message}`,
)
}

const sample = sampleResponse.sample
const sampleDir = join(outputPath, `sample${sampleNumber}`)
await mkdir(sampleDir, { recursive: true })

const [sampleDirResult, sampleDirError] = await mkdir(sampleDir, {
recursive: true,
})
.then(() => [true, null])
.catch((err) => [null, err])

if (sampleDirError) {
throw new Error(
`Failed to create directory for sample ${sampleNumber} at "${sampleDir}". Original error: ${sampleDirError.message}`,
)
}

// Download each file for the sample
for (const filePath of sample.available_file_paths) {
const fileContent = await ky
const [fileContent, fileError] = await ky
.get("samples/view_file", {
searchParams: {
sample_id: sample.sample_id,
file_path: filePath,
},
})
.text()
.then((data) => [data, null])
.catch((err) => [null, err])

if (fileError) {
throw new Error(
`Failed to fetch file "${filePath}" for sample ${sampleNumber}. Original error: ${fileError.message}`,
)
}

if (filePath.includes("_routed")) {
await mkdir(join(sampleDir, "outputs"), { recursive: true })
await writeFile(join(sampleDir, "outputs", filePath), fileContent)
const [outputsDirResult, outputsDirError] = await mkdir(
join(sampleDir, "outputs"),
{ recursive: true },
)
.then(() => [true, null])
.catch((err) => [null, err])

if (outputsDirError) {
throw new Error(
`Failed to create outputs directory for sample ${sampleNumber} at "${sampleDir}/outputs". Original error: ${outputsDirError.message}`,
)
}

const [writeOutputResult, writeError] = await writeFile(
join(sampleDir, "outputs", filePath),
fileContent,
)
.then(() => [true, null])
.catch((err) => [null, err])

if (writeError) {
throw new Error(
`Failed to write file "${filePath}" for sample ${sampleNumber}. Original error: ${writeError.message}`,
)
}

continue
}
await writeFile(join(sampleDir, filePath), fileContent)

const [writeResult, writeError] = await writeFile(
join(sampleDir, filePath),
fileContent,
)
.then(() => [true, null])
.catch((err) => [null, err])

if (writeError) {
throw new Error(
`Failed to write file "${filePath}" for sample ${sampleNumber}. Original error: ${writeError.message}`,
)
}
}
}

Expand Down
16 changes: 6 additions & 10 deletions cli/download/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,11 @@ export const datasetDownloadCommand = (program: Command) => {
)
.option("-o, --output <directory>", "Output directory", cwd())
.action(async (datasetName, options) => {
try {
const outputPath = await downloadDatasetToDirectory({
datasetNameWithOwner: datasetName,
outputDirectory: options.output,
})
console.log(`Successfully downloaded dataset to: ${outputPath}`)
} catch (error) {
console.error("Failed to download dataset:", error)
process.exit(1)
}
const outputPath = await downloadDatasetToDirectory({
datasetNameWithOwner: datasetName,
outputDirectory: options.output,
})

console.log(`Successfully downloaded dataset to: ${outputPath}`)
})
}
56 changes: 56 additions & 0 deletions tests/cli/download/download-dataset.error.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { it, expect, describe } from "bun:test"
import { getTestServer } from "tests/fixtures/get-test-server"
import { temporaryDirectory } from "tempy"
import { downloadDatasetToDirectory } from "@/cli/download/download-dataset"
import { join } from "path"

describe("downloadDatasetToDirectory error cases", () => {
it("should return error for non-existent dataset", async () => {
const { ky } = await getTestServer()
const tempDir = temporaryDirectory()

await expect(
downloadDatasetToDirectory({
datasetNameWithOwner: "nonexistent/dataset",
outputDirectory: tempDir,
ky,
}),
).rejects.toThrow(
/Failed to fetch dataset "nonexistent\/dataset".*Please check if the dataset exists and you have access to it.*Original error/,
)
})

it("should return error for invalid output directory", async () => {
const { ky } = await getTestServer()

const invalidDir = join(
"/",
"nonexistent",
"path",
"that",
"should",
"fail",
)

await expect(
downloadDatasetToDirectory({
datasetNameWithOwner: "testuser/custom-keyboards",
outputDirectory: invalidDir,
ky,
}),
).rejects.toThrow(/Failed to create dataset directory.*Original error/)
})

it("should return error for failed file download", async () => {
const tempDir = temporaryDirectory()

await expect(
downloadDatasetToDirectory({
datasetNameWithOwner: "testuser/custom-keyboards",
outputDirectory: tempDir,
}),
).rejects.toThrow(
/Failed to fetch dataset "testuser\/custom-keyboards".*Please check if the dataset exists and you have access to it.*Original error/,
)
})
})
1 change: 0 additions & 1 deletion tests/cli/run/run-autorouter-local1.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ test("should run local autorouter on a single circuit file", async () => {
await mkdir(sampleDir, { recursive: true })
await createMockFiles(sampleDir)
const inputPath = join(sampleDir, "unrouted_circuit.json")
console.log("inputPath", inputPath)
// Run command
await runAutorouter({
inputPath,
Expand Down
1 change: 0 additions & 1 deletion tests/cli/run/run-autorouter-local2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ test("should run local autorouter on a dataset directory", async () => {
await createMockCircuitFile(fullSamplePath)
}

console.log("Running local autorouter with dataset directory:", datasetDir)
// Run command
await runAutorouter({
inputPath: datasetDir,
Expand Down

0 comments on commit 03a5f38

Please sign in to comment.