-
Notifications
You must be signed in to change notification settings - Fork 92
feat: Add set node account id method #362
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
base: main
Are you sure you want to change the base?
Changes from all commits
f694268
d5310c8
76b0d85
256e807
32443f1
3c3a6ea
1a453ee
1c6ac60
df1069c
67c9eb7
d7d6c18
79c1f10
a4372ad
17ace16
090b2bb
854c0ca
168d706
143b727
ad6df88
fdcc0cb
b9ad245
7f4ee47
e17c504
22ba822
2741bde
9621065
9469f23
cfd8e1c
1cb2132
9328d1f
e594364
a5267b8
13b3573
177248e
7744971
277bf9d
1f4f5cf
91d86fb
fce4f9f
333f755
50f258e
94c89d3
e83e701
cc7d9a8
dac9cd2
13a542a
7ffc7e4
f521241
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -59,6 +59,9 @@ def __init__(self) -> None: | |
| self.node_index: int = 0 | ||
| self.payment_amount: Optional[Hbar] = None | ||
|
|
||
| self.node_account_ids: Optional[List[AccountId]] = None | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just seen - this is duplicated I've also created a branch here to see if it all works with some minor changes, see here: mostly wanted to fix the weird error in the pipeline, hard to see what's going on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for taking the time to clean this up and address the CI issues. I’ve reviewed the changes in your branch and they all make sense. Everything looks good from my side.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @aceppaluni
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @exploreriii, I’m currently awaiting @nadineloepfe 's guidance on how she’d like to proceed. My understanding is that the branch she created will address the test issue and allow us to finalize this PR.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @exploreriii @aceppaluni: correct! the branch I've raised is passing, so if you'd like you can take inspiration there, and we can also jump on a call to walk through what has been done and why - bit easier than doing it here in writing :)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nadineloepfe @exploreriii : Great! Let me know what days and times work for everyone! |
||
| self._used_node_account_id: Optional[AccountId] = None | ||
|
|
||
| def _get_query_response(self, response: Any) -> query_pb2.Query: | ||
| """ | ||
| Extracts the query-specific response object from the full response. | ||
|
|
@@ -379,3 +382,4 @@ def _is_payment_required(self) -> bool: | |
| bool: True if payment is required, False otherwise | ||
| """ | ||
| return True | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| import pytest | ||
| from hiero_sdk_python.query.query import Query | ||
| from hiero_sdk_python.account.account_id import AccountId | ||
|
|
||
| def test_set_single_node_account_id(): | ||
| q = Query() | ||
| node = AccountId(0, 0, 3) | ||
|
|
||
| q.set_node_account_id(node) | ||
|
|
||
| assert q.node_account_ids == [node] | ||
| assert q._used_node_account_id is None # not selected until execution | ||
|
|
||
| def test_set_multiple_node_account_ids(): | ||
| q = Query() | ||
| nodes = [AccountId(0, 0, 3), AccountId(0, 0, 4)] | ||
|
|
||
| q.set_node_account_ids(nodes) | ||
|
|
||
| assert q.node_account_ids == nodes | ||
| assert q._used_node_account_id is None | ||
|
|
||
| def test_select_node_account_id(): | ||
| q = Query() | ||
| nodes = [AccountId(0, 0, 3), AccountId(0, 0, 4)] | ||
| q.set_node_account_ids(nodes) | ||
|
|
||
| selected = q._select_node_account_id() | ||
|
|
||
| assert selected == nodes[0] | ||
| assert q._used_node_account_id == nodes[0] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import pytest | ||
| from hiero_sdk_python.transaction.transaction import Transaction | ||
| from hiero_sdk_python.account.account_id import AccountId | ||
|
|
||
|
|
||
| class DummyTransaction(Transaction): | ||
| """ | ||
| Minimal subclass of Transaction for testing. | ||
| Transaction is abstract (requires build methods), so we stub them out. | ||
| """ | ||
| def __init__(self): | ||
| super().__init__() | ||
|
|
||
| def build_base_transaction_body(self): | ||
| return None # stub | ||
|
|
||
| def _make_request(self): | ||
| return None # stub | ||
|
|
||
| def _get_method(self): | ||
| return None # stub | ||
|
|
||
|
|
||
| def test_set_single_node_account_id(): | ||
| txn = DummyTransaction() | ||
| node = AccountId(0, 0, 3) | ||
|
|
||
| txn.set_node_account_id(node) | ||
|
|
||
| assert txn.node_account_ids == [node] | ||
| assert txn._used_node_account_id is None | ||
|
|
||
|
|
||
| def test_set_multiple_node_account_ids(): | ||
| txn = DummyTransaction() | ||
| nodes = [AccountId(0, 0, 3), AccountId(0, 0, 4)] | ||
|
|
||
| txn.set_node_account_ids(nodes) | ||
|
|
||
| assert txn.node_account_ids == nodes | ||
| assert txn._used_node_account_id is None | ||
|
|
||
|
|
||
| def test_select_node_account_id(): | ||
| txn = DummyTransaction() | ||
| nodes = [AccountId(0, 0, 3), AccountId(0, 0, 4)] | ||
| txn.set_node_account_ids(nodes) | ||
|
|
||
| selected = txn._select_node_account_id() | ||
|
|
||
| assert selected == nodes[0] | ||
| assert txn._used_node_account_id == nodes[0] |
Uh oh!
There was an error while loading. Please reload this page.
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.
here in the imports you need to add
Listto:from typing import Callable, Optional, Any, TYPE_CHECKING, ListThere 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.
and actually underL
I'd also add AccountId