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

Fix: create_instance required a program encoding #100

Merged
merged 1 commit into from
Feb 8, 2024

Conversation

hoh
Copy link
Member

@hoh hoh commented Feb 8, 2024

Problem: Instances don't use program encodings. This argument was left from refactoring.

Solution: Drop the argument.

@hoh hoh requested a review from MHHukiewitz February 8, 2024 17:07
Copy link

github-actions bot commented Feb 8, 2024

The PR modifies multiple files in the aleph-sdk-python repository. The changes are extensive and may have a high potential for introducing bugs or affecting existing functionality. It appears to involve significant refactoring, new features, workflows, and modifications of existing logic. This is likely due to the large number of changed lines (1006) in this PR.

The diff shows that changes are made across multiple files including abstract.py which seems to be a core file for the SDK client. The refactoring includes removing an unused parameter 'encoding' and adding new parameters like 'volumes', 'volume_persistence', and 'ssh_keys'.

These changes could potentially affect other parts of the codebase that depend on this SDK, as well as any applications or services using it. Therefore, a deep understanding of the project architecture is required to review these changes effectively. Only experienced developers should be allowed to merge these changes into the main branch. The label 'BLACK' indicates this level of complexity and urgency for reviewing and merging these PRs.

Please note that while this PR may seem complex, it also provides new features or improvements which can benefit other parts of the codebase. It is important to consider the potential impact on the system as a whole when making decisions about whether to merge these changes.

This response was generated by the Code Review Categorizer (CRC) and is designed for machine parsing, so it follows markdown formatting and bullet points for clarity.

@github-actions github-actions bot added the BLACK This PR has critical implications and must be reviewed by a senior engineer. label Feb 8, 2024
Problem: Instances don't use program encodings. This argument was left from refactoring.

Solution: Drop the argument.
@hoh hoh force-pushed the hoh-fix-instance-encoding branch from ca43c3d to f88183e Compare February 8, 2024 17:35
Copy link
Member

@MHHukiewitz MHHukiewitz left a comment

Choose a reason for hiding this comment

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

LGTM

@MHHukiewitz MHHukiewitz merged commit fed1d95 into main Feb 8, 2024
11 checks passed
@MHHukiewitz MHHukiewitz deleted the hoh-fix-instance-encoding branch February 9, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLACK This PR has critical implications and must be reviewed by a senior engineer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants