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

feat: upload-client uploadDirectory, by default, sorts the provided files by file name to help the user call us in a way that is deterministic and minimizes cost #1173

Merged
merged 37 commits into from
Nov 29, 2023

Conversation

gobengo
Copy link
Contributor

@gobengo gobengo commented Nov 21, 2023

Motivation:

…ded files are sorted by file name to help the user call us in a way that is deterministic and saves them money
@gobengo gobengo self-assigned this Nov 21, 2023
Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

Ok, this is not the PR I was expecting! I was expecting a tweak to automatically sort the files list and perhaps an option to disable that behaviour.

So something like this: storacha/files-from-path#27

My assumption was that the majority of users would want this to happen automatically and by pushing that responsibility onto users we're forcing a breaking change as well as adding an additional code task for them to sort the files themselves.

I've made some suggestions in the PR but on balance I think we should more preferably do the above (i.e. same as files-from-path) instead.

packages/upload-client/src/sharding.js Outdated Show resolved Hide resolved
packages/upload-client/src/index.js Outdated Show resolved Hide resolved
packages/upload-client/src/index.js Outdated Show resolved Hide resolved
packages/upload-client/src/sharding.js Outdated Show resolved Hide resolved
packages/upload-client/src/index.js Outdated Show resolved Hide resolved
gobengo and others added 19 commits November 28, 2023 09:09
its better if we direct folks to the docs or the workshopper rather than
try and explain it all here.

License: MIT

Signed-off-by: Oli Evans <[email protected]>
@juliangruber reported an issue with our documented space creation flow
when the exported space access delegation doesn't specify a name and the
client doesn't specify one manually:

#1175

I think it's reasonable to require a name for spaces at some level, but
I think the access-client API is too low-level for this.
This PR allows you to get a list of subscriptions for your account plan,
and also get the current per space usage (I've not exposed the full
report yet since we don't have any UI that would use it).

e.g.

Getting subscriptions list:

```js
const account = Object.values(client.accounts())[0]
const subs = await account.plan.subscriptions()
for (const sub of subs.ok) {
  console.log(`ID: ${sub.subscription}`)
  console.log(`Consumers: ${sub.consumers.join(', ')}`)
  console.log(`Provider: ${sub.provider}`)
}
```

Getting usage:

```js
const space = client.spaces()[0]
const usage = await space.usage.get()
console.log(`${space.did()}: ${usage.ok} bytes`)
```

So, you no longer have to invoke capabilities directly like this:

https://github.com/web3-storage/w3cli/blob/ca21f6736fb8c2180d3634f40931b91e4f0bb964/index.js#L553-L571
🤖 I have created a release *beep* *boop*
---


##
[12.0.3](capabilities-v12.0.2...capabilities-v12.0.3)
(2023-11-22)


### Fixes

* package metadata
([#1161](#1161))
([b8a1cc2](b8a1cc2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Alan Shaw <[email protected]>
🤖 I have created a release *beep* *boop*
---


##
[12.0.2](upload-client-v12.0.1...upload-client-v12.0.2)
(2023-11-22)


### Fixes

* package metadata
([#1161](#1161))
([b8a1cc2](b8a1cc2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Alan Shaw <[email protected]>
🤖 I have created a release *beep* *boop*
---


##
[18.0.3](access-v18.0.2...access-v18.0.3)
(2023-11-22)


### Fixes

* don't error when we can't figure out a name for a space
([#1177](#1177))
([a31f667](a31f667))
* package metadata
([#1161](#1161))
([b8a1cc2](b8a1cc2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Alan Shaw <[email protected]>
This was implemented in `capabilities` and `upload-api` but for some
reason was never hooked in the clients.
🤖 I have created a release *beep* *boop*
---


##
[12.1.0](upload-client-v12.0.2...upload-client-v12.1.0)
(2023-11-25)


### Features

* add store.get and upload.get to clients
([#1178](#1178))
([d1be42a](d1be42a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop*
---


##
[11.1.0](w3up-client-v11.0.2...w3up-client-v11.1.0)
(2023-11-25)


### Features

* add store.get and upload.get to clients
([#1178](#1178))
([d1be42a](d1be42a))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Alan Shaw <[email protected]>
These were previously not exported from `w3up-client` 😢
🤖 I have created a release *beep* *boop*
---


##
[11.1.1](w3up-client-v11.1.0...w3up-client-v11.1.1)
(2023-11-27)


### Fixes

* export filecoin types
([#1185](#1185))
([9b1f526](9b1f526))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop*
---


##
[11.1.2](w3up-client-v11.1.1...w3up-client-v11.1.2)
(2023-11-27)


### Fixes

* export ProgressStatus
([ab29c05](ab29c05))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
we rely on this in console to allow users to cancel login
🤖 I have created a release *beep* *boop*
---


##
[11.1.3](w3up-client-v11.1.2...w3up-client-v11.1.3)
(2023-11-28)


### Fixes

* thread abort signal through login functions
([#1189](#1189))
([8e908a9](8e908a9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
Conveniently store the timestamp of the oldest piece in table, so that
when monitoring aggregates time to deal for alerts, we do not need to
read buffer, decode it, and find out it every time a CRON runs.
Running batches of concurrency 20 in my machine handled in 2.40s a batch
of updates. A page query of `4046` in parallel trying to crawl the
entire chain was being too much and mostly the CRON would timeout before
things getting updated, causing cron to be struggling to move on
it-dag-house and others added 5 commits November 28, 2023 14:34
🤖 I have created a release *beep* *boop*
---


##
[3.1.3](filecoin-client-v3.1.2...filecoin-client-v3.1.3)
(2023-11-28)


### Fixes

* package metadata
([#1161](#1161))
([b8a1cc2](b8a1cc2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop*
---


##
[4.2.0](filecoin-api-v4.1.2...filecoin-api-v4.2.0)
(2023-11-28)


### Features

* aggregator keeping oldest piece ts
([#1188](#1188))
([97a7def](97a7def))


### Fixes

* package metadata
([#1161](#1161))
([b8a1cc2](b8a1cc2))
* storefront events cron with max concurrency
([#1191](#1191))
([11010c9](11010c9))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Vasco Santos <[email protected]>
🤖 I have created a release *beep* *boop*
---


##
[7.3.4](upload-api-v7.3.3...upload-api-v7.3.4)
(2023-11-28)


### Fixes

* package metadata
([#1161](#1161))
([b8a1cc2](b8a1cc2))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: Vasco Santos <[email protected]>
@gobengo gobengo changed the title feat: upload-client uploadDirectory, by default, checks to ensure the provided files are sorted by file name to help the user call us in a way that is deterministic and saves them money feat: upload-client uploadDirectory, by default, sorts the provided files by file name to help the user call us in a way that is deterministic and msinimizes cost Nov 28, 2023
@gobengo gobengo changed the title feat: upload-client uploadDirectory, by default, sorts the provided files by file name to help the user call us in a way that is deterministic and msinimizes cost feat: upload-client uploadDirectory, by default, sorts the provided files by file name to help the user call us in a way that is deterministic and minimizes cost Nov 28, 2023
@gobengo
Copy link
Contributor Author

gobengo commented Nov 28, 2023

My assumption was that the majority of users would want this to happen automatically and by pushing that responsibility onto users we're forcing a breaking change as well as adding an additional code task for them to sort the files themselves.

@alanshaw cool. I updated to make this match the expectations you shared.

Copy link
Member

@alanshaw alanshaw left a comment

Choose a reason for hiding this comment

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

❤️ Some minor feedback but nothing blocking.

packages/upload-client/src/index.js Outdated Show resolved Hide resolved
* @param {FileLike} b
* @param {(file: FileLike) => string} getSortKey
*/
export const defaultFileComparator = (a, b, getSortKey = (a) => a.name) => {
Copy link
Member

Choose a reason for hiding this comment

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

Minor, but getSortKey is actually returning a value not a key.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah I was using key in the sense of this https://docs.python.org/3/howto/sorting.html#key-functions
but I think you mean it like the 'key/value' of the File object, which is probably a more reasonable interpretation in this context than what I meant. I will rename it to getSortValue because that seems at least slightly less likely to cause confusion than what I did

@@ -258,7 +258,10 @@ export interface UploadOptions
export interface UploadDirectoryOptions
extends UploadOptions,
UnixFSDirectoryEncoderOptions,
UploadProgressTrackable {}
UploadProgressTrackable {
// whether the directory files have already been ordered in a custom way. indicates that the upload must not use a different order than the one provided.
Copy link
Member

Choose a reason for hiding this comment

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

This should be /** ... */ style comment in order to expose this helpful info to consumers.

@gobengo gobengo merged commit 8cd2555 into main Nov 29, 2023
4 checks passed
@gobengo gobengo deleted the 1172-uploadDirectory-sorted branch November 29, 2023 19:18
gobengo pushed a commit that referenced this pull request Dec 1, 2023
🤖 I have created a release *beep* *boop*
---


##
[12.2.0](upload-client-v12.1.0...upload-client-v12.2.0)
(2023-11-29)


### Features

* upload-client uploadDirectory, by default, sorts the provided files by
file name to help the user call us in a way that is deterministic and
minimizes cost
([#1173](#1173))
([8cd2555](8cd2555))


### Fixes

* floating promises and add no-floating-promises to eslint-config-w3up
([#1198](#1198))
([1b8c5aa](1b8c5aa))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).
vasco-santos added a commit that referenced this pull request Jan 15, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.3.1](filecoin-api-v4.3.0...filecoin-api-v4.3.1)
(2024-01-15)


### ⚠ BREAKING CHANGES

* return allocated bytes in `store/add` receipt
([#1213](#1213))

### Features

* return allocated bytes in `store/add` receipt
([#1213](#1213))
([5d52e44](5d52e44))
* upload-client uploadDirectory, by default, sorts the provided files by
file name to help the user call us in a way that is deterministic and
minimizes cost
([#1173](#1173))
([8cd2555](8cd2555))


### Fixes

* avoid duplicates on aggregator buffer concat
([#1259](#1259))
([9e64bab](9e64bab))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

---------

Co-authored-by: Vasco Santos <[email protected]>
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.

6 participants