fix: local running dx improvements #12
Conversation
| &cli.StringFlag{ | ||
| Name: "ipfs-api-host", | ||
| Usage: "port to reach a Kubo-compatible RPC API", | ||
| EnvVars: []string{"TIROS_RUN_IPFS_API_HOST"}, | ||
| Value: "127.0.0.1", | ||
| }, |
There was a problem hiding this comment.
we should also update the multiaddr used in ipfsClient = shell.NewShell(fmt.Sprintf("/ip4/127.0.0.1/tcp/%d", c.Int("ipfs-api-port"))) to use this, but I wanted to submit PR with smaller changes and get your thoughts on this @dennis-tra
There was a problem hiding this comment.
Is the ipfs-api-host different from the ipfs-host? This would mean that the IPFS gateway has a different host than the API, is that the case for helia?
Just curious, I'm totally fine adding this additional flag but if we can get away with just using the ipfs-host flag, I would prefer just keeping that. If we indeed need both and ipfs-api-host is unset, I would fall back to whatever is set with ipfs-host.
There was a problem hiding this comment.
The difference is kubo has two endpoints, an api endpoint and http gateway endpoint. Helia-http-gateway ( and others might) has only one.
Youre right that we should probably use the single http host for fallback to version via http. I will update this
| @@ -2,7 +2,7 @@ version: "3.9" | |||
| name: tiros | |||
| services: | |||
| ipfs: | |||
| image: ipfs/kubo:v0.19.0 | |||
| image: ${IPFS_IMAGE:-ipfs/kubo:v0.19.0} | |||
There was a problem hiding this comment.
allows overriding the image easily, so I can rebuild helia-http-gateway locally and have it update here.
| env_file: | ||
| - ${IPFS_ENV_FILE:-/dev/null} |
There was a problem hiding this comment.
dockerfile syntax of optional env files doesn't work well, so this is a fix for ignoring env file if IPFS_ENV_FILE is not set. see docker/compose#3560 (comment) for more info
This allows me to set .env files for testing helia-dr(.env.helia-dr), helia(.env.helia), and helia-tg(.env.helia-tg) and easily change them with the following config:
# .envrc (for direnv)
dotenv .env.local
dotenv .env
# .env.local (already in this repo
...
# .env
TIROS_RUN_IPFS_API_HOST="localhost"
TIROS_RUN_IPFS_API_PORT=8080
IPFS_ENV_FILE=.env.helia-dr
IPFS_IMAGE=helia-http-gateway:local-arm64
and all i have to do is update .env and run direnv reload then docker compose up -d to get updated services.
| &cli.StringFlag{ | ||
| Name: "ipfs-api-host", | ||
| Usage: "port to reach a Kubo-compatible RPC API", | ||
| EnvVars: []string{"TIROS_RUN_IPFS_API_HOST"}, | ||
| Value: "127.0.0.1", | ||
| }, |
There was a problem hiding this comment.
Is the ipfs-api-host different from the ipfs-host? This would mean that the IPFS gateway has a different host than the API, is that the case for helia?
Just curious, I'm totally fine adding this additional flag but if we can get away with just using the ipfs-host flag, I would prefer just keeping that. If we indeed need both and ipfs-api-host is unset, I would fall back to whatever is set with ipfs-host.
| apiHost := c.String("ipfs-api-host") | ||
| apiPort := c.Int("ipfs-api-port") | ||
| apiUrl := fmt.Sprintf("http://%s:%d/api/v0/version", apiHost, apiPort) | ||
| // log that we're falling back to HTTP url for version | ||
| log.WithField("url", apiUrl).Debugln("Falling back to HTTP request for version") | ||
|
|
||
| resp, httpErr := http.Get(apiUrl) | ||
| if httpErr != nil { | ||
| return "", "", fmt.Errorf("ipfs api and http fallback both failed: %v, fallback error: %w", err, httpErr) | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| body, readErr := io.ReadAll(resp.Body) | ||
| if readErr != nil { | ||
| return "", "", fmt.Errorf("error reading version endpoint response: %w", readErr) | ||
| } | ||
| // log the entire body of the response | ||
| log.WithField("body", string(body)).Debugln("HTTP response body") | ||
|
|
||
| var data struct { | ||
| Version string `json:"Version"` | ||
| Commit string `json:"Commit"` | ||
| } | ||
|
|
||
| jsonErr := json.Unmarshal(body, &data) | ||
| if jsonErr != nil { | ||
| return "", "", fmt.Errorf("error parsing version endpoint JSON: %w", jsonErr) | ||
| } | ||
|
|
||
| return data.Version, data.Commit, nil |
There was a problem hiding this comment.
Under the hood t.ipfs.Version() is doing almost the same HTTP request to fmt.Sprintf("http://%s:%d/api/v0/version", apiHost, apiPort). The client is not using gRPC or anything. The only difference here seems to be that the Kubo HTTP endpoints expect a POST request [docs]. Therefore, t.ipfs.Version() is also doing a POST instead of a GET as in line 303.
Would it work if we changed helia to also expect a POST instead of a GET request?
If that is possible AND we really need the ipfs-api-host flag from above, I think we could get away with just changing:
ipfsClient = shell.NewShell(fmt.Sprintf("/ip4/127.0.0.1/tcp/%d", c.Int("ipfs-api-port")))to
ipfsClient = shell.NewShell(fmt.Sprintf("/ip4/%s/tcp/%d", c.String("ipfs-api-host"), c.Int("ipfs-api-port")))There was a problem hiding this comment.
Post requests for version are also supported in helia-http-gateway and this fails against helia-http-gateway when ran locally. Maybe due to docker networking on mac?
I tried using the change you suggest and that didnt work, while explicitly doing an http request did
| env_file: | ||
| - ${IPFS_ENV_FILE:-/dev/null} |
This PR makes it much easier to run a custom IPFS gateway, and override env vars for them.
I'm using this to test helia-http-gateway.