-
Notifications
You must be signed in to change notification settings - Fork 7
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
fix: find equivalent cids also if not car cid #327
fix: find equivalent cids also if not car cid #327
Conversation
View stack outputs
|
export async function findEquivalentCarCids (piece, fetchClaims = createClaimsClientForEnv()) { | ||
/** @type {Set<CARLink>} */ | ||
export async function findEquivalentCids (piece, fetchClaims = createClaimsClientForEnv()) { | ||
/** @type {Set<import('multiformats').UnknownLink>} */ | ||
const cids = new Set() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I think this is not going to dedupe like you think it will.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I also think this set will probably not guarantee . will create an issue, as this was like it and look after
Co-authored-by: Alan Shaw <alan.shaw@protocol.ai>
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
this was already happening on https://protocol-labs-it.sentry.io/issues/4439567707/?project=4506587250819072&project=4504253926539264&project=4504515009970176&query=is%3Aunresolved&referrer=issue-stream&statsPeriod=7d&stream_index=22 and is a failure from content claims service |
We should keep also equivalent CIDs that are not CARs to attempt in the bucket to get presigned URL
We will try for each of them to check if it is in bucket and generate presigned URL per: