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

fix: ability to upload the same file after deleting #197

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

IgorShadurin
Copy link
Collaborator

@IgorShadurin IgorShadurin commented Dec 15, 2022

Close #192

@IgorShadurin IgorShadurin marked this pull request as ready for review December 15, 2022 12:19
@IgorShadurin IgorShadurin requested a review from nugaon as a code owner December 15, 2022 12:19
Copy link
Collaborator

@nugaon nugaon left a comment

Choose a reason for hiding this comment

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

This whole fix seems wrong for me.
If there is a chunk conflict on feed upload, check whether the identifiers of SOC match or not, if so, the problem is there -> you shouldn't write to the same index.

src/account/account.ts Outdated Show resolved Hide resolved
src/feed/api.ts Outdated Show resolved Hide resolved
@IgorShadurin IgorShadurin requested a review from nugaon January 17, 2023 14:50
@IgorShadurin IgorShadurin force-pushed the fix/192-upload-deleted-file-again branch from f73bc81 to fd584fa Compare January 19, 2023 12:23
Copy link
Collaborator

@nugaon nugaon left a comment

Choose a reason for hiding this comment

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

so if I understand, the problem was the writeFeedDataRaw attempted to write on a reserved index since the next subsequent index was not calculated.
it is quite strange since it should have thrown an error because of writing on a reserved index.
maybe instead of

  if (!epoch) {
    epoch = new Epoch(HIGHEST_LEVEL, getUnixTimestamp())
  }

this snippet should find for the latest free index instead if epoch is not defined

package.json Outdated Show resolved Hide resolved
src/file/file.ts Outdated
Comment on lines 112 to 118
let nextEpoch
try {
const feedData = await getFeedData(connection.bee, fullPath, prepareEthAddress(podWallet.address))
feedData.epoch.level = feedData.epoch.getNextLevel(feedData.epoch.time)
nextEpoch = feedData.epoch
// eslint-disable-next-line no-empty
} catch (e) {}
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this snippet could be moved to writeFeedData to ensure the feed write will happen on the next free index, wdyt?

@@ -82,5 +82,7 @@ export async function writeFeedDataRaw(
const id = getId(topicHash, epoch.time, epoch.level)
const socWriter = connection.bee.makeSOCWriter(privateKey)

return socWriter.upload(connection.postageBatchId, id, data)
return await socWriter.upload(connection.postageBatchId, id, data, {
pin: true,
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do you need pinning? we do not require this

@IgorShadurin IgorShadurin force-pushed the fix/192-upload-deleted-file-again branch from fd584fa to ad5d959 Compare February 13, 2023 11:04
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.

fdp.file.uploadData does not allow overwrites
2 participants