feat: Adding packagemeta to config and project manager #802

sabotagebeats · 2022-06-13T20:24:57Z

What I did

Adding the package meta to the project config and project manager

fixes: #793

How I did it

following instructions laid out in issue #793

How to verify it

When it works, the package meta should be filled out in the contract manifest

Checklist

All changes are completed
New test cases have been added
Documentation has been updated

sabotagebeats · 2022-06-13T20:52:43Z

src/ape/managers/project/manager.py:452: error: "PluginConfig" has no attribute "serialize"

sabotagebeats · 2022-06-13T20:53:52Z

code is not hitting my breakpoint within publish_manifest(self):

src/ape/managers/config.py

src/ape/managers/project/manager.py

antazoey · 2022-06-13T21:23:39Z

src/ape/managers/project/manager.py

+
+    def publish_manifest(self):
+        manifest = self.manifest.dict()
+        if not manifest["name"]:


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.

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

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

src/ape/managers/project/manager.py

src/ape/managers/config.py

src/ape/managers/project/manager.py

src/ape/managers/config.py

src/ape/managers/project/manager.py

fubuloubu · 2022-06-13T21:53:31Z

src/ape/managers/project/manager.py

+
+    def publish_manifest(self):
+        manifest = self.manifest.dict()
+        if not manifest["name"]:


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

src/ape/managers/project/manager.py

fubuloubu · 2022-06-13T21:57:05Z

src/ape/managers/project/manager.py

    #     TODO: Publish sources to IPFS and replace with CIDs
    #     TODO: Publish to IPFS


Needs #208 to complete these TODOs

Is this required for this PR?

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

#365 also this related issue for IPFS

fubuloubu · 2022-06-13T21:58:18Z

src/ape/managers/project/manager.py

-    #     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
    #     TODO: Publish sources to IPFS and replace with CIDs


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

Co-authored-by: Juliya Smith <yingthi@live.com>

antazoey

Update config.md to include information about meta

It might be also worth adding a test in tests/functional/test_project.py like test_manifest_config() and use the temp_config() fixture to temporary adjust the project config and make sure you can read it on the meta() property you are adding as well other publishing related fields you are going to add over time, including name and version

antazoey · 2022-06-14T00:10:09Z

src/ape/managers/project/manager.py

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


add doc str to this property

docstring added: e892b36 please review for completeness

added test, probably should be reworked: 230b1b5

antazoey · 2022-06-14T00:11:17Z

src/ape/managers/project/manager.py

    #     TODO: Publish sources to IPFS and replace with CIDs
    #     TODO: Publish to IPFS


Is this required for this PR?

sabotagebeats · 2022-06-14T16:33:44Z

@unparalleled-js ipfs publishing is not required for this PR, this issue here is for IPFS publishing: #365

sabotagebeats · 2022-06-14T17:06:45Z

I can't seem to get this to actually function properly:

In [1]: manifest = project.Token.project_manager.extract_manifest()

In [2]: manifest.meta

In [3]: type(manifest.meta)
Out[3]: NoneType

In [5]: manifest.meta()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [5], in <module>
----> 1 manifest.meta()

TypeError: 'NoneType' object is not callable

fubuloubu · 2022-06-14T18:55:43Z

I can't seem to get this to actually function properly:

In [1]: manifest = project.Token.project_manager.extract_manifest()

In [2]: manifest.meta

In [3]: type(manifest.meta)
Out[3]: NoneType

In [5]: manifest.meta()
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
Input In [5], in <module>
----> 1 manifest.meta()

TypeError: 'NoneType' object is not callable

PackageManifest.meta is an optional field, so you have to handle when it is None by raising in that scenario

Although, more broadly speaking might need to fill in the meta fields for when dependencies get downloaded via DependencyAPI?

sabotagebeats · 2022-06-14T18:56:20Z

need to add testing and make sure functionality works

sabotagebeats · 2022-06-14T19:01:59Z

src/ape/managers/project/manager.py

+        meta = self.config_manager.get_config("meta")
+        try:
+            return PackageMeta(**meta.serialize())
+        except Exception as e:
+            raise ConfigError(f"Incorrect configuration of package metadata:\n{meta}") from e


this code isn't getting ran when compiling or extracting manifest. I also have the following error in mypy src/ape/managers/project/manager.py:459: error: "PluginConfig" has no attribute "serialize"

it does get ran when I do this command in console: In [1]: project.Token.project_manager.meta

sabotagebeats · 2022-06-14T19:02:46Z

src/ape/managers/config.py

@@ -147,6 +152,7 @@ def _plugin_configs(self) -> Dict[str, PluginConfig]:
        user_config = load_config(config_file) if config_file.exists() else {}
        self.name = configs["name"] = user_config.pop("name", "")
        self.version = configs["version"] = user_config.pop("version", "")
+        self.meta = configs["meta"] = user_config.pop("meta", "")


does this look right?

fubuloubu · 2022-06-14T19:06:55Z

src/ape/managers/project/manager.py

+        meta = self.config_manager.get_config("meta")
+        try:
+            return PackageMeta(**meta.serialize())


Try this:

Suggested change

meta = self.config_manager.get_config("meta")

try:

return PackageMeta(**meta.serialize())

try:

return self.config_manager.meta

I was able to get the meta into the project by using this
0b68bf1

still need to get the meta into the manifest

fubuloubu · 2022-06-14T19:07:41Z

src/ape/managers/config.py

@@ -147,6 +152,7 @@ def _plugin_configs(self) -> Dict[str, PluginConfig]:
        user_config = load_config(config_file) if config_file.exists() else {}
        self.name = configs["name"] = user_config.pop("name", "")
        self.version = configs["version"] = user_config.pop("version", "")
+        self.meta = configs["meta"] = user_config.pop("meta", "")


Along with this

Suggested change

self.meta = configs["meta"] = user_config.pop("meta", "")

self.meta = configs["meta"] = PackageMeta(**user_config.pop("meta", ""))

sabotagebeats · 2022-06-14T19:25:51Z

src/ape/managers/project/manager.py

+        try:
+            return self.config_manager.meta
+        except Exception as e:
+            raise ConfigError(f"Incorrect configuration of package metadata:\n{meta}") from e


flake8 . ./src/ape/managers/project/manager.py:459:80: F821 undefined name 'meta'

@unparalleled-js I'm still having this issue and not sure how to resolve it.

antazoey · 2022-06-14T19:39:28Z

src/ape/managers/project/manager.py

-    # def meta(self) -> PackageMeta:
-    #     return PackageMeta(**self.config_manager.get_config("ethpm").serialize())
+    @property
+    def meta(self) -> PackageMeta:  # type: ignore


probably put the type ignore on the return stmt

I don't think this fixed the type issue.

antazoey · 2022-06-14T19:41:11Z

src/ape/managers/project/manager.py

+    @property
+    def meta(self) -> PackageMeta:  # type: ignore
+        """
+        Populate package manifest with metadata as per EIP


This does not populate the manifest, so I would change this doc.
This just reads the value from the config manager.

Try something like

Metadata about the active project as per EIP ... Use when publishing your package manifest.

antazoey · 2022-06-14T22:01:35Z

tests/functional/test_project.py

@@ -45,6 +46,12 @@ def test_extract_manifest(dependency_config, project_manager):
    assert type(manifest) == PackageManifest


+def test_meta(dependency_config, project_manager):


Instead of using dependency_config, use the temp_config fixture to make your own meta config:

def test_meta(temp_config, project_manager): meta_config = {"meta": {"authors": ["Test Testerson"]}} with temp_config(meta_config): assert project_amager.meta.authors == ["Test Testerson"]

Expand on my example a little, add the other properties and stuff

antazoey

LGTM

feat: adding packagemeta to config and project manager

d6ef1df

sabotagebeats self-assigned this Jun 13, 2022

sabotagebeats requested review from NotPeopling2day and antazoey June 13, 2022 20:54

antazoey requested changes Jun 13, 2022

View reviewed changes

fubuloubu reviewed Jun 13, 2022

View reviewed changes

sabotagebeats and others added 8 commits June 13, 2022 15:08

docs: rework meta docstring

06de772

Co-authored-by: Juliya Smith <yingthi@live.com>

refactor: adds error for incorrect configuration of metadata

67ccc3a

Co-authored-by: Juliya Smith <yingthi@live.com>

fix: breakpoint

93be732

Co-authored-by: Juliya Smith <yingthi@live.com>

fix: PackageMeta must be called

cd76db4

Co-authored-by: Juliya Smith <yingthi@live.com>

docs: removed unnecessary TODO

2c9526a

fix: import configerror from ape.exceptions

9e6539c

refactor: commented out publish_manifest method

099d610

style: linting

3e47528

antazoey reviewed Jun 14, 2022

View reviewed changes

docs: added docstring for packagemeta method

e892b36

fix: add meta to plugin config manager

ed41979

sabotagebeats commented Jun 14, 2022

View reviewed changes

sabotagebeats requested a review from antazoey June 14, 2022 19:03

fubuloubu reviewed Jun 14, 2022

View reviewed changes

fix: project now has meta

0b68bf1

sabotagebeats commented Jun 14, 2022

View reviewed changes

style: linting

218fe1c

antazoey reviewed Jun 14, 2022

View reviewed changes

sabotagebeats added 3 commits June 14, 2022 12:48

docs: type ignore and docstring for meta

6a06e23

style: linting

d8537b2

test: added test for meta

230b1b5

sabotagebeats requested a review from antazoey June 14, 2022 19:58

antazoey reviewed Jun 14, 2022

View reviewed changes

sabotagebeats added 2 commits June 14, 2022 16:20

test: test meta with temp_config fixture

863083d

test: filled out extra meta fields in test temp_config

f97e1d2

sabotagebeats requested a review from antazoey June 14, 2022 23:30

sabotagebeats added 2 commits June 15, 2022 15:43

Merge branch 'main' of https://github.com/apeworx/ape into feat/meta

c72aba2

fix: removed exception for meta

552ce43

antazoey approved these changes Jun 16, 2022

View reviewed changes

chore: Merge remote-tracking branch 'upstream/main' into feat/meta

b6b9f74

sabotagebeats merged commit c74f3fb into ApeWorX:main Jun 16, 2022

sabotagebeats deleted the feat/meta branch June 16, 2022 01:57

		# TODO: Publish sources to IPFS and replace with CIDs
		# TODO: Publish to IPFS

	self.meta = configs["meta"] = user_config.pop("meta", "")
	self.meta = configs["meta"] = PackageMeta(**user_config.pop("meta", ""))

		@@ -45,6 +46,12 @@ def test_extract_manifest(dependency_config, project_manager):
		assert type(manifest) == PackageManifest


		def test_meta(dependency_config, project_manager):

feat: Adding packagemeta to config and project manager #802

feat: Adding packagemeta to config and project manager #802

Conversation

sabotagebeats commented Jun 13, 2022 • edited Loading

What I did

How I did it

How to verify it

Checklist

sabotagebeats commented Jun 13, 2022 • edited Loading

sabotagebeats commented Jun 13, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antazoey left a comment

Choose a reason for hiding this comment

antazoey Jun 14, 2022 • edited by sabotagebeats Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabotagebeats commented Jun 14, 2022

sabotagebeats commented Jun 14, 2022 • edited Loading

fubuloubu commented Jun 14, 2022

sabotagebeats commented Jun 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sabotagebeats Jun 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antazoey Jun 14, 2022 • edited by fubuloubu Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

antazoey left a comment

Choose a reason for hiding this comment

sabotagebeats commented Jun 13, 2022 •

edited

Loading

sabotagebeats commented Jun 13, 2022 •

edited

Loading

antazoey Jun 14, 2022 •

edited by sabotagebeats

Loading

sabotagebeats commented Jun 14, 2022 •

edited

Loading

sabotagebeats Jun 14, 2022 •

edited

Loading

antazoey Jun 14, 2022 •

edited by fubuloubu

Loading