[ethpm] Allow registry uri to namespace package assets #1576
Conversation
njgheorghita
changed the title
njgheorghita
force-pushed
the
ethpm-registry-uri-update
branch
2 times, most recently
from
6e98b8f
to
5ad7647
Compare
| RegistryURI = namedtuple("RegistryURI", ["address", "chain_id", "name", "version", "ens"]) | ||
| RegistryURI = namedtuple( | ||
| "RegistryURI", | ||
| ["address", "chain_id", "name", "version", "namespaced_asset", "ens"] |
There was a problem hiding this comment.
FYI, if you aren't aware, what you are calling namespace_asset is a JSON Pointer
There was a problem hiding this comment.
Oh, good to know! I'm not sure if it's strictly a JSON pointer? Essentially, when trying to specify a certain deployment - I'm trying to avoid requiring the entire blockchain URI. Do you think that's a bad idea?
Basically...
ethpm://maker.ethpm.eth:1/[email protected]/deployments/DeploymentName
Is what you would need to fetch the DeploymentName deployment on the mainnet . . . . It seems like there might be some edge cases where this isn't ideal - in terms of forked chains that share a chain ID? Since this is for the widget - I was thinking of just specifying that a registry URI with a pointer/namespaced asset only supports the 5 main chains.
ethpm/backends/registry.py
Outdated
| def _parse_pkg_path(pkg_path: str) -> Tuple[str, Optional[str]]: | ||
| if "/" in pkg_path: | ||
| pkg_id = pkg_path.split("/")[0] | ||
| namespaced_asset = "/".join(pkg_path.split("/")[1:]) |
There was a problem hiding this comment.
These two lines can be replaced with:
pkg_id, _, namespaced_asset = pkg_path.partition('/')
ethpm/backends/registry.py
Outdated
|
|
||
| def _parse_pkg_id(pkg_id: str) -> Tuple[str, Optional[str]]: | ||
| if "@" not in pkg_id: | ||
| return pkg_id, None | ||
| pkg_name, safe_pkg_version = pkg_id.split("@") |
There was a problem hiding this comment.
This call still jumps out to me as a minor problem. It relies on validation that happens outside of this function. If you change this to pkg_name, _, safe_pkg_version = pkg_id.partition('@') then this code would be slightly more robust. Totally a nitpick on my part.
njgheorghita
force-pushed
the
ethpm-registry-uri-update
branch
from
5ad7647
to
6f86dba
Compare
What was wrong?
Updated the ethpm uri parsing functions to support namespaced assets.
ethpm://compound.ethpm.eth:1/[email protected]/deployments/cDaiThis will be useful in the ethpm widget - so that all the data needed to fetch the contract address of a deployment is in a single url.
Todo:
Cute Animal Picture