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.

Sign up for GitHub

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

feat: Adding packagemeta to config and project manager #802

Merged
merged 21 commits into from
Jun 16, 2022
Merged

feat: Adding packagemeta to config and project manager #802

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
d6ef1df
feat: adding packagemeta to config and project manager
sabotagebeats Jun 13, 2022
06de772
docs: rework meta docstring
sabotagebeats Jun 13, 2022
67ccc3a
refactor: adds error for incorrect configuration of metadata
sabotagebeats Jun 13, 2022
93be732
fix: breakpoint
sabotagebeats Jun 13, 2022
cd76db4
fix: PackageMeta must be called
sabotagebeats Jun 13, 2022
2c9526a
docs: removed unnecessary TODO
sabotagebeats Jun 13, 2022
9e6539c
fix: import configerror from ape.exceptions
sabotagebeats Jun 13, 2022
099d610
refactor: commented out publish_manifest method
sabotagebeats Jun 13, 2022
3e47528
style: linting
sabotagebeats Jun 13, 2022
e892b36
docs: added docstring for packagemeta method
sabotagebeats Jun 14, 2022
ed41979
fix: add meta to plugin config manager
sabotagebeats Jun 14, 2022
0b68bf1
fix: project now has meta
sabotagebeats Jun 14, 2022
218fe1c
style: linting
sabotagebeats Jun 14, 2022
6a06e23
docs: type ignore and docstring for meta
sabotagebeats Jun 14, 2022
d8537b2
style: linting
sabotagebeats Jun 14, 2022
230b1b5
test: added test for meta
sabotagebeats Jun 14, 2022
863083d
test: test meta with temp_config fixture
sabotagebeats Jun 14, 2022
f97e1d2
test: filled out extra meta fields in test temp_config
sabotagebeats Jun 14, 2022
c72aba2
Merge branch 'main' of https://github.com/apeworx/ape into feat/meta
sabotagebeats Jun 15, 2022
552ce43
fix: removed exception for meta
sabotagebeats Jun 15, 2022
b6b9f74
chore: Merge remote-tracking branch 'upstream/main' into feat/meta
sabotagebeats Jun 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/ape/managers/config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
if TYPE_CHECKING:
from .project import ProjectManager

from ethpm_types import PackageMeta

CONFIG_FILE_NAME = "ape-config.yaml"
DEFAULT_TRANSACTION_ACCEPTANCE_TIMEOUT = 120
Expand Down Expand Up @@ -86,6 +87,9 @@ class ConfigManager(BaseInterfaceModel):
version: str = ""
"""The project's version."""

meta: PackageMeta = PackageMeta
sabotagebeats marked this conversation as resolved.
Show resolved Hide resolved
"""The project's meta package"""
sabotagebeats marked this conversation as resolved.
Show resolved Hide resolved

contracts_folder: Path = None # type: ignore
"""
The path to the project's ``contracts/`` directory
Expand Down
22 changes: 12 additions & 10 deletions src/ape/managers/project/manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from pathlib import Path
from typing import Dict, List, Optional, Type, Union

from ethpm_types import Compiler, ContractType, PackageManifest
from ethpm_types import Compiler, ContractType, PackageManifest, PackageMeta

from ape.api import DependencyAPI, ProjectAPI
from ape.contracts import ContractContainer, ContractNamespace
Expand Down Expand Up @@ -447,16 +447,18 @@ def _get_contract(self, name: str) -> Optional[ContractContainer]:

return None

# @property
# def meta(self) -> PackageMeta:
# return PackageMeta(**self.config_manager.get_config("ethpm").serialize())
@property
def meta(self) -> PackageMeta:
Copy link
Member

@antazoey antazoey Jun 14, 2022 •
edited by sabotagebeats
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • add doc str to this property

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

docstring added: e892b36 please review for completeness

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added test, probably should be reworked: 230b1b5

return PackageMeta(**self.config_manager.get_config("ethpm").serialize())
sabotagebeats marked this conversation as resolved.
Show resolved Hide resolved
sabotagebeats marked this conversation as resolved.
Show resolved Hide resolved

def publish_manifest(self):
sabotagebeats marked this conversation as resolved.
Show resolved Hide resolved
manifest = self.manifest.dict()
if not manifest["name"]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If name was not in the manifest, I believe self.manifest would already raise a ValidationError, no?
We should leave this method commented out, but at the same time, we could probably remove the if not checks here since they happen at the model-level now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, name and version field are optional in spec, but recommended/required for publishing
https://github.com/ApeWorX/ethpm-types/blob/770ba756947215a58a41391c71cbe717708e8ab8/ethpm_types/manifest.py#L58-L60

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this resolved since this code has now been commented out for the time being?

raise ProjectError("Need name to release manifest")
if not manifest["version"]:
raise ProjectError("Need version to release manifest")
breakpoint()
sabotagebeats marked this conversation as resolved.
Show resolved Hide resolved

# def publish_manifest(self):
# manifest = self.manifest.dict()
# if not manifest["name"]:
# raise ProjectError("Need name to release manifest")
# if not manifest["version"]:
# raise ProjectError("Need version to release manifest")
# TODO: Clean up manifest and minify it
sabotagebeats marked this conversation as resolved.
Show resolved Hide resolved
# TODO: Publish sources to IPFS and replace with CIDs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this would require looping through all the sources and publishing them to IPFS as well, then updating all of the links in the package from local links to global ones.

Also side-note is that eventually DependencyAPI should store everything as IPFS content locally to make it super easy to publish

# TODO: Publish to IPFS
Comment on lines 466 to 467
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs #208 to complete these TODOs

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required for this PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we are building towards that though with the Deployments story

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#365 also this related issue for IPFS