feat: Add dag_to_car functionality by d4v1d03 · Pull Request #1622 · storacha/w3up

d4v1d03 · 2025-02-13T00:35:19Z

Draft PR on the issue #1621

Signed-off-by: d4v1d03 <amit08072005@gmail.com>

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

alanshaw · 2025-03-19T13:33:26Z

+    print("\n⚠️ Some dependencies could not be installed automatically.")
+    print("Please install them manually before using the package.")
+    print("- ipfs-car: npm install -g ipfs-car")
+    print("- w3cli: npm install -g @web3-storage/w3")


Suggested change

print("- w3cli: npm install -g @web3-storage/w3")

print("- w3cli: npm install -g @web3-storage/w3cli")

alanshaw · 2025-03-19T13:34:07Z

@@ -0,0 +1,154 @@
+"""import os


Is this meant to be commented?

my bad, i forgot to remove this file

alanshaw · 2025-03-19T13:35:55Z

+
+    auth_secret = os.getenv("X-AUTH-SECRET-HEADER")
+    authorization = os.getenv("AUTHORIZATION-HEADER")
+    endpoint = os.getenv("HTTPS-ENDPOINT", "https://up.storacha.network/bridge")


Env vars should use underscore (_) not dash (-) to be a valid bash variable.

alanshaw · 2025-03-19T13:36:37Z

+        return response.json()
+    else:
+        print(f"❌ Upload failed with status code {response.status_code}: {response.text}")
+        return None


Why not raise an error here?

Fixed, please have a look

alanshaw · 2025-03-19T13:40:04Z

+        json.dump(payload, temp, indent=2)
+        temp_json_path = temp.name
+
+    return temp_json_path


Why does it store these and not return them?

alanshaw · 2025-03-19T13:40:28Z

+
+def upload_to_storacha(json_file):
+    """Upload to Storacha using the HTTP Bridge."""
+    load_dotenv()


I would pass these dotenv vars to the function as parameters, and load them in main(). This allows the script to be used as a library where the values could come from places oter than environment vars.

Fixed, please have a look

alanshaw · 2025-03-19T13:41:54Z

+
+    cid, car_file = create_dag_and_car(file_path)
+
+    json_file = create_all_instructions_json(cid, space_did)


Why all "instructions"? We only want to do store/add next...

Fixed this, I've put other instructions that the user can use in the README.md

alanshaw · 2025-03-19T13:43:53Z

+        return cid, car_file
+    except subprocess.CalledProcessError as e:
+        print(f"❌ ipfs-car failed: {e.stderr.decode('utf-8')}")
+        sys.exit(1)


I'd avoid printing and sys.exit in methods that could be used as library code. Instead catch the error here, and re-throw it with the constructed error message. It can be caught in main() if you like but I assume a raised error is going to be printed to the console and the exit code set to non-zero anyways.

alanshaw · 2025-03-19T13:47:06Z

+        return None
+
+def create_dag_and_car(file_path):
+    if not os.path.exists(file_path):


ipfs-car will do this - I think this is redundant.

It is? I still see the conditional here.

can you review again pls , ive used ipfs-car directly this time

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

alanshaw · 2025-03-21T09:54:22Z

+
+## Instructions to be used over the http bridge
+
+Below instructions can be used over the http bridge by adidng them to the list.json file.


Suggested change

Below instructions can be used over the http bridge by adidng them to the list.json file.

Below instructions can be used over the http bridge by adding them to the list.json file.

Thanks, fixed the changes.

alanshaw · 2025-03-21T09:55:19Z

+    ]
+    */
+  ]
+}


I think this is a bit confusing, why not expose these as separate methods on the client?

alanshaw · 2025-03-21T09:58:53Z

+        return None
+
+def create_dag_and_car(file_path):
+    if not os.path.exists(file_path):


It is? I still see the conditional here.

alanshaw · 2025-03-21T10:00:15Z

+        raise FileNotFoundError(f"File '{file_path}' not found.")
+
+    car_file = f"{file_path}.car"
+    ipfs_car_cmd = f"npx ipfs-car pack {file_path} --output {car_file}" if platform.system() == "Windows" else f"ipfs-car pack {file_path} --output {car_file}"


Why npx only on windows?

Why not install ipfs-car as part of the setup?

fixed this and added it to package.json

alanshaw · 2025-03-21T10:01:09Z

+        print("❌ w3cli not found.")
+        print("📦 Installing w3cli globally...")
+        try:
+            subprocess.run("npm install -g @web3-storage/w3cli", shell=True, check=True)


Does it need to be global - can you just add it to the package.json here and install and use it locally?

alanshaw · 2025-03-21T10:04:13Z

+        payload = f.read()
+    return payload
+
+def upload_to_storacha(json_payload, auth_secret, authorization, endpoint):


I'd perhaps rename - this is "send request to the bridge".

alanshaw · 2025-03-21T10:06:14Z

+    try:
+     cid, car_file = create_dag_and_car(file_path)
+     json_payload = read_list_json()
+     result = upload_to_storacha(json_payload, auth_secret, authorization, endpoint)


I think we're missing a step here. The response from the bridge will have the URL for where you should HTTP PUT the data. Have a read through https://docs.storacha.network/how-to/http-bridge/#storing-files

My bad, i checked this right now. Fixed it now

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

dag_to_car working

8d88e1d

Signed-off-by: d4v1d03 <amit08072005@gmail.com>

d4v1d03 changed the title ~~dag_to_car working~~ feat: Add dag_to_car functionality Feb 13, 2025

d4v1d03 and others added 7 commits February 16, 2025 01:15

Merge branch 'storacha:main' into main

51ad358

"w3cli used"

b243aee

added the use of http bridge

42f972c

test

9d978d9

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

level-1 done

859b057

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

clean code and readme added

3bb1338

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

added all the available instructions in the http bridge

799582c

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

d4v1d03 marked this pull request as ready for review March 18, 2025 20:07

Merge branch 'main' into main

aa2b148

alanshaw requested changes Mar 19, 2025

View reviewed changes

d4v1d03 added 2 commits March 20, 2025 08:59

fixes after review

2093ead

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

"fixes done after review"

d026401

d4v1d03 requested a review from alanshaw March 21, 2025 01:38

all fixes done

6112b9d

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

alanshaw requested changes Mar 21, 2025

View reviewed changes

d4v1d03 and others added 4 commits March 25, 2025 16:25

fixes

a2cbd59

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

Merge branch 'main' into main

eb84770

changes according to the feedback given

1c87135

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

added to package.json

14ed8b5

Signed-off-by: Amit Pandey <amit08072005@gmail.com>

d4v1d03 requested a review from alanshaw April 3, 2025 13:39

	print("- w3cli: npm install -g @web3-storage/w3")
	print("- w3cli: npm install -g @web3-storage/w3cli")


		cid, car_file = create_dag_and_car(file_path)

		json_file = create_all_instructions_json(cid, space_did)


		## Instructions to be used over the http bridge

		Below instructions can be used over the http bridge by adidng them to the list.json file.

Conversation

d4v1d03 commented Feb 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants