-
Notifications
You must be signed in to change notification settings - Fork 9
Conversation
792cf62
to
9982cdd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should @bajtos also be reviewing these for integration with station?
.circleci/config.yml
Outdated
|
||
# Define a job to be invoked later in a workflow. | ||
# See: https://circleci.com/docs/2.0/configuration-reference/#jobs | ||
orbs: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you hook this up to the standard PL git management setup rather than hand-maintaining this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willscott What is this "standard PL git management setup" that thou refer to ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willscott Don't wanna bother @bajitos with reviewing Golang code and that too one that uses so much of the PL Stack. I'll have a separate discussion/doc/long running interaction with him for integrating with Station. |
him or the L1 devs will know what sort of responses they want to be getting from the l2 interaction so may be better reviewers than me |
@aarshkshah1992 I am happy to learn more about Go :) I quickly skimmed through your code and have one question - how is this new code and the new HTTP server integrating with the existing |
My understanding is that the golang code here will be a libp2p node, and will make connections outbound to L1's. |
) | ||
|
||
var ( | ||
defaultURL = "https://ipfs.io/api/v0/dag/export" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: go-ipfs 0.13+ allows you to replace legacy RPC API at /api/v0/dag/export
with implementation-agnostic Gateway request GET https://ipfs.io/ipfs/{cid}?format=car
(or GET https://ipfs.io/ipfs/{cid}
with Accept: application/vnd.ipld.car
).
This is already supported by ipfs.io:
curl -H "Accept: application/vnd.ipld.car" "https://ipfs.io/ipfs/bafybeidd2gyhagleh47qeg77xqndy2qy3yzn4vkxmk775bg2t5lpuy7pcu" > webui.car
Switching from RPC to Gateway enables you to swap gateway endpoint and its implementation in the future + benefit from planned selector support.
carserver/server_test.go
Outdated
gwAPI := carstore.NewGatewayAPI(svc.URL) | ||
lg := logs.NewSaturnLogger() | ||
cfg := carstore.Config{} | ||
cs, err := carstore.New(temp, gwAPI, cfg, lg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When running inside Filecoin Station, we need Saturn L2 node to store all cache files in the directory provided by the Station. (Typically, this directory is OS-specific. See #36 and ipfs/ipfs-desktop#1656 for more details.)
I am proposing to introduce a new env var, e.g. CACHE_DIR
or SATURN_CACHEDIR
:
- Filecoin Station sets
CACHE_DIR
env var, similarly to how it setsFIL_WALLET_ADDRESS
- Saturn L2 Node uses this value as the root dir for all caches
To make it easier to run saturn-l2 on its own, we can introduce a sensible default when CACHE_DIR
is not provided. I don't think t.TempDir()
is a good one - IMHO, we should preserve cached data across node restarts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey I have introduced an ENV VAR called ROOT_DIR. This will be the root directory for the L2 Node where it will persist all it's state and cached data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't added a sensible default yet but we can add it down the line if need be. Should be easy.
@aarshkshah1992 in my limited knowledge of Go, I believe it's a common convention (a best practice?) to put CLI main files into In our case, I think we should move WDYT? As a nice side effect, I guess such a change is out of scope of your PR, so the question is whether you want to do it before or after your PR lands? |
@bajtos -> Yes good suggestion, I have moved all binaries to cmd/. |
No description provided.