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.

Sign up for GitHub

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

Jump to bottom

Add new transaction retry integration test #89

Merged
merged 6 commits into from
Apr 15, 2022
Merged

Add new transaction retry integration test #89

merged 6 commits into from
Apr 15, 2022

Conversation

heifner
Copy link
Member

@heifner heifner commented Apr 8, 2022 •
edited
Loading

  • Includes a fix for transaction retry expiration comparison.
  • New integration test for transaction retry feature
# This test sets up 3 producing nodes and 4 non-producing
#   nodes; 2 API nodes and 2 relay nodes. The API nodes will be
#   sent many transfers.  When it is complete it verifies that
#   all of the transactions made it into blocks.
#
# During processing the two relay nodes are killed so that
# one of the api nodes is isolated and its transactions will
# be lost. The api node retry logic should allow it to resend
# those transactions once connectivity is restored.

@heifner heifner changed the title WIP: Add new transaction retry integration test Add new transaction retry integration test Apr 14, 2022
brianjohnson5972
brianjohnson5972 suggested changes Apr 14, 2022
View reviewed changes
tests/Node.py Outdated
cmd="%s %s -v transfer -j %s %s" % (
Utils.EosClientPath, self.eosClientArgs(), source.name, destination.name)
cmdRetry = ""
if retry is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should either have a method for getting the retry cmd or else have a Retry class that gets created with the retry value and passed in and the to_string conversion could return the cmdRetry (default will be retry=Retry() and that will return ""), since this can really be passed along to any command that eventually calls send_transaction

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

tests/Node.py Outdated
@@ -752,6 +760,48 @@ def transferFunds(self, source, destination, amountStr, memo="memo", force=False

return self.waitForTransBlockIfNeeded(trans, waitForTransBlock, exitOnError=exitOnError)

# Trasfer funds. Returns (popen, cmdArr) for checkDelayedOutput
def transferFundsAsync(self, source, destination, amountStr, memo="memo", force=False, exitOnError=True, retry=None):
assert isinstance(amountStr, str)
Copy link
Contributor

Choose a reason for hiding this comment

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

What about making a __transferFundsCmdLine (or whatever the internal convention is) that created the cmd line for all transferFunds*** methods?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@brianjohnson5972 brianjohnson5972 merged commit e1ca58c into feature/oci_api_phase1 Apr 15, 2022
@brianjohnson5972 brianjohnson5972 deleted the ENFS-37-retry-trx-test branch April 15, 2022 04:34
@heifner heifner mentioned this pull request Apr 28, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brianjohnson5972 brianjohnson5972 brianjohnson5972 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@heifner @brianjohnson5972