ethpm / py-ethpm

This library is deprecated. ethPM python tooling is now located in web3.py
MIT License
24 stars 13 forks source link

Generalize URI handling (`Package.from_uri`) #79

Closed njgheorghita closed 6 years ago

njgheorghita commented 6 years ago

What was wrong?

A more generalized scheme for handling package instantiation from URI was needed.

How was it fixed?

Got rid of Package.from_ipfs / Package.from_registry in favor of Package.from_uri which will return a Package object instantiated by a manifest found at a valid content-addressed URI.

Cute Animal Picture

image

fixes #37

njgheorghita commented 6 years ago

@pipermerriam ping when you get the chance

pipermerriam commented 6 years ago

Correct me if I'm wrong here: I think the passing around of the w3 instance is specifically for the on-chain registry backend. If so, here's my proposal for how we can kick that can down the road a bit.

What do you think about defaulting the registry backend to use its own web3 instance that's by default connected to infura?

njgheorghita commented 6 years ago

@pipermerriam I'm down to default to an infura web3 for the registry backend. What would that look like in terms of the api key? Just generate a new one strictly for this repo to use and if it gets abused/becomes problematic then come up with a fix?

pipermerriam commented 6 years ago

RE infura and API keys.

Yes, I think that's an acceptable short term solution while this stuff is still experimental.

njgheorghita commented 6 years ago

@pipermerriam Could you give this another :eyes: when you get the chance. Switched up the uri handling somewhat significantly to mirror the populus strategy more closely. can_handle_uri became ... can_resolve_uri if uri points to a manifest (basically any ipfs uri) & can_translate_uri if uri points to another uri (basically any registry uri)

njgheorghita commented 6 years ago

@pipermerriam re-pinging for another quick lookover at your convenience in case ^ got buried

pipermerriam commented 6 years ago

:crying_cat_face: sorry, got lost in a storm of github notifications. Looking now for anything that should get cleaned up post merge