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(ipfs-manager): adapt RPC usage with POST methods #871

Merged
merged 10 commits into from
Jul 5, 2022

Conversation

benjlevesque
Copy link
Contributor

@benjlevesque benjlevesque commented Jun 23, 2022

modern versions of IPFS now require using POST for every calls

ipfs/kubo#7097
ipfs/js-ipfs#2971

Also in this PR: replace axios-retry since it seems to not retry POST requests

@@ -14,7 +14,7 @@ const invalidHostIpfsGatewayConnection: StorageTypes.IIpfsGatewayConnection = {
host: 'nonexistent',
port: 5001,
protocol: StorageTypes.IpfsGatewayProtocol.HTTP,
timeout: 1000,
timeout: 1500,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure what changed, but without this the test fails...

@@ -76,24 +76,24 @@ describe('Ipfs manager', () => {
it('allows to pin one file ipfs', async () => {
await ipfsManager.add(content);
const pinnedHash = await ipfsManager.pin([hash]);
expect(hash).toBe(pinnedHash[0]);
expect(pinnedHash[0]).toBe(hash);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

wrong order for expect

});

it('timeout errors should not generate any retry', async () => {
it('timeout errors should generate retry', async () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMPORTANT. We now want to retry on timeout

@benjlevesque benjlevesque marked this pull request as ready for review June 27, 2022 14:09
@@ -3,9 +3,9 @@ import { BigNumber } from 'ethers';

// This contains default values used to use Ethereum Network and IPFS
// if information are not provided by the user
const config: any = {
const config = {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unrelated, annying any


'/dns4/ipfs-survival.request.network/tcp/4001/ipfs/Qmb6a5DH45k8JwLdLVZUhRhv1rnANpsbXjtsH41esGhNCh',
],
expectedBootstrapNodes: {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the format of the address changed, this accounts for the change while supporting older format

@benjlevesque benjlevesque requested a review from yomarion June 27, 2022 14:11
@coveralls
Copy link

coveralls commented Jun 28, 2022

Coverage Status

Coverage increased (+0.2%) to 88.303% when pulling a6a5acf on ipfs-rpc-post into 8fdf34d on master.

benjlevesque and others added 3 commits July 4, 2022 17:18
Co-authored-by: Alexandre ABRIOUX <alexandre-abrioux@users.noreply.github.com>
@benjlevesque benjlevesque merged commit 9b99d72 into master Jul 5, 2022
@benjlevesque benjlevesque deleted the ipfs-rpc-post branch July 5, 2022 06:18
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.

5 participants