Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

fix: offer to use collection instead of tenant #19

Merged
merged 1 commit into from
Sep 18, 2023
Merged

fix: offer to use collection instead of tenant #19

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/architecture.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ interface Offer {
aggregate: PieceCID
// CID of all the segments part of the aggregate
pieces: PieceCID[]
// identifier of the tenant for the storefront `did:web:web3.storage`
// spade relies on tenant naming, so we map it here to tenant
tenant: string
// identifier of the collection for the tenant `did:web:web3.storage`
// spade relies on collections to identify different replication constraints.
collection: string
Copy link
Member

Choose a reason for hiding this comment

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

Why not have both?

Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest naming things we store under names that makes senses to us, and then mapping them at the points where we interface with spade. This would be "storefront" for us right?

Copy link
Contributor Author

@vasco-santos vasco-santos Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is the bucket that Spade reads from us. In other words, this is the interface...

Copy link
Contributor Author

@vasco-santos vasco-santos Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Might be worth to read https://github.com/web3-storage/dealer/blob/main/docs/architecture.md#spade-integration + the issue where we iterated on format in calls with Spade team #7 (comment)
@alanshaw

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok got it!

// enables ordering of offers to handle.
// Being it the number of ms since epoch, also means offers that fail and are retried will have "priority"
// It can also have lower numbers to prioritize certain actors in the future
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/data/offer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const encode = {
const value = {
aggregate: dealRecord.aggregate.toString(),
pieces: dealRecord.pieces.map(p => p.toString()),
tenant: dealRecord.storefront,
collection: dealRecord.storefront,
orderID: dealRecord.insertedAt || Date.now()
} satisfies Offer

Expand Down Expand Up @@ -53,7 +53,7 @@ export const decode = {
return {
aggregate: parseLink(record.aggregate),
pieces: record.pieces.map(p => parseLink(p)),
storefront: record.tenant,
storefront: record.collection,
insertedAt: record.orderID
}
},
Expand All @@ -74,6 +74,6 @@ export type EncodedRecord = {
export type Offer = {
aggregate: string
pieces: string[]
tenant: string
collection: string
orderID: number
}
2 changes: 1 addition & 1 deletion test/api.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ test('POST /', async t => {
const encodedOffer = JSON.parse(toString(bucketItem))

t.is(encodedOffer.aggregate, aggregate.link.link().toString())
t.is(encodedOffer.tenant, invocationConfig.with)
t.is(encodedOffer.collection, invocationConfig.with)
t.truthy(encodedOffer.orderID)
t.deepEqual(encodedOffer.pieces, offer.map(o => o.toString()))
})