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

fix: files.add with pull streams + ability to seek #1319

Merged
merged 10 commits into from
May 6, 2018
Merged

fix: files.add with pull streams + ability to seek #1319

merged 10 commits into from
May 6, 2018

Conversation

alanshaw
Copy link
Member

@ghost ghost assigned alanshaw Apr 20, 2018
@ghost ghost added the status/in-progress In progress label Apr 20, 2018
This was referenced Apr 20, 2018
Closed
Closed
@daviddias
Copy link
Member

I understand that this PR needs the latest of js-ipfs-api. I'm having a hard time releasing js-ipfs-api. Too many timeouts. Wanna take a look at that?

@alanshaw
Copy link
Member Author

Sure, I'll take a look now

@alanshaw
Copy link
Member Author

Ok so, for js-ipfs-api:

  1. The CircleCI build is failing because it's using Node.js 10 and the native modules are failing to compile
  2. Locally, if using Node 9.9.0+ all the tests fail because of this issue in ipfsd-ctl
  3. name.publish just takes ages, I've submitted a PR to increase the timeout on this test here fix: increase timeout for name.publish and fix setup code ipfs-inactive/js-ipfs-http-client#749

@daviddias
Copy link
Member

Needs ipfs-inactive/js-ipfs-http-client#751

@daviddias daviddias mentioned this pull request Apr 30, 2018
Closed
30 tasks
@ghost ghost assigned daviddias Apr 30, 2018
@daviddias daviddias mentioned this pull request Apr 30, 2018
Merged
@daviddias
Copy link
Member

@alanshaw something is not working properly in this PR.

@alanshaw
Copy link
Member Author

alanshaw commented May 1, 2018 •
edited
Loading

According to the CI the failing tests are to do with files.cat...not something that this PR should have affected 😟 - I'll take a look.

daviddias
daviddias suggested changes May 1, 2018
View reviewed changes
Copy link
Member

@daviddias daviddias left a comment

Choose a reason for hiding this comment

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

@alanshaw Master is currently working (check Travis and Jenkins). Perhaps was a change on the test suite?

package.json Outdated
@@ -73,7 +73,7 @@
"expose-loader": "^0.7.5",
"form-data": "^2.3.2",
"hat": "0.0.3",
"interface-ipfs-core": "^0.61.0",
"interface-ipfs-core": "^0.64.2",
Copy link
Member

Choose a reason for hiding this comment

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

@alanshaw something might have changed here

@alanshaw
Copy link
Member Author

alanshaw commented May 1, 2018

@diasdavid yes that's it - the tests are failing because the new interface-ipfs-core has tests for files.cat with offset and length - this functionality hasn't been merged yet #1231

This was referenced May 1, 2018
Merged
Closed
@achingbrain @daviddias
feat: Allows for byte offsets when using ipfs.files.cat and friends t…
fff0aa1 
…o request slices of files

feat: Allows for byte offsets when using ipfs.files.cat and friends to request slices of files
@daviddias daviddias assigned achingbrain and unassigned daviddias May 1, 2018
@daviddias daviddias changed the title fix: files.add with pull streams fix: files.add with pull streams + ability to seek May 1, 2018
@achingbrain
Copy link
Member

@diasdavid CI seems happy with this now but some of our (unrelated to this PR) tests time out occasionally.

daviddias
daviddias reviewed May 3, 2018
View reviewed changes
circle.yml Outdated
@@ -1,7 +1,7 @@
# Warning: This file is automatically synced from https://github.com/ipfs/ci-sync so if you want to change it, please change it there and ask someone to sync all repositories.
machine:
node:
version: 9
Copy link
Member

Choose a reason for hiding this comment

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

Use LTS please

@daviddias daviddias merged commit 7a027cc into master May 6, 2018
@ghost ghost removed the status/in-progress In progress label May 6, 2018
@daviddias daviddias deleted the fix/files.add-pull-streams branch June 4, 2018 11:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@daviddias daviddias daviddias approved these changes

Assignees

@alanshaw alanshaw

@achingbrain achingbrain

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

files.add with pull stream does not work
4 participants
@alanshaw @daviddias @achingbrain @vasco-santos