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

chore: bump tomte #543

Merged
merged 13 commits into from
Jan 17, 2023
Merged

chore: bump tomte #543

merged 13 commits into from
Jan 17, 2023

Conversation

DavidMinarsch
Copy link

Proposed changes

Bumps tomte to latest version.

Fixes

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 and CI passes too
  • 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 added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Further comments

Comment on lines +39 to +40
"pytest>=7.0.0,<7.3.0",
"coverage>=6.4.4,<8.0.0",
Copy link
Author

Choose a reason for hiding this comment

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

To match latest tomte ranges

@@ -372,7 +372,7 @@ def remove(self) -> None:
@property
def agent_items(self) -> Set[PublicId]:
"""Return items registered with agent of the same type as item."""
return getattr(self.agent_config, self.item_type_plural, set)
return getattr(self.agent_config, self.item_type_plural, set())
Copy link
Author

Choose a reason for hiding this comment

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

found a bug

for agent in agents:
wait_for_condition(lambda: agent.is_running, timeout=5)
for agent_ in agents:
wait_for_condition(lambda a=agent_: a.is_running, timeout=5)
Copy link
Author

Choose a reason for hiding this comment

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

this was a bug!

Copy link
Collaborator

Choose a reason for hiding this comment

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

actually not a bug, it's used inplace. so closure is not needed.
but automatic check can suppose closure is needed.

for agent in agents:
wait_for_condition(lambda: agent.is_running, timeout=5)
for agent_ in agents:
wait_for_condition(lambda a=agent_: a.is_running, timeout=5)
Copy link
Author

Choose a reason for hiding this comment

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

this too!

Copy link
Collaborator

Choose a reason for hiding this comment

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

for agent in agents:
wait_for_condition(lambda: agent.is_running, timeout=5)
for agent_ in agents:
wait_for_condition(lambda a=agent_: a.is_running, timeout=5)
Copy link
Author

Choose a reason for hiding this comment

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

!

Copy link
Collaborator

Choose a reason for hiding this comment

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

for agent in agents:
wait_for_condition(lambda: agent.is_running, timeout=5)
for agent_ in agents:
wait_for_condition(lambda a=agent_: a.is_running, timeout=5)
Copy link
Author

Choose a reason for hiding this comment

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

!

Copy link
Collaborator

Choose a reason for hiding this comment

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

@@ -13,7 +13,7 @@ exclude=.md,
scripts/oef/launch.py
max-line-length = 88
select = B,C,D,E,F,I,W,
ignore = E203,E501,W503,D202,B014,D400,D401,DAR
ignore = E203,E501,W503,D202,B014,D400,D401,DAR,B028,B017
Copy link
Author

Choose a reason for hiding this comment

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

Need to enable at some point

@@ -173,7 +173,7 @@ def create_async_task(self, loop: AbstractEventLoop) -> TaskAwaitable:
raise ValueError(
"Agent runtime is not async compatible. Please use runtime_mode=async"
)
return loop.create_task(self._agent.runtime.start_and_wait_completed())
return loop.create_task(self._agent.runtime.start_and_wait_completed()) # type: ignore
Copy link
Author

Choose a reason for hiding this comment

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

@solarw for you to investigate. I just added the ignore temporarily. It is masking an issue!

@@ -79,7 +79,7 @@ def create_async_task(self, loop: AbstractEventLoop) -> TaskAwaitable:
raise ValueError(
"Agent runtime is not async compatible. Please use runtime_mode=async"
)
return loop.create_task(self._agent.runtime.start_and_wait_completed())
return loop.create_task(self._agent.runtime.start_and_wait_completed()) # type: ignore
Copy link
Author

Choose a reason for hiding this comment

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

@solarw for you to investigate. I just added the ignore temporarily. It is masking an issue!

Copy link
Collaborator

Choose a reason for hiding this comment

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

resolved



def check_ipfs_hash_pushed(ipfs_hash: str) -> Tuple[str, bool]:
def check_ipfs_hash_pushed(ipfs_hash: str, retries: int = 5) -> Tuple[str, bool]:
Copy link
Author

Choose a reason for hiding this comment

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

@8ball030 this should make you happy ;)

@@ -126,7 +127,7 @@ def make_ledger_api_connection(
return connection


@pytest.fixture()
Copy link
Author

Choose a reason for hiding this comment

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

breaking change in this library

@@ -58,7 +58,7 @@ jobs:
sudo apt-get update --fix-missing
sudo apt-get autoremove
sudo apt-get autoclean
pip install tomte[tox]==0.1.5
pip install tomte[tox]==0.2.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

make ci-requirements.txt and use pip install -r to make version edit easier?

Copy link
Author

Choose a reason for hiding this comment

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

Good idea. Will open a Trello ticket

Copy link
Collaborator

@solarw solarw left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -20,7 +20,7 @@
"""Implementation of the 'aea fetch' subcommand."""
import os
import shutil
from distutils.dir_util import copy_tree
from distutils.dir_util import copy_tree # pylint: disable=deprecated-module

Choose a reason for hiding this comment

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

Can't we use shutil.copytree here?

Copy link
Author

Choose a reason for hiding this comment

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

Probably

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants