Skip to content

Added a ledger chunk class to manage the Transactions in a single chunk#1644

Merged
achamayou merged 10 commits into
microsoft:masterfrom
msftsettiy:master
Sep 29, 2020
Merged

Added a ledger chunk class to manage the Transactions in a single chunk#1644
achamayou merged 10 commits into
microsoft:masterfrom
msftsettiy:master

Conversation

@msftsettiy

Copy link
Copy Markdown
Member

This PR contains changes to split the Ledger class into two. The Ledger class is responsible for iterating over the ledger chunks and the Ledgerchunk class is responsible for iterating over the Transactions in a chunk.

@msftsettiy msftsettiy requested a review from a team as a code owner September 23, 2020 18:54
Comment thread python/ccf/ledger.py Outdated
Comment thread python/ccf/ledger.py Outdated
Comment thread python/ccf/ledger.py Outdated
Comment thread python/ccf/ledger.py Outdated
Comment thread python/ccf/ledger.py Outdated
Comment thread python/ccf/ledger.py Outdated
Comment thread python/ccf/ledger.py
@achamayou

Copy link
Copy Markdown
Member

Thanks @msftsettiy, that change looks good!

@jumaffre

Copy link
Copy Markdown
Contributor

/azp run

Comment thread python/ccf/ledger.py Outdated
@ghost

ghost commented Sep 24, 2020

Copy link
Copy Markdown

master@13311 aka 20200929.38 vs master ewma over 50 builds from 12741 to 13311
images

@jumaffre

Copy link
Copy Markdown
Contributor

@msftsettiy Changes look good - thanks!

Could you format ledger.py with black? Also, the governance_history_test seems to be failing:

38: Traceback (most recent call last):
38:   File "/mnt/build/2/s/tests/governance_history.py", line 124, in <module>
38:     run(args)
38:   File "/mnt/build/2/s/tests/governance_history.py", line 72, in run
38:     ) = count_governance_operations(ledger)
38:   File "/mnt/build/2/s/tests/governance_history.py", line 23, in count_governance_operations
38:     tables = tr.get_public_domain().get_tables()
38: AttributeError: 'Ledgerchunk' object has no attribute 'get_public_domain'
29/51 Test #38: governance_history_test ..........***Failed   11.66 sec

It is the only test we have that uses the ledger.py file. The fix should be pretty trivial but please reach out if you need a hand.

@jumaffre

Copy link
Copy Markdown
Contributor

/azp run

@achamayou

Copy link
Copy Markdown
Member

/azp run

@achamayou

Copy link
Copy Markdown
Member

@msftsettiy could you run './scripts/ci-checks.sh -f' to apply canonical formatting to your changes please? The PR is otherwise ready to merge.

@eddyashton

Copy link
Copy Markdown
Member

/azp run

@achamayou achamayou merged commit 3bfac60 into microsoft:master Sep 29, 2020
@achamayou achamayou mentioned this pull request Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants