AEA-1189 Crypto plug-in mechanism #2112

marcofavorito · 2021-01-05T14:09:52Z

Proposed changes

This PR introduces the plug-in mechanism proposed in #1229 for Cryptos.

Fixes

Addresses #1229

Types of changes

What types of changes does your code introduce to agents-aea?
Put an x in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

Put an x in the boxes that apply.

I have read the CONTRIBUTING doc
I am making a pull request against the develop branch (left side). Also you should start your branch off our develop.
Lint and unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have checked that code coverage does not decrease.
I have checked that the documentation about the aea cli tool works
I have added necessary documentation (if appropriate)
Any dependent changes have been merged and published in downstream modules

Further comments

n/a

aea/__init__.py

examples/crypto_ex/README.md

aea/crypto/plugin.py

codecov-io · 2021-01-05T18:50:50Z

Codecov Report

Merging #2112 (3d5e391) into develop (97b9a15) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           develop     #2112    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          301       299     -2     
  Lines        24240     23578   -662     
==========================================
- Hits         24240     23578   -662

Flag	Coverage Δ
unittests	`100.00% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
aea/aea_builder.py	`100.00% <ø> (ø)`
aea/cli/utils/package_utils.py	`100.00% <100.00%> (ø)`
aea/configurations/constants.py	`100.00% <100.00%> (ø)`
aea/crypto/__init__.py	`100.00% <100.00%> (ø)`
aea/crypto/ledger_apis.py	`100.00% <100.00%> (ø)`
aea/crypto/plugin.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 97b9a15...6fa801e. Read the comment docs.

.github/workflows/workflow.yml

removed as it turned out to be useless

plugins/fetchai-crypto/setup.py

Error: "There are no .py[i] files in directory 'plugins'"

DavidMinarsch

The tests should be collocated to the plugins

aea/crypto/ledger_apis.py

benchmark/framework/aea_test_wrapper.py

docs/build-aea-programmatically.md

examples/crypto_ex/README.md

examples/crypto_ex/my_crypto/__init__.py

plugins/fetchai-crypto/setup.py

scripts/whitelist.py

setup.cfg

setup.py

.github/workflows/workflow.yml

marcofavorito · 2021-02-22T14:17:40Z

tox.ini

+[plugins]
+commands =
+    python -m pip install --no-deps file://{toxinidir}/plugins/aea-crypto-ethereum
+    python -m pip install --no-deps file://{toxinidir}/plugins/aea-crypto-cosmos
+    python -m pip install --no-deps file://{toxinidir}/plugins/aea-crypto-fetchai
+    pytest -rfE plugins/aea-crypto-fetchai/tests --cov-report=html --cov-report=xml --cov-report=term --cov-report=term-missing --cov=aea_crypto_fetchai --cov-config=.coveragerc --suppress-no-test-exit-code {posargs}
+    pytest -rfE plugins/aea-crypto-ethereum/tests --cov-report=html --cov-report=xml --cov-report=term --cov-report=term-missing --cov=aea_crypto_ethereum --cov-config=.coveragerc --suppress-no-test-exit-code {posargs}
+    pytest -rfE plugins/aea-crypto-cosmos/tests --cov-report=html --cov-report=xml --cov-report=term --cov-report=term-missing --cov=aea_crypto_cosmos --cov-config=.coveragerc --suppress-no-test-exit-code {posargs}
+
+[testenv:plugins-py3.6]
+basepython = python3.6
+commands = {[plugins]commands}
+
+[testenv:plugins-py3.7]
+basepython = python3.7
+commands = {[plugins]commands}
+
+[testenv:plugins-py3.7-cov]
+basepython = python3.7
+usedevelop = True
+commands = {[plugins]commands}
+
+[testenv:plugins-py3.8]
+basepython = python3.8
+commands = {[plugins]commands}
+
+[testenv:plugins-py3.9]
+basepython = python3.9
+commands = {[plugins]commands}
+


I feel this might still be improved, as the "main testenv" above, still good enough.

marcofavorito · 2021-02-22T14:18:53Z

.github/workflows/workflow.yml

@@ -356,6 +356,9 @@ jobs:
      name: Unit tests
      run: |
        tox -e py${{ matrix.python_version }} -- -m 'not integration and not unstable'
+    - name: Plugin tests
+      run: |
+        tox -e plugins-py${{ matrix.python_version }} -- --cov-append -m 'not integration and not unstable'


note --cov-append. An alternative is to generate different coverage file for each plugin and then merge with coverage combine:

https://coverage.readthedocs.io/en/latest/cmd.html#cmd-combine

marcofavorito · 2021-02-22T14:19:01Z

.github/workflows/workflow.yml

-      run: tox -e py3.7-cov -- --ignore=tests/test_docs --ignore=tests/test_examples --ignore=tests/test_packages/test_contracts --ignore=tests/test_packages/test_skills_integration -m 'not unstable'
+      run:
+        tox -e py3.7-cov -- --ignore=tests/test_docs --ignore=tests/test_examples --ignore=tests/test_packages/test_contracts --ignore=tests/test_packages/test_skills_integration -m 'not unstable'
+        tox -e py3.7-cov -- --cov-append -m 'not unstable'


see --cov-append

packages/fetchai/contracts/erc1155/contract.py

DavidMinarsch · 2021-02-22T15:21:37Z

plugins/aea-crypto-cosmos/LICENSE

@@ -0,0 +1,201 @@
+                                 Apache License
+                           Version 2.0, January 2004


Can we not point to root license from Manifest file?

sorry, no:

pypa/setuptools#2339 (comment)

but we could add a check that make sure the LICENSE is the same

Ok, let's worry about these checks separately (new PR)...there are a few more

done in 8af620f

DavidMinarsch · 2021-02-22T15:25:30Z

plugins/aea-crypto-ethereum/tests/test_ethereum.py

+# -*- coding: utf-8 -*-
+# ------------------------------------------------------------------------------
+#
+#   Copyright 2018-2019 Fetch.AI Limited


Why does this not show relative changes?

because we have another file in tests/test_crypto/test_ethereum.py.

What's in the other one? Why is not everything here?

a test on an ethereum contract

DavidMinarsch · 2021-02-22T15:32:14Z

tox.ini

+    ; because we use --no-deps to install the plugins.
+    ; aea_crypto_cosmos/aea_crypto_fetchai


Why the no deps install?

because we want to test the plugins with the aea on develop (installed by tox during env setup), not from PyPI.

marcofavorito · 2021-02-22T15:53:50Z

plugins/aea-crypto-ethereum/tests/test_ethereum.py

@@ -25,22 +25,16 @@
 from unittest.mock import MagicMock, patch


@DavidMinarsch now diffs work :-)

solarw

Why do we have mentions of the crypto plugins as constants in aea?
Do we need to load all the plugins on aea module load?

solarw · 2021-02-22T15:41:59Z

.github/workflows/workflow.yml

@@ -100,6 +105,7 @@ jobs:
          sudo apt-get autoremove
          sudo apt-get autoclean
          pip install tox
+          pip install --user --upgrade setuptools


pip and setuptools always show a warning that can be updated.

DavidMinarsch

LGTM

in workflow, coverage_checks: - missing '|' - plugin-py3.7-cov test environment should have been called, instead of py3.7-cov.

- add instructions regarding the 'dependencies' field in 'aea-config.yaml'; - add instructions regarding crypto plug-ins.

Update upgrading.md (after #2112 and #2289)

AEA-1189 Fixes to Crypto plugin PR #2112

add first draft of crypto plugin feature

cd21f8a

marcofavorito commented Jan 5, 2021

View reviewed changes

aea/__init__.py Outdated Show resolved Hide resolved

marcofavorito commented Jan 5, 2021

View reviewed changes

examples/crypto_ex/README.md Outdated Show resolved Hide resolved

marcofavorito commented Jan 5, 2021

View reviewed changes

aea/crypto/plugin.py Outdated Show resolved Hide resolved

marcofavorito added 3 commits January 5, 2021 16:37

fix linting

d14a630

handle also ledger_apis and faucet_apis plugins

411e440

add 'fetchai_crypto' as dependency of 'aea'

c0c5dbc

marcofavorito requested a review from DavidMinarsch January 5, 2021 18:26

marcofavorito force-pushed the feature/crypto-plugin branch from 5a521b9 to 4fbe9e3 Compare January 5, 2021 19:10

try to fix liccheck and pylint errors

2e51f6a

marcofavorito force-pushed the feature/crypto-plugin branch from 4fbe9e3 to 2e51f6a Compare January 5, 2021 19:15

marcofavorito added 2 commits January 5, 2021 22:59

update tests with fetchai_crypto instead of aea.crypto.fetchai

e929989

add 'ethereum_crypto' as dependency

0adf752

marcofavorito force-pushed the feature/crypto-plugin branch from 39bcbb9 to cc23765 Compare January 5, 2021 22:41

add 'cosmos_crypto' as dependency

1ca8c0c

marcofavorito force-pushed the feature/crypto-plugin branch from cc23765 to 1ca8c0c Compare January 5, 2021 22:47

marcofavorito commented Jan 5, 2021

View reviewed changes

.github/workflows/workflow.yml Outdated Show resolved Hide resolved

marcofavorito added 4 commits January 5, 2021 23:51

update hashes

6411fef

remove 'fetchai_crypto' from strategy.ini

8f08d74

removed as it turned out to be useless

add plugins to lint/static checks

ffcdcd8

remove cosmos_crypto from fetchai_crypto dependencies

f104e26

marcofavorito commented Jan 5, 2021

View reviewed changes

plugins/fetchai-crypto/setup.py Outdated Show resolved Hide resolved

remove 'plugins' folder from static checks

da59178

Error: "There are no .py[i] files in directory 'plugins'"

DavidMinarsch reviewed Jan 6, 2021

View reviewed changes

DavidMinarsch changed the title ~~Crypto plug-in mechanism~~ AEA-1189 Crypto plug-in mechanism Jan 16, 2021

DavidMinarsch added the v0.10.0 label Jan 16, 2021

marcofavorito added 2 commits January 18, 2021 10:40

Merge branch 'develop' into feature/crypto-plugin

da839f3

remove temporary crypto plugin example

9698fad

marcofavorito mentioned this pull request Jan 19, 2021

Plug-in mechanism for new CLI commands #2170

Closed

replace '<ledger-id>_crypto' with 'aea_crypto_<ledger-id'

667c981

marcofavorito force-pushed the feature/crypto-plugin branch from 7a47feb to 667c981 Compare February 22, 2021 11:13

marcofavorito added 2 commits February 22, 2021 12:26

fix mocking of 'requests' for fetchai tests

2b16627

append coverage of crypto tests

622b61a

marcofavorito commented Feb 22, 2021

View reviewed changes

marcofavorito marked this pull request as ready for review February 22, 2021 14:36

DavidMinarsch reviewed Feb 22, 2021

View reviewed changes

packages/fetchai/contracts/erc1155/contract.py Outdated Show resolved Hide resolved

remove obsolete methods in erc1155/contract.py

3927c11

DavidMinarsch reviewed Feb 22, 2021

View reviewed changes

marcofavorito added 2 commits February 22, 2021 16:49

rename test ethereum modules

618e41c

move original test ethereum module

9f75044

DavidMinarsch removed the v0.10.0 label Feb 22, 2021

add diff checks between main license and plugins' licenses

8af620f

marcofavorito commented Feb 22, 2021

View reviewed changes

solarw reviewed Feb 22, 2021

View reviewed changes

DavidMinarsch approved these changes Feb 22, 2021

View reviewed changes

marcofavorito added 2 commits February 22, 2021 18:49

fix coverage check

fea4480

in workflow, coverage_checks: - missing '|' - plugin-py3.7-cov test environment should have been called, instead of py3.7-cov.

trigger CI

6fa801e

DavidMinarsch merged commit 29b8dee into develop Feb 22, 2021

marcofavorito added a commit that referenced this pull request Feb 24, 2021

update upgrading.md (after #2112 and #2289)

1a984f3

- add instructions regarding the 'dependencies' field in 'aea-config.yaml'; - add instructions regarding crypto plug-ins.

marcofavorito mentioned this pull request Feb 24, 2021

AEA-1189 Update upgrading.md (after #2112 and #2289) #2303

Merged

11 tasks

DavidMinarsch added a commit that referenced this pull request Feb 24, 2021

Merge pull request #2303 from fetchai/fix/upgrading.md

45d98f1

Update upgrading.md (after #2112 and #2289)

DavidMinarsch added a commit that referenced this pull request Feb 24, 2021

Merge pull request #2291 from fetchai/fix/crypto-plugin

d649280

AEA-1189 Fixes to Crypto plugin PR #2112

DavidMinarsch deleted the feature/crypto-plugin branch February 25, 2021 17:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AEA-1189 Crypto plug-in mechanism #2112

AEA-1189 Crypto plug-in mechanism #2112

marcofavorito commented Jan 5, 2021

codecov-io commented Jan 5, 2021 •

edited

Loading

DavidMinarsch left a comment

marcofavorito Feb 22, 2021

marcofavorito Feb 22, 2021

marcofavorito Feb 22, 2021

DavidMinarsch Feb 22, 2021

marcofavorito Feb 22, 2021

marcofavorito Feb 22, 2021

marcofavorito Feb 22, 2021

DavidMinarsch Feb 22, 2021

marcofavorito Feb 22, 2021

DavidMinarsch Feb 22, 2021 •

edited

Loading

marcofavorito Feb 22, 2021

DavidMinarsch Feb 22, 2021

marcofavorito Feb 22, 2021

DavidMinarsch Feb 22, 2021

marcofavorito Feb 22, 2021

DavidMinarsch Feb 22, 2021

marcofavorito Feb 22, 2021 •

edited

Loading

solarw left a comment

solarw Feb 22, 2021

DavidMinarsch left a comment

		; because we use --no-deps to install the plugins.
		; aea_crypto_cosmos/aea_crypto_fetchai

		@@ -25,22 +25,16 @@
		from unittest.mock import MagicMock, patch

AEA-1189 Crypto plug-in mechanism #2112

AEA-1189 Crypto plug-in mechanism #2112

Conversation

marcofavorito commented Jan 5, 2021

Proposed changes

Fixes

Types of changes

Checklist

Further comments

codecov-io commented Jan 5, 2021 • edited Loading

Codecov Report

DavidMinarsch left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidMinarsch Feb 22, 2021 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcofavorito Feb 22, 2021 • edited Loading

Choose a reason for hiding this comment

solarw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavidMinarsch left a comment

Choose a reason for hiding this comment

codecov-io commented Jan 5, 2021 •

edited

Loading

DavidMinarsch Feb 22, 2021 •

edited

Loading

marcofavorito Feb 22, 2021 •

edited

Loading