Skip to content

Conversation

@nesitor
Copy link
Member

@nesitor nesitor commented Jul 2, 2024

Problem: As a user we cannot create a confidential VM using the SDK.

Solution: Implement new confidential VM fields.

Solution: Implement new confidential VM fields.
@nesitor nesitor requested review from hoh, olethanh and philogicae July 2, 2024 12:51
@nesitor nesitor self-assigned this Jul 2, 2024
@github-actions github-actions bot added the RED This PR is complex and may require more time to review. label Jul 2, 2024
@github-actions
Copy link

github-actions bot commented Jul 2, 2024

The PR also contains a mix of small changes and large changes. Small changes include changes to the 'Payment' class, which appears to be a simple refactoring. Large changes include the complex modifications to the 'HostRequirements', 'NodeRequirements', and 'TrustedExecutionEnvironment' classes.

Overall, the PR appears to be complex and may have a high potential for introducing bugs. Only the most experienced developers should review this PR.

--- aleph-im/aleph-sdk-python

Category: RED

Changes to HostRequirements class in 'aleph/sdk/client/authenticated_http.py' file. This change involves a complex addition and deletion of methods and properties, which may impact the behavior of the code in unexpected ways.

Changes to test_create_confidential_instance function in 'tests/unit/test_asynchronous.py' file. This change involves a complex modification, which may impact the behavior of the tests in unexpected ways.

This PR contains a mix of small changes and large changes, which may increase the risk of introducing bugs. Only the most experienced developers should review this PR.

Changes to HostRequirements class in 'aleph/sdk/client/authenticated_http.py' file. This change involves a complex addition and deletion of methods and properties, which may impact the behavior of the code in unexpected ways. 

Changes to test_create_confidential_instance function in 'tests/unit/test_asynchronous.py' file. This change involves a complex modification, which may impact the behavior of the tests in unexpected ways. 

This PR contains a mix of small changes and large changes, which may increase the risk of introducing bugs. Only the most experienced developers should review this PR.

--- aleph-im/aleph-sdk-python

This PR appears to be categorized as 'RED'. The diff contains changes to the 'aleph/sdk/client/authenticated_http.py' file, which appears to be a part of the SDK library for the Aleph.im network. This change involves altering the 'HostRequirements' class, which appears to be a complex modification. The PR also contains changes to the 'test_asynchronous.py' file, which appears to be a part of the unit tests for the SDK. These changes involve altering the 'test_create_confidential_instance' function, which appears to be a complex modification.

The PR also contains a mix of small changes and large changes. Small changes include changes to the 'Payment' class, which appears to be a simple refactoring. Large changes include the complex modifications to the 'HostRequirements', 'NodeRequirements', and 'TrustedExecutionEnvironment' classes.

Overall, the PR appears to be complex and may have a high potential for introducing bugs. Only the most experienced developers should review this PR.

Changes to HostRequirements class in 'aleph/sdk/client/authenticated_http.py' file. This change involves a complex addition and deletion of methods and properties, which may impact the behavior of the code in unexpected ways. 

Changes to test_create_confidential_instance function in 'tests/unit/test_asynchronous.py' file. This change involves a complex modification, which may impact the behavior of the tests in unexpected ways. 

This PR contains a mix of small changes and large changes, which may increase the risk of introducing bugs. Only the most experienced developers should review this PR.
import difflib

def diff(a, b):
    return '\n'.join(difflib.unified_diff(a.splitlines(), b.splitlines()))

a = """
- from aleph_message.models.execution.environment import HypervisorType, MachineResources
+ from aleph_message.models.execution.environment import HostRequirements, HypervisorType, MachineResources, NodeRequirements
"""
b = """
- from aleph_message.models.execution.environment import HypervisorType, MachineResources
+ from aleph_message.models.execution.environment import HostRequirements, HypervisorType, MachineResources, NodeRequirements, TrustedExecutionEnvironment
"""

print(diff(a, b))

This script will output:

--- 
+++ 
- from aleph_message.models.execution.environment import HypervisorType, MachineResources
+ from aleph_message.models.execution.environment import HostRequirements, HypervisorType, MachineResources, NodeRequirements

This shows the changes in the import statements from the 'aleph_message.models.execution.environment' module.

import re

def find_imports(diff):
    import_re = re.compile(r'\+\s*from (.*?) import (.*)')
    for line in diff.splitlines():
        match = import_re.match(line)
        if match:
            yield match.groups()

diff = """
--- 
+++ 
- from aleph_message.models.execution.environment import HypervisorType, MachineResources
+ from aleph_message.models.execution.environment import HostRequirements, HypervisorType, MachineResources, NodeRequirements
"""

for module, classes in find_imports(diff):
    print(f'Module: {module}, Classes: {classes}')

# Outputs:
# Module: aleph_message.models.execution.environment, Classes: HostRequirements, HypervisorType, MachineResources, NodeRequirements

This script will find and print the module and classes that are being imported in the diff.

@nesitor nesitor requested a review from hoh July 2, 2024 14:08
Psycojoker
Psycojoker previously approved these changes Jul 4, 2024
Copy link
Collaborator

@Psycojoker Psycojoker left a comment

Choose a reason for hiding this comment

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

I can't tell for the business logic part really well yet, but at least from the python part this PR looks good to me 👍

@nesitor nesitor merged commit be5a697 into main Jul 5, 2024
@Psycojoker Psycojoker deleted the andres-feature-add_confidential_new_fields branch July 24, 2024 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RED This PR is complex and may require more time to review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants