Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

fix: ipfs get with raw blocks #3683

Merged
merged 6 commits into from
May 11, 2021
Merged

fix: ipfs get with raw blocks #3683

merged 6 commits into from
May 11, 2021

Conversation

olizilla
Copy link
Member

  • tweak ipfs-cli get to handle raw
  • tweak ipfs-core mapFile to handle raw

fixes #3682

License: MIT
Signed-off-by: Oli Evans [email protected]

- tweak ipfs-cli get to handle `raw`
- tweak ipfs-core mapFile to handle `raw`

fixes #3682

License: MIT
Signed-off-by: Oli Evans <[email protected]>
@@ -103,28 +103,32 @@ const mapFile = (file, options = {}) => {
name: file.name,
depth: file.path.split('/').length,
size: 0,
type: 'file'
type: file.type
Copy link
Member

@achingbrain achingbrain May 10, 2021

Choose a reason for hiding this comment

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

Types are broken - output has a type of IPFSEntry - IPFSEntry.type can only be 'dir' or 'file'. file.type has more possible values than that so the typecheck fails.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to check, should the value of output.type be file when file.type === 'raw' ?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok so i guess I need to push the "can't deal with objects or identity" check down into mapFiles.

Copy link
Member

Choose a reason for hiding this comment

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

objects or identity

AFAIK these parts of UnixFS aren't used by anything.

should the value of output.type be file when file.type === 'raw'

Yes, unless go-ipfs does something different 🙂

License: MIT
Signed-off-by: Oli Evans <[email protected]>
- mapFile doesn't handle all mapping object or identity types to an IPFSEntry, so throw if it finds one. This makes the cli behaviour match go-ipfs for ipfs get <cbor cid>

License: MIT
Signed-off-by: Oli Evans <[email protected]>
@olizilla
Copy link
Member Author

@achingbrain i think i have sated the type machine

- add interface-core test for get with rawLeaves: true
- remove cli test for "raw" as core normalises the type to "file" so it was not testing a valid conditon

License: MIT
Signed-off-by: Oli Evans <[email protected]>
@olizilla
Copy link
Member Author

olizilla commented May 10, 2021

Nope, more noodling was required. The cli test was failing was testing a condition that the core normalises out, so that is removed. Meanwhile the interface-core tests were missing a test for getting a block that was added with rawLeaves: true so that is added. I can confirm that that test would fail on default branch and passes (locally at least) on this branch with the changes to the mapFiles fn.

@achingbrain
Copy link
Member

achingbrain commented May 11, 2021

Linting is failling?

ipfs-core: /home/travis/build/ipfs/js-ipfs/packages/ipfs-core/src/utils.js:130:1: Trailing spaces not allowed. [Error/no-trailing-spaces]

@achingbrain
Copy link
Member

Also the new test doesn't seem to work over http:

ipfs:   1) interface-ipfs-core over ipfs-http-client tests against js-ipfs
ipfs:        .get
ipfs:          should get a file added as CIDv1 with rawLeaves:
ipfs:      Error: Timeout of 120000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (/home/travis/build/ipfs/js-ipfs/packages/ipfs/test/interface-http-js.js)
ipfs:       at listOnTimeout (internal/timers.js:554:17)
ipfs:       at processTimers (internal/timers.js:497:7)

@olizilla
Copy link
Member Author

yes i've been leaning on ci to lint and test as it's not working locally. am trying a npm run reset && npm i now. I'll take a look at the http test failure.

❯ npm run lint                                                                                                                                                        09:23:32

> [email protected] lint
> lerna run lint

lerna notice cli v3.22.1
lerna info versioning independent
lerna info Executing command in 16 packages: "npm run lint"
interface-ipfs-core: > [email protected] lint
interface-ipfs-core: > aegir lint
ipfs-core-types: > [email protected] lint
ipfs-core-types: > aegir lint
ipfs-core-types: [09:23:42] Lint files [started]
interface-ipfs-core: [09:23:42] Lint files [started]
interface-ipfs-core: [09:23:44] Lint files [failed]
interface-ipfs-core: [09:23:44] → Plugin "no-only-tests" was conflicted between "package.json » eslint-config-ipfs#overrides[0] » ./js" and "../../package.json » eslint-config-ipfs#overrides[0] » ./js".
interface-ipfs-core: Plugin "no-only-tests" was conflicted between "package.json » eslint-config-ipfs#overrides[0] » ./js" and "../../package.json » eslint-config-ipfs#overrides[0] » ./js".
lerna ERR! npm run lint exited 1 in 'interface-ipfs-core'
lerna WARN complete Waiting for 1 child process to exit. CTRL-C to exit immediately.

@achingbrain
Copy link
Member

it's not working locally

interface-ipfs-core: [09:23:44] → Plugin "no-only-tests" was conflicted between "package.json » eslint-config-ipfs#overrides[0] » ./js" and "../../package.json » eslint-config-ipfs#overrides[0] » ./js".

Do you have a rogue package.json at ../../package.json that has a dep on a different version of eslint-plugin-no-only-tests somehow, or some other weird eslint config?

The ./js it refers to I think is this one, but it looks like it's resolving some other eslint config in your path or env somewhere that's causing the conflict.

@olizilla
Copy link
Member Author

running reset and install has fixed it. I'll get those checks green yet.

License: MIT
Signed-off-by: Oli Evans <[email protected]>
@olizilla
Copy link
Member Author

The only place I can see an issue could occur for fetching a rawLeaves node via the js-ipfs http api, that doesn't occur on the ipfs-core get path would be here

if (file.type === 'file' && file.content != null) {
yield { header: { ...header, size: file.size }, body: toBuffer(file.content) }
} else {

Is there an easy way to run the interface core get tests against my local js-ipfs code and see the server side console log output? I've running npm run test:interface:http-go from the root which is working well, but I need to see what's going on.

@achingbrain
Copy link
Member

I need to see what's going on.

Run with the DEBUG env var, e.g.:

$ DEBUG=ipfs* npm run test:interface:http-js -- -- -- --grep 'should get a file added as CIDv1 with rawLeaves'

It's probably more legible if you run it from the packages/ipfs dir as you'll get colour-coded output without lerna getting in the way and stripping it:

$ cd packages/ipfs
$ DEBUG=ipfs* npm run test:interface:http-js -- --grep 'should get a file added as CIDv1 with rawLeaves'

License: MIT
Signed-off-by: Oli Evans <[email protected]>
@olizilla
Copy link
Member Author

the file.size wasn't being set for raw nodes. Setting it fixes the test.

/** @type {import('ipfs-core-types/src/root').IPFSEntry} */
const output = {
cid: file.cid,
path: file.path,
name: file.name,
depth: file.path.split('/').length,
size: 0,
size: file.size,
Copy link
Member Author

Choose a reason for hiding this comment

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

is it legit to pass through the raw size for a raw node?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it'll be the size of the block

@achingbrain
Copy link
Member

CI failure is dns related, I've restarted the job & will merge when it's green.

@achingbrain achingbrain merged commit 28235b0 into master May 11, 2021
@achingbrain achingbrain deleted the get-raw branch May 11, 2021 18:11
@achingbrain
Copy link
Member

Shipped in [email protected]

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

ipfs get with single raw unixfs block writes empty dir
2 participants