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

Add support for a helia container #5

Merged
merged 1 commit into from
Oct 18, 2023
Merged

Add support for a helia container #5

merged 1 commit into from
Oct 18, 2023

Conversation

dennis-tra
Copy link
Contributor

Name: "kubo-host",
Usage: "port to reach the Kubo Gateway",
EnvVars: []string{"TIROS_RUN_KUBO_HOST"},
Name: "ipfs-host",
Copy link

Choose a reason for hiding this comment

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

Suggested change
Name: "ipfs-host",
Name: "helia-host",

Copy link

Choose a reason for hiding this comment

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

not sure if this is per instance or the global name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is per instance and would point to either Kubo or Helia. That's why I renamed it. However, I can't find any usage of that config flag anyway :D

@SgtPooki
Copy link

@dennis-tra are you waiting on anything from @ipfs/helia-dev to get this merged? Is there anything we can do to help this along?

@@ -2,7 +2,7 @@ version: "3.9"
name: tiros
services:
ipfs:
image: ipfs/kubo:v0.19.0
Copy link

Choose a reason for hiding this comment

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

Does this mean our measurement hasn't been using the latest Kubo? Will it use the latest Kubo and Helia going forward?

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 idea was to indeed run the latest version of kubo and the most common one based on our measurements. In the past, this has been 0.18 and 0.19. While you're correct that we weren't running the very latest version of Kubo, his docker compose file doesn't represent our deployment. The current deployment configuration is here and with the new helia version the config looks like this

Copy link

Choose a reason for hiding this comment

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

Thanks for the info. I can't click through to the link. Can you please give me access?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

@dennis-tra
Copy link
Contributor Author

@SgtPooki the helia image didn't fully work for me yet. That's why I haven't deployed it. I'm in touch with Nishant about it and as soon as the websites and all resources are loaded properly I'll deploy it 👍

@dennis-tra
Copy link
Contributor Author

Removed all Kubo references and made it generic on the IPFS implementation. The new database migrations also allow to track which IPFS implementation was under test as there was the hidden (or not so hidden) assumption that it's Kubo.

@dennis-tra dennis-tra marked this pull request as ready for review October 18, 2023 16:21
@dennis-tra
Copy link
Contributor Author

As discussed with @whizzzkid I'll deploy the current version of the Helia image while he works on the remaining bits. This allows us gain early experience in running Helia alongside Kubo in our measurement infra.

Comment on lines 264 to +271
func (t *tiros) InitRun(c *cli.Context) (*models.Run, error) {
version, sha, err := t.kubo.Version()
version, sha, err := t.ipfs.Version()
if err != nil {
return nil, fmt.Errorf("kubo api offline: %w", err)
return nil, fmt.Errorf("ipfs api offline: %w", err)
}

dbRun, err := t.dbClient.InsertRun(c, fmt.Sprintf("%s-%s", version, sha))
ipfsImpl := c.String("ipfs-implementation")
dbRun, err := t.dbClient.InsertRun(c, ipfsImpl, fmt.Sprintf("%s-%s", version, sha))

Choose a reason for hiding this comment

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

FYI @dennis-tra this is failing for me locally saying ERRO[0000] error: init run: ipfs api offline: version: command not found. Is this trying to call 'ipfs' binary locally to return version?

Copy link
Contributor Author

@dennis-tra dennis-tra Oct 30, 2023

Choose a reason for hiding this comment

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

It's not using the local Kubo binary. It's using the HTTP API exposed by Kubo. In this case, the must be an endpoint that implements this specification: https://docs.ipfs.tech/reference/kubo/rpc/#api-v0-version

I thought this was already done by @whizzzkid?

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.

3 participants