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
AEA-1610 fix: add check consistency of package versions (#2459) #2550
Conversation
@@ -202,6 +202,9 @@ def __str__(self) -> str: | |||
return str(self.value) | |||
|
|||
|
|||
PackageIdPrefix = Tuple[ComponentType, str, str] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
helper type, more readable, and does not affect much code (it replaces Tuple[ComponentType, str, str]
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah is backward compatible too I guess.
self.conflicting_packages = conflicting_packages | ||
super().__init__(self._build_error_message()) | ||
|
||
def _build_error_message(self) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for an example of the generated message, see below:
https://github.com/fetchai/agents-aea/pull/2550/files#diff-5a86c9151c53eb9b7fde346e231dc88a7b18f51ff369081b9116ef1c02423bddR604
self.manager.start_manager() | ||
weather_station_id = PublicId.from_str("fetchai/weather_station:0.27.0") | ||
self.manager.add_project(weather_station_id) | ||
weather_client_id = PublicId.from_str("fetchai/weather_client:0.27.0") | ||
with pytest.raises( | ||
ProjectPackageConsistencyCheckError, | ||
match=re.escape(self.EXPECTED_ERROR_MESSAGE), | ||
): | ||
self.manager.add_project(weather_client_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the code to reproduce issue #2459
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
self._prefix_to_components = ( | ||
{} | ||
) # type: Dict[Tuple[ComponentType, str, str], Set[ComponentId]] | ||
self._prefix_to_components = {} # type: Dict[PackageIdPrefix, Set[ComponentId]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this is just refactoring of typing, i.e. no material change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes
@@ -202,6 +202,9 @@ def __str__(self) -> str: | |||
return str(self.value) | |||
|
|||
|
|||
PackageIdPrefix = Tuple[ComponentType, str, str] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah is backward compatible too I guess.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fill coverage gap in manager.py (due to #2550)
Proposed changes
Add consistency of package versions in MAM.
MAM should track all versions of packages and raise an exception when incompatible versions of any packages are added. Otherwise, this can lead to problems at runtime (with whatever version being loaded last being in memory).
This PR addresses the above issue.
Fixes
Fix #2459
Types of changes
What types of changes does your code introduce to agents-aea?
Put an
x
in the boxes that applyChecklist
Put an
x
in the boxes that apply.develop
branch (left side). Also you should start your branch off ourdevelop
.Further comments
n/a