-
Notifications
You must be signed in to change notification settings - Fork 374
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
add-ledgerId field in validator #461
Conversation
Signed-off-by: Lovesh <[email protected]>
Signed-off-by: Lovesh <[email protected]>
@@ -55,6 +56,7 @@ class ClientNYMOperation(MessageValidator): | |||
class ClientGetTxnOperation(MessageValidator): | |||
schema = ( | |||
(TXN_TYPE, ConstantField(GET_TXN)), | |||
(f.LEDGER_ID.nm, LedgerIdField()), |
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.
Adding a new field breaks backward compatibility, isn't it?
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.
Please make it optional
Signed-off-by: Lovesh <[email protected]>
@@ -9,24 +11,48 @@ | |||
op_get_txn = ClientGetTxnOperation() | |||
|
|||
|
|||
def test_small_seq_no_fails(): | |||
def test_no_ledgerId_fails(): |
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.
It should be test_no_ledgerId_succeeds
* add-ledgerId field in validator Signed-off-by: Lovesh <[email protected]> * uncommenting Signed-off-by: Lovesh <[email protected]> * making ledger id optional Signed-off-by: Lovesh <[email protected]>
Signed-off-by: Lovesh [email protected]