Problem: New collections are not created in existing DB.#2520
Problem: New collections are not created in existing DB.#2520vrde merged 4 commits intobigchaindb:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2520 +/- ##
=========================================
- Coverage 93.37% 93.2% -0.18%
=========================================
Files 43 44 +1
Lines 2583 2547 -36
=========================================
- Hits 2412 2374 -38
- Misses 171 173 +2 |
| try: | ||
| logger.info(f'Create `{table_name}` table.') | ||
| conn.conn[dbname].create_collection(table_name) | ||
| create_indexes(conn, dbname, table_name, INDEXES[table_name]) |
There was a problem hiding this comment.
What if we just add new index?
There was a problem hiding this comment.
We can. But we would need to change logging then. Unconditional Creating indexes for .. messages are disorienting.
There was a problem hiding this comment.
Then we should change logging and ensure all the indexes exist too
There was a problem hiding this comment.
In the code above, if the collection already exists then the exception is generated and the code jumps to the except clause. So if there is merely an addition of a new index then that wouldn't be updated in the existing collection. The point here would be to ensure that the collection and indexes are the way we expect them to be
There was a problem hiding this comment.
I was thinking about it. Most of our indexes come with a unique constraint. Unique constraints are not backwards-compatible and thus can not be introduced by a new bigchaindb start or bigchaindb init.
On the other hand, we are losing a good means of adding those rare indexes without constraints.. So, I think it's better to arrange it how you suggest.
|
|
||
|
|
||
| @pytest.mark.tendermint | ||
| def test_bigchain_run_init_when_db_exists(mocker, capsys): |
There was a problem hiding this comment.
I guess this test could be re-purposed to asset a success message?
There was a problem hiding this comment.
This particular test does not use a real database but asserts that one function catches an exception raised by the other.
There was a problem hiding this comment.
We can remove the mock and execute the actual code. But I would recommend skipping this comment if its too much work.
There was a problem hiding this comment.
There is a pattern of not using the real database in this file so I would leave it to be handled by future test (re)organisation efforts.
ldmberman
force-pushed
the
create-collections-if-do-not-exist
branch
from
bc3b13a to
479d6d7
Compare
Solution: Do not abort the initialisation if a collection exists. Unify the index creation.
ldmberman
force-pushed
the
create-collections-if-do-not-exist
branch
from
479d6d7 to
3a0e1af
Compare
vrde
left a comment
vrde
left a comment
There was a problem hiding this comment.
Nice job!
I see you added/removed some markers from the tests, is it something specific to this PR? If not, they should live in another one.
| return create_tx.sign([alice.private_key]) | ||
|
|
||
|
|
||
| @pytest.mark.abci |
There was a problem hiding this comment.
Is it something related to this PR?
| register_schema = module_dispatch_registrar(backend.schema) | ||
|
|
||
|
|
||
| INDEXES = { |
4 participants
Solution: Do not abort the initialisation if a collection exists. Unify the index creation.