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 _load_account when both string & path private key are provided #160

Merged
merged 2 commits into from
Aug 28, 2024

Conversation

philogicae
Copy link
Member

Check title

@github-actions github-actions bot added the BLUE This PR is simple and straightforward. label Aug 28, 2024
Copy link

Summary:
This PR contains a small change to the account.py file, specifically addressing an assertion that ensures a private key is not provided both as a string and as a file path. This change is likely to be a simple refactoring or a minor bug fix, with minimal potential impact on the codebase.

Highlighting relevant parts of the diff:

-    assert not (
-        private_key_str and private_key_path
-    ), "Private key should be a string or a filepath, not both."
+    """Load private key from a string or a file."""

Explanation:
The provided diff shows a change to the docstring of the _load_account function, simplifying the assertion that checks if both private_key_str and private_key_path are provided. This change does not introduce any new functionality or modify existing logic significantly. It is likely aimed at improving clarity and reducing the potential for bugs by explicitly stating what the function does. Given the nature of the change, it falls under the 'BLUE' category, indicating a low complexity and minimal risk for the codebase.

Copy link
Contributor

@olethanh olethanh left a comment

Choose a reason for hiding this comment

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

That's not a bug, that a warning because both shouldn't be provided at the same time.

@olethanh olethanh self-requested a review August 28, 2024 13:46
src/aleph/sdk/account.py Outdated Show resolved Hide resolved
Copy link
Contributor

@olethanh olethanh left a comment

Choose a reason for hiding this comment

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

After discussion with @philogicae I was wrong since the SDK provide a file by default it was failing when we passed a string adress on the client.

@philogicae philogicae merged commit 5e0c979 into main Aug 28, 2024
13 checks passed
@philogicae philogicae deleted the fix-load-account branch August 28, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BLUE This PR is simple and straightforward.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants