Problem: Bigchain class is unused and redundant#2366
Problem: Bigchain class is unused and redundant#2366ttmc merged 17 commits intobigchaindb:masterfrom
Conversation
Solution: Remove core.py. Refactor BigchainDB Class to remove inheritance from Bigchain.
Solution: Remove core.py. Refactor BigchainDB Class to remove inheritance from Bigchain.
…its, as I don't know what I'm doing, and I can't experiment without running the CI... Sorry in advance!
…its, as I don't know what I'm doing, and I can't experiment without running the CI... Sorry in advance!
…gchaindb into z-bowen-remove_bigchain_class
…ivate method, so I had to align with that.
Codecov Report
@@ Coverage Diff @@
## master #2366 +/- ##
=========================================
- Coverage 86.96% 86.87% -0.1%
=========================================
Files 38 38
Lines 2132 2163 +31
=========================================
+ Hits 1854 1879 +25
- Misses 278 284 +6 |
bigchaindb/models.py
Outdated
| bigchain (:class:`~bigchaindb.Bigchain`): An instantiated Bigchain | ||
| object. | ||
| bigchain (:class:`~bigchaindb.tendermint.BigchainDB`): An | ||
| instantiated BigchainDB object. |
There was a problem hiding this comment.
This whole class is going to be removed in #2368
|
|
||
| from bigchaindb import backend | ||
| from bigchaindb import Bigchain | ||
| import bigchaindb |
There was a problem hiding this comment.
Do we need this import?
There was a problem hiding this comment.
It was like that in the old Bigchain class, and it causes my local unit tests to fail if I change it. I agree that it's ugly, and we should fix it, but I think it's outside the scope of this PR.
tests/db/test_bigchain_api.py
Outdated
| def test_get_owned_ids_calls_get_outputs_filtered(): | ||
| from bigchaindb.core import Bigchain | ||
| from bigchaindb.tendermint import BigchainDB | ||
| with patch('bigchaindb.core.Bigchain.get_outputs_filtered') as gof: |
There was a problem hiding this comment.
I think this needs to be changed too
There was a problem hiding this comment.
Yup. Good thing this test is deactivated! lol
|
For codecov I wouldn't mind merging it with a decrease of .3 |
| BLOCK_UNDECIDED = TX_UNDECIDED = 'undecided' | ||
| """return if block is undecided, or tx is in undecided block""" | ||
|
|
||
| TX_IN_BACKLOG = 'backlog' |
There was a problem hiding this comment.
The parameters BLOCK_INVALID, BLOCK_VALID, BLOCK_UNDECIDED , TX_IN_BACKLOG are not required anymore and should be removed.
There was a problem hiding this comment.
It breaks a bunch of tests when I remove those parameters. I suggest we excavate them in a separate PR.
There was a problem hiding this comment.
I think this is a good idea. This PR is already big enough.
There was a problem hiding this comment.
I'm taking care of removing the parameters in a different PR
Solution: resolved merge conflicts to excavate Block class from my branch
|
There are some conflicts in this change now, probably because of #2342 |
Solution: Merged and repushed
Solution: Merge changes into Master
7 participants
Solution
Remove Bigchain class and BigchainDB's dependency on it.