Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ethpm] Allow registry uri to namespace package assets #1576

Merged

Conversation

@njgheorghita
Copy link
Contributor

njgheorghita commented Feb 7, 2020

What was wrong?

Updated the ethpm uri parsing functions to support namespaced assets.
ethpm://compound.ethpm.eth:1/compound@1.0.0/deployments/cDai

This 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

image

@njgheorghita njgheorghita changed the title Allow registry uri to namespace package assets [ethpm] Allow registry uri to namespace package assets Feb 7, 2020
@njgheorghita njgheorghita force-pushed the njgheorghita:ethpm-registry-uri-update branch 2 times, most recently from 6e98b8f to 5ad7647 Feb 7, 2020
@njgheorghita njgheorghita requested a review from pipermerriam Feb 7, 2020
RegistryURI = namedtuple("RegistryURI", ["address", "chain_id", "name", "version", "ens"])
RegistryURI = namedtuple(
"RegistryURI",
["address", "chain_id", "name", "version", "namespaced_asset", "ens"]

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Feb 7, 2020

Member

FYI, if you aren't aware, what you are calling namespace_asset is a JSON Pointer

This comment has been minimized.

Copy link
@njgheorghita

njgheorghita Feb 8, 2020

Author Contributor

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/dai@1.0.0/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.

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:])

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Feb 7, 2020

Member

These two lines can be replaced with:

pkg_id, _, namespaced_asset = pkg_path.partition('/')

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("@")

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Feb 7, 2020

Member

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 njgheorghita force-pushed the njgheorghita:ethpm-registry-uri-update branch from 5ad7647 to 6f86dba Feb 8, 2020
@njgheorghita njgheorghita merged commit 1a6804a into ethereum:master Feb 10, 2020
51 checks passed
51 checks passed
ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: lint Your tests passed on CircleCI!
Details
ci/circleci: py36-core Your tests passed on CircleCI!
Details
ci/circleci: py36-ens Your tests passed on CircleCI!
Details
ci/circleci: py36-ethpm Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-ethtester-pyevm Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-http-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-http-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-http-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ipc-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ipc-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ipc-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ws-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ws-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-goethereum-ws-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-http Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-ipc Your tests passed on CircleCI!
Details
ci/circleci: py36-integration-parity-ws Your tests passed on CircleCI!
Details
ci/circleci: py37-core Your tests passed on CircleCI!
Details
ci/circleci: py37-ens Your tests passed on CircleCI!
Details
ci/circleci: py37-ethpm Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-ethtester-pyevm Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-http-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-http-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-http-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ipc-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ipc-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ipc-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ws-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ws-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-goethereum-ws-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-http Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-ipc Your tests passed on CircleCI!
Details
ci/circleci: py37-integration-parity-ws Your tests passed on CircleCI!
Details
ci/circleci: py38-core Your tests passed on CircleCI!
Details
ci/circleci: py38-ens Your tests passed on CircleCI!
Details
ci/circleci: py38-ethpm Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-ethtester-pyevm Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-http-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-http-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-http-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ipc-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ipc-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ipc-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ws-1.7.2 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ws-1.8.22 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-goethereum-ws-1.9.7 Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-parity-http Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-parity-ipc Your tests passed on CircleCI!
Details
ci/circleci: py38-integration-parity-ws Your tests passed on CircleCI!
Details
continuous-documentation/read-the-docs Read the Docs build succeeded!
Details
@njgheorghita njgheorghita deleted the njgheorghita:ethpm-registry-uri-update branch Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.