Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IndexOptionsConflict for chunk collection - pr #372

Merged
merged 6 commits into from
Aug 10, 2022

Conversation

sekko27
Copy link
Contributor

@sekko27 sekko27 commented Aug 10, 2022

Comment on lines 35 to 49
chunkSizeBytes: 255 * 8,
});

await (await fetch("https://deno.land/images/deno_logo.png")).body!.pipeTo(
upstream,
);
await (await fetch("https://deno.land/images/artwork/snow_den.png")).body!
.pipeTo(
upstream,
);

const getId =
(await bucket.find({ filename: "deno_logo.png" }).toArray())[0]._id;

const expected = await fetch("https://deno.land/images/deno_logo.png");
const expected = await fetch("https://deno.land/images/artwork/snow_den.png");
const actual = await new Response(await bucket.openDownloadStream(getId))
.arrayBuffer();

Copy link
Member

@erfanium erfanium Aug 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please revert these changes, because we already localized these assets so this PR has merge conflicts

await writer.close();
};
await addAsset(0);
await addAsset(1);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is the assert in this code? Shouldn’t the test case somehow check if the indexes where skipped?

@lucsoft lucsoft requested a review from erfanium August 10, 2022 07:03
Copy link
Collaborator

@lucsoft lucsoft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

LGTM

@lucsoft lucsoft removed the request for review from erfanium August 10, 2022 07:04
@lucsoft lucsoft merged commit d45dd12 into denodrivers:main Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants