Bitmessage/PyBitmessage-2024-12-05
Archived
2
0
Fork 0
Code Issues 168 Pull Requests 53 Actions Packages Projects Releases 5 Wiki Activity

Refactor sqlthread #1794

Open
cis-muzahid wants to merge 1 commits from cis-muzahid/test_refactor_sqlthread into v0.6
pull from: cis-muzahid/test_refactor_sqlthread
merge into: Bitmessage:v0.6
Conversation 7 Commits 1 Files Changed 4 +547 -537
cis-muzahid commented 2021-07-28 10:33:50 +02:00 (Migrated from github.com)
Copy Link

Refactor sqlthread and add test cases for setting versions and SQL internal function called create_function

Refactor sqlthread and add test cases for setting versions and SQL internal function called create_function
g1itch (Migrated from github.com) reviewed 2021-07-28 10:33:50 +02:00
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:40:20 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
PeterSurda (Migrated from github.com) commented 2021-07-28 10:40:20 +02:00

this should contain an argument that allows to use an in memory database instead of the file.

this should contain an argument that allows to use an in memory database instead of the file.
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:41:02 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
PeterSurda (Migrated from github.com) commented 2021-07-28 10:41:02 +02:00

here allow to call it optionally with a parameter for in memory database

here allow to call it optionally with a parameter for in memory database
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:42:48 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
PeterSurda (Migrated from github.com) commented 2021-07-28 10:42:48 +02:00

this should be in parent class

this should be in parent class
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:43:00 +02:00
src/class_sqlThread.py
@ -491,3 +210,3 @@
parameters = (int(time.time()),)
self.cur.execute(item, parameters)
self.cur.execute(query, parameters)
PeterSurda (Migrated from github.com) commented 2021-07-28 10:43:00 +02:00

this should be in parent class

this should be in parent class
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:43:11 +02:00
src/class_sqlThread.py
@ -641,0 +430,4 @@
helper_sql.sql_available = True
config_ready.wait()
PeterSurda (Migrated from github.com) commented 2021-07-28 10:43:11 +02:00

this should all be in parent class

this should all be in parent class
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:43:27 +02:00
src/class_sqlThread.py
@ -641,0 +345,4 @@
'(while movemessagstoappdata) Alert: Your disk or data storage volume is full.'
' sqlThread will now exit.')
queues.UISignalQueue.put((
'alert', (
PeterSurda (Migrated from github.com) commented 2021-07-28 10:43:27 +02:00

move to parent

move to parent
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:43:36 +02:00
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-07-28 10:43:36 +02:00

move to parent

move to parent
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:44:24 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
PeterSurda (Migrated from github.com) commented 2021-07-28 10:44:24 +02:00

only this should be in this class, the other functionality should be refactored into methods and put into parent.

only this should be in this class, the other functionality should be refactored into methods and put into parent.
PeterSurda (Migrated from github.com) reviewed 2021-07-28 10:45:56 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
PeterSurda (Migrated from github.com) commented 2021-07-28 10:45:55 +02:00

maybe an UpgradeDB object would be an attribute of the sqlThread rarther than sqlThread inheriting it. That way we isolate the DB stuff and can test it without threading, and the thread would need to use the same interfaces as the test.

maybe an `UpgradeDB` object would be an attribute of the `sqlThread` rarther than `sqlThread` inheriting it. That way we isolate the DB stuff and can test it without threading, and the thread would need to use the same interfaces as the test.
cis-muzahid (Migrated from github.com) reviewed 2021-08-02 18:33:19 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
cis-muzahid (Migrated from github.com) commented 2021-08-02 18:33:18 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-02 18:33:21 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
cis-muzahid (Migrated from github.com) commented 2021-08-02 18:33:21 +02:00

pass arguments with DB file name

pass arguments with DB file name
cis-muzahid (Migrated from github.com) reviewed 2021-08-02 18:33:23 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
cis-muzahid (Migrated from github.com) commented 2021-08-02 18:33:23 +02:00

Inherit connection vars from UpgradeDB class

Inherit connection vars from UpgradeDB class
cis-muzahid (Migrated from github.com) reviewed 2021-08-04 18:05:10 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
cis-muzahid (Migrated from github.com) commented 2021-08-04 18:05:10 +02:00

Changes done,
Refactored code and kept in Upgrade DB,

Changes done, Refactored code and kept in Upgrade DB,
cis-muzahid (Migrated from github.com) reviewed 2021-08-04 18:05:26 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
cis-muzahid (Migrated from github.com) commented 2021-08-04 18:05:26 +02:00

done

done
cis-muzahid (Migrated from github.com) reviewed 2021-08-04 18:05:36 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-04 18:05:36 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-04 18:05:42 +02:00
src/class_sqlThread.py
@ -641,0 +345,4 @@
'(while movemessagstoappdata) Alert: Your disk or data storage volume is full.'
' sqlThread will now exit.')
queues.UISignalQueue.put((
'alert', (
cis-muzahid (Migrated from github.com) commented 2021-08-04 18:05:42 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-04 18:05:49 +02:00
src/class_sqlThread.py
@ -641,0 +430,4 @@
helper_sql.sql_available = True
config_ready.wait()
cis-muzahid (Migrated from github.com) commented 2021-08-04 18:05:49 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-04 18:06:06 +02:00
src/class_sqlThread.py
@ -491,3 +210,3 @@
parameters = (int(time.time()),)
self.cur.execute(item, parameters)
self.cur.execute(query, parameters)
cis-muzahid (Migrated from github.com) commented 2021-08-04 18:06:05 +02:00

Done

Done
PeterSurda (Migrated from github.com) requested changes 2021-08-05 10:53:27 +02:00
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-08-05 10:26:23 +02:00

make another class which overrides this to point to in-memory database, this can then be used for tests. You can call it something like class MockDB(UpgradeDB)

make another class which overrides this to point to in-memory database, this can then be used for tests. You can call it something like `class MockDB(UpgradeDB)`
PeterSurda (Migrated from github.com) commented 2021-08-05 10:26:37 +02:00

Put this inside UpgradeDB

Put this inside `UpgradeDB`
PeterSurda (Migrated from github.com) commented 2021-08-05 10:27:11 +02:00

update docstring

update docstring
PeterSurda (Migrated from github.com) commented 2021-08-05 10:27:31 +02:00

update docstring

update docstring
PeterSurda (Migrated from github.com) commented 2021-08-05 10:35:07 +02:00

this should return the version, not store

this should return the version, not store
PeterSurda (Migrated from github.com) commented 2021-08-05 10:35:57 +02:00

should not store

should not store
PeterSurda (Migrated from github.com) commented 2021-08-05 10:37:09 +02:00

use atomic increment

'''UPDATE settings SET value=value+1 WHERE key='version';'''

But check for types (string vs. int).

use atomic increment ``` '''UPDATE settings SET value=value+1 WHERE key='version';''' ```` But check for types (string vs. int).
PeterSurda (Migrated from github.com) commented 2021-08-05 10:40:22 +02:00 
while self.settings_version < self.max_level:
    self._upgrade_one_level_sql_statement(self.settings_version)
    self.settings_version = 1
``` while self.settings_version < self.max_level: self._upgrade_one_level_sql_statement(self.settings_version) self.settings_version = 1 ```
PeterSurda (Migrated from github.com) commented 2021-08-05 10:42:48 +02:00

no need for this

no need for this
PeterSurda (Migrated from github.com) commented 2021-08-05 10:43:21 +02:00

typo

typo
PeterSurda (Migrated from github.com) commented 2021-08-05 10:44:08 +02:00

should be called something like init_database

should be called something like `init_database`
src/class_sqlThread.py
@ -86,2 +80,2 @@
if str(err) == 'table inbox already exists':
logger.debug('Database file already exists.')
logger.debug(
'ERROR trying to initialize database. Error message: %s\n', str(err))
PeterSurda (Migrated from github.com) commented 2021-08-05 10:25:00 +02:00

@getter something

```@getter something```
src/class_sqlThread.py
@ -423,0 +132,4 @@
# too-many-branches, too-many-statements
"""
Check if sqlite can store binary zeros.
"""
PeterSurda (Migrated from github.com) commented 2021-08-05 10:29:03 +02:00

we should at least differenciate between SQL exceptions and other exceptions (e.g. file not found)

we should at least differenciate between SQL exceptions and other exceptions (e.g. file not found)
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-08-05 10:30:55 +02:00

this check is only for testing if the database already has been initialised. It wasn't used to check upgrade levels.

this check is only for testing if the database already has been initialised. It wasn't used to check upgrade levels.
src/class_sqlThread.py
@ -641,0 +397,4 @@
_translate(
"MainWindow",
"Disk full"),
_translate(
PeterSurda (Migrated from github.com) commented 2021-08-05 10:47:10 +02:00

write getter + setter for this

write getter + setter for this
PeterSurda (Migrated from github.com) commented 2021-08-05 10:50:14 +02:00

Something like this to skip init when running tests:

if not self.instanceof(MockDB):
   self.initiate_database()
Something like this to skip init when running tests: ``` if not self.instanceof(MockDB): self.initiate_database() ```
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-08-05 10:45:12 +02:00

different name, to show it's related to BMConfigParser version rather than sql version

different name, to show it's related to `BMConfigParser` version rather than sql version
src/debug.py
PeterSurda (Migrated from github.com) commented 2021-08-05 10:51:07 +02:00

puth these lines together

puth these lines together
cis-muzahid (Migrated from github.com) reviewed 2021-08-09 08:27:43 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-09 08:27:42 +02:00

Change name to upgrade_config_parser_setting_version

Change name to ```upgrade_config_parser_setting_version```
cis-muzahid (Migrated from github.com) reviewed 2021-08-09 08:29:29 +02:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
cis-muzahid (Migrated from github.com) commented 2021-08-09 08:29:29 +02:00

Moved in to __init__

Moved in to ```__init__```
cis-muzahid (Migrated from github.com) reviewed 2021-08-09 08:30:47 +02:00
src/class_sqlThread.py
@ -86,2 +80,2 @@
if str(err) == 'table inbox already exists':
logger.debug('Database file already exists.')
logger.debug(
'ERROR trying to initialize database. Error message: %s\n', str(err))
cis-muzahid (Migrated from github.com) commented 2021-08-09 08:30:47 +02:00

Already using getter for the same

Already using getter for the same
cis-muzahid (Migrated from github.com) reviewed 2021-08-09 08:32:08 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-09 08:32:07 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-09 08:52:55 +02:00
src/class_sqlThread.py
@ -423,0 +132,4 @@
# too-many-branches, too-many-statements
"""
Check if sqlite can store binary zeros.
"""
cis-muzahid (Migrated from github.com) commented 2021-08-09 08:52:55 +02:00

Differed between
except IOError as err: and except Exception as err:
Not add others exceptions

Differed between ```except IOError as err:``` and ```except Exception as err:``` Not add others exceptions
cis-muzahid (Migrated from github.com) reviewed 2021-08-09 09:02:26 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-09 09:02:25 +02:00

Removed this line

Removed this line
cis-muzahid (Migrated from github.com) reviewed 2021-08-09 10:11:43 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-09 10:11:43 +02:00

return value

return value
g1itch (Migrated from github.com) reviewed 2021-08-09 11:50:04 +02:00
src/debug.py
g1itch (Migrated from github.com) commented 2021-08-09 11:50:04 +02:00

You don't need this at all. Just don't use debug in sqlThread and make it like other threads to use own logger, gotten in the class definition, look: https://github.com/Bitmessage/PyBitmessage/blob/v0.6/src/network/threads.py#L9

I reverted it once already: b75585f2

You don't need this at all. Just don't use debug in `sqlThread` and make it like other threads to use own logger, gotten in the class definition, look: https://github.com/Bitmessage/PyBitmessage/blob/v0.6/src/network/threads.py#L9 I reverted it once already: b75585f2
g1itch (Migrated from github.com) reviewed 2021-08-09 11:55:55 +02:00
src/class_sqlThread.py
g1itch (Migrated from github.com) commented 2021-08-09 11:55:55 +02:00

Why don't you add the exception type?

Why don't you add the exception type?
g1itch commented 2021-08-09 12:10:25 +02:00 (Migrated from github.com)
Copy Link

Your sql directories are not python packages and should not contain the empty __init__ modules. If you want to install SQL scripts with the app, put them into the package data: https://github.com/Bitmessage/PyBitmessage/blob/v0.6/setup.py#L137
This doesn't apply to tests, they are not installed, and you may find the files based on os.path.abspath(__file__)

Your `sql` directories are not python packages and should not contain the empty `__init__` modules. If you want to install `SQL` scripts with the app, put them into the package data: https://github.com/Bitmessage/PyBitmessage/blob/v0.6/setup.py#L137 This doesn't apply to tests, they are not installed, and you may find the files based on `os.path.abspath(__file__)`
g1itch (Migrated from github.com) reviewed 2021-08-09 12:14:24 +02:00
src/tests/sql/init_version_5.sql
g1itch (Migrated from github.com) commented 2021-08-09 12:14:24 +02:00

Cool (; I haven't seen this used

$ sqlite3 .config/PyBitmessage/messages.dat
SQLite version 3.34.1 2021-01-20 14:10:07
Enter ".help" for usage hints.
sqlite> select * from knownnodes;
Error: no such table: knownnodes
Cool (; I haven't seen this used ``` $ sqlite3 .config/PyBitmessage/messages.dat SQLite version 3.34.1 2021-01-20 14:10:07 Enter ".help" for usage hints. sqlite> select * from knownnodes; Error: no such table: knownnodes ```
g1itch (Migrated from github.com) reviewed 2021-08-09 12:16:30 +02:00
src/tests/sql/init_version_5.sql
g1itch (Migrated from github.com) commented 2021-08-09 12:16:29 +02:00

Why drop here, if you are going to create it later?

Why drop here, if you are going to create it later?
g1itch (Migrated from github.com) reviewed 2021-08-09 12:19:59 +02:00
src/tests/sql/init_version_7.sql
g1itch (Migrated from github.com) commented 2021-08-09 12:19:59 +02:00

I don't see a difference from the previous version - in the init_version_6.sql

I don't see a difference from the previous version - in the `init_version_6.sql`
PeterSurda (Migrated from github.com) reviewed 2021-08-09 12:22:40 +02:00
src/debug.py
PeterSurda (Migrated from github.com) commented 2021-08-09 12:22:40 +02:00

This PR is still a WIP, you don't need to look at it yet, except in some cases like these imports which are not my strength.

This PR is still a WIP, you don't need to look at it yet, except in some cases like these imports which are not my strength.
PeterSurda (Migrated from github.com) reviewed 2021-08-09 12:23:40 +02:00
src/tests/sql/init_version_7.sql
PeterSurda (Migrated from github.com) commented 2021-08-09 12:23:40 +02:00

I haven't reviewed the SQL files yet, I'm focusing on getting the structure of classes and tests right, it's already at like 3rd iteration.

I haven't reviewed the SQL files yet, I'm focusing on getting the structure of classes and tests right, it's already at like 3rd iteration.
g1itch (Migrated from github.com) reviewed 2021-08-09 12:25:08 +02:00
src/debug.py
g1itch (Migrated from github.com) commented 2021-08-09 12:25:08 +02:00

I receive a notification from this PR, because it was not originally a draft, it was me, who made it draft.

I receive a notification from this PR, because it was not originally a draft, it was me, who made it draft.
PeterSurda commented 2021-08-09 12:26:02 +02:00 (Migrated from github.com)
Copy Link

@g1itch This PR should among other things separate the SQL execution from threads, so that you can run the tests without launching the SQL thread. There is a lot of work and that's why it's taking long.

@g1itch This PR should among other things separate the SQL execution from threads, so that you can run the tests without launching the SQL thread. There is a lot of work and that's why it's taking long.
g1itch commented 2021-08-09 12:43:07 +02:00 (Migrated from github.com)
Copy Link

@g1itch This PR should among other things separate the SQL execution from threads, so that you can run the tests without launching the SQL thread. There is a lot of work and that's why it's taking long.

I see. I have no problem with the speed yet. I'm glad to see many line of code deleted in this PR. But I'd prefer to report the strange things, I can see in the code before it got merged. Because I believe we can broke it harder than before. More tests will help to avoid such case.

> @g1itch This PR should among other things separate the SQL execution from threads, so that you can run the tests without launching the SQL thread. There is a lot of work and that's why it's taking long. I see. I have no problem with the speed yet. I'm glad to see many line of code deleted in this PR. But I'd prefer to report the strange things, I can see in the code before it got merged. Because I believe we can broke it harder than before. More tests will help to avoid such case.
g1itch commented 2021-08-09 12:48:06 +02:00 (Migrated from github.com)
Copy Link

By the way, the core test suite reports an interesting issue here: https://buildbot.bitmessage.org/#builders/25/builds/1515

By the way, the core test suite reports an interesting issue here: https://buildbot.bitmessage.org/#builders/25/builds/1515
cis-muzahid (Migrated from github.com) reviewed 2021-08-12 23:01:41 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-12 23:01:41 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-12 23:01:44 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-12 23:01:43 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-12 23:01:47 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-12 23:01:47 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-12 23:01:50 +02:00
src/class_sqlThread.py
@ -641,0 +397,4 @@
_translate(
"MainWindow",
"Disk full"),
_translate(
cis-muzahid (Migrated from github.com) commented 2021-08-12 23:01:50 +02:00

managed in earlier logic used in memory for test mocking

managed in earlier logic used in memory for test mocking
cis-muzahid (Migrated from github.com) reviewed 2021-08-13 12:35:28 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-13 12:35:28 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-16 16:13:44 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-16 16:13:44 +02:00

returned that value

returned that value
cis-muzahid (Migrated from github.com) reviewed 2021-08-16 18:30:24 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-16 18:30:23 +02:00

convert into this str(int(current_level) + 1)

convert into this str(int(current_level) + 1)
cis-muzahid (Migrated from github.com) reviewed 2021-08-16 19:52:08 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-16 19:52:08 +02:00

Changes are done for that and, but for some query I applied same query script.

Changes are done for that and, but for some query I applied same query script.
cis-muzahid (Migrated from github.com) reviewed 2021-08-17 09:38:30 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-17 09:38:30 +02:00

Applied

        while self._current_level < self.max_level:
            print(self._current_level)
            self._upgrade_one_level_sql_statement(int(self._current_level))
            self._current_level = self._current_level + 1```
Applied ``` while self._current_level < self.max_level: print(self._current_level) self._upgrade_one_level_sql_statement(int(self._current_level)) self._current_level = self._current_level + 1```
cis-muzahid (Migrated from github.com) reviewed 2021-08-17 09:41:16 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-17 09:41:15 +02:00

Removed that

Removed that
PeterSurda (Migrated from github.com) reviewed 2021-08-18 10:45:55 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
PeterSurda (Migrated from github.com) commented 2021-08-18 10:45:55 +02:00 
def connection_build(self, file_name='messages.dat'):
``` def connection_build(self, file_name='messages.dat'): ```
PeterSurda (Migrated from github.com) reviewed 2021-08-18 10:48:05 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
PeterSurda (Migrated from github.com) commented 2021-08-18 10:48:05 +02:00

instead of current_level use ignore

instead of `current_level` use `ignore`
PeterSurda (Migrated from github.com) reviewed 2021-08-18 10:51:37 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
PeterSurda (Migrated from github.com) commented 2021-08-18 10:51:37 +02:00

variable self.current_level shouldn't be needed

variable `self.current_level` shouldn't be needed
PeterSurda (Migrated from github.com) reviewed 2021-08-18 10:56:22 +02:00
src/class_sqlThread.py
@ -641,0 +398,4 @@
"MainWindow",
"Disk full"),
_translate(
"MainWindow",
PeterSurda (Migrated from github.com) commented 2021-08-18 10:56:22 +02:00

separate getter setter for settingsversion

separate getter setter for settingsversion
PeterSurda (Migrated from github.com) reviewed 2021-08-18 10:58:28 +02:00
src/class_sqlThread.py
@ -641,0 +398,4 @@
"MainWindow",
"Disk full"),
_translate(
"MainWindow",
PeterSurda (Migrated from github.com) commented 2021-08-18 10:58:28 +02:00

insetad just create one object and save it, like

self.db = UpgradeDB()

and then just reference
self.db.cur.execute(blabla)

insetad just create one object and save it, like ``` self.db = UpgradeDB() ``` and then just reference `self.db.cur.execute(blabla)`
PeterSurda (Migrated from github.com) reviewed 2021-08-18 10:59:11 +02:00
src/class_sqlThread.py
@ -641,0 +398,4 @@
"MainWindow",
"Disk full"),
_translate(
"MainWindow",
PeterSurda (Migrated from github.com) commented 2021-08-18 10:59:11 +02:00

self.db.create_function(blabla)

`self.db.create_function(blabla)`
PeterSurda (Migrated from github.com) reviewed 2021-08-18 10:59:45 +02:00
src/class_sqlThread.py
@ -641,0 +398,4 @@
"MainWindow",
"Disk full"),
_translate(
"MainWindow",
PeterSurda (Migrated from github.com) commented 2021-08-18 10:59:44 +02:00

self.db.initialize_schema()

`self.db.initialize_schema()`
PeterSurda (Migrated from github.com) reviewed 2021-08-18 10:59:59 +02:00
src/class_sqlThread.py
@ -641,0 +398,4 @@
"MainWindow",
"Disk full"),
_translate(
"MainWindow",
PeterSurda (Migrated from github.com) commented 2021-08-18 10:59:58 +02:00

typo

typo
PeterSurda (Migrated from github.com) reviewed 2021-08-18 11:01:54 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
PeterSurda (Migrated from github.com) commented 2021-08-18 11:01:54 +02:00

just
self._connection_build('messages.dat')

just `self._connection_build('messages.dat')`
PeterSurda (Migrated from github.com) requested changes 2021-08-19 04:58:00 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
PeterSurda (Migrated from github.com) commented 2021-08-18 11:04:27 +02:00

instead of return, just

self.conn = conn
self.cur = cur

and then in MockDB just override __init__ to call `self._connection_build(':memory:')
Then you can remove all ifs here

instead of return, just ``` self.conn = conn self.cur = cur ``` and then in `MockDB` just override `__init__` to call `self._connection_build(':memory:') Then you can remove all ifs here
cis-muzahid (Migrated from github.com) reviewed 2021-08-19 09:11:41 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-19 09:11:41 +02:00

change into
'''update settings set value=value+1 WHERE key='version';'''

change into ```'''update settings set value=value+1 WHERE key='version';'''```
cis-muzahid (Migrated from github.com) reviewed 2021-08-19 10:13:34 +02:00
src/class_sqlThread.py
@ -641,0 +397,4 @@
_translate(
"MainWindow",
"Disk full"),
_translate(
cis-muzahid (Migrated from github.com) commented 2021-08-19 10:13:34 +02:00

Applied setter and getter

Applied setter and getter
cis-muzahid (Migrated from github.com) reviewed 2021-08-19 10:23:46 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-19 10:23:46 +02:00

removed current_level

removed current_level
cis-muzahid (Migrated from github.com) reviewed 2021-08-19 10:24:53 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-19 10:24:52 +02:00

Now its

    def sql_schema_version(self):
        """
            Update version with one level
        """
        item = '''update settings set value=value+1 WHERE key='version';'''
        self.cur.execute(item)
        self._current_level = self.__get_current_settings_version()
Now its ``` @sql_schema_version.setter def sql_schema_version(self): """ Update version with one level """ item = '''update settings set value=value+1 WHERE key='version';''' self.cur.execute(item) self._current_level = self.__get_current_settings_version() ```
cis-muzahid (Migrated from github.com) reviewed 2021-08-19 10:25:37 +02:00
src/class_sqlThread.py
@ -641,0 +398,4 @@
"MainWindow",
"Disk full"),
_translate(
"MainWindow",
cis-muzahid (Migrated from github.com) commented 2021-08-19 10:25:37 +02:00

passed while initialise

passed while initialise
cis-muzahid (Migrated from github.com) reviewed 2021-08-19 10:26:59 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-19 10:26:59 +02:00

Done

Done
PeterSurda (Migrated from github.com) requested changes 2021-08-19 11:05:06 +02:00
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-08-19 10:51:57 +02:00

get rid of self._current_level

get rid of `self._current_level`
PeterSurda (Migrated from github.com) commented 2021-08-19 10:56:49 +02:00

let's call it upgrade_schema_if_old_version

let's call it `upgrade_schema_if_old_version`
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
PeterSurda (Migrated from github.com) commented 2021-08-19 10:41:28 +02:00

make it a class variable

make it a class variable
PeterSurda (Migrated from github.com) commented 2021-08-19 10:43:19 +02:00 
self._connection_build()
``` self._connection_build() ```
PeterSurda (Migrated from github.com) commented 2021-08-19 10:43:25 +02:00

no static method

no static method
PeterSurda (Migrated from github.com) commented 2021-08-19 10:43:50 +02:00

no return, just directly assign the self.con and self.cur here

no return, just directly assign the `self.con` and `self.cur` here
PeterSurda (Migrated from github.com) commented 2021-08-19 10:44:12 +02:00

also declare

self.cur = None
self.con = None
also declare ``` self.cur = None self.con = None ```
PeterSurda (Migrated from github.com) commented 2021-08-19 10:44:49 +02:00

rename to

def __connection_build_internal(self, file_name="messages.dat")
rename to ``` def __connection_build_internal(self, file_name="messages.dat") ```
PeterSurda (Migrated from github.com) commented 2021-08-19 10:52:44 +02:00

this should be internal to the object

this should be internal to the object
PeterSurda (Migrated from github.com) commented 2021-08-19 10:53:56 +02:00

get rid of self._current_level

get rid of `self._current_level`
PeterSurda (Migrated from github.com) commented 2021-08-19 10:55:01 +02:00

no need for caching, this is uses only during startup and tests, and only a dozen times, no need to optimize, just use the property

no need for caching, this is uses only during startup and tests, and only a dozen times, no need to optimize, just use the property
src/class_sqlThread.py
@ -49,1 +69,4 @@
self.initialise_sql("init_version_{}".format(file_name))
def initialise_sql(self, file_name):
try:
PeterSurda (Migrated from github.com) commented 2021-08-19 10:45:33 +02:00

add a new method

def _connection_build(self)
    self.__connection_build_internal(self, 'messages.dat')
add a new method ``` def _connection_build(self) self.__connection_build_internal(self, 'messages.dat') ````
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-08-19 10:59:40 +02:00 
def check_colums_can_store_binary_null(self)
``` def check_colums_can_store_binary_null(self) ```
PeterSurda (Migrated from github.com) commented 2021-08-19 11:01:04 +02:00

write a meanignful docstring

write a meanignful docstring
src/class_sqlThread.py
@ -641,0 +397,4 @@
_translate(
"MainWindow",
"Disk full"),
_translate(
PeterSurda (Migrated from github.com) commented 2021-08-19 10:48:11 +02:00

at the bottom add a new class

class TestDB(UpgradeDB):
    def _connection_build(self):
        self.__connection_build_internal(':memory:')
at the bottom add a new class ``` class TestDB(UpgradeDB): def _connection_build(self): self.__connection_build_internal(':memory:') ```
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 19:59:35 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 19:59:35 +02:00

applied changes method name to __connection_build_internal and pass file_name

applied changes method name to ```__connection_build_internal``` and pass file_name
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 22:43:48 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 22:43:48 +02:00

Its changed into

        self.cur.execute(item, self.__get_current_settings_version() + 1)

Because its not taking value+1

Its changed into ``` item = '''update settings set value=? WHERE key='version';''' self.cur.execute(item, self.__get_current_settings_version() + 1) ``` Because its not taking ```value+1```
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 22:46:43 +02:00
src/class_sqlThread.py
@ -641,0 +398,4 @@
"MainWindow",
"Disk full"),
_translate(
"MainWindow",
cis-muzahid (Migrated from github.com) commented 2021-08-23 22:46:43 +02:00

Changed into initialize_schema

Changed into ```initialize_schema```
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 22:48:01 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 22:48:01 +02:00

Changed

Changed
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:01:37 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:01:37 +02:00 
        self._current_level = None
        self.max_level = 11
        self.conn = None
        self.cur = None
        self.__connection_build_internal(db_name)

    def __connection_build_internal(self, file_name="messages.dat"):
        """
            Stablish SQL connection
        """

        if file_name == "memory":
            self.conn = sqlite3.connect(':memory:')
        else:
            self.conn = sqlite3.connect(state.appdata + file_name)
        self.conn.text_factory = str
        self.cur = self.conn.cursor()
``` def __init__(self, db_name="messages.dat"): self._current_level = None self.max_level = 11 self.conn = None self.cur = None self.__connection_build_internal(db_name) def __connection_build_internal(self, file_name="messages.dat"): """ Stablish SQL connection """ if file_name == "memory": self.conn = sqlite3.connect(':memory:') else: self.conn = sqlite3.connect(state.appdata + file_name) self.conn.text_factory = str self.cur = self.conn.cursor() ```
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:02:18 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:02:18 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:02:50 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:02:50 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:03:31 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:03:31 +02:00

Removed static method

Removed static method
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:04:04 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:04:04 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:04:15 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:04:15 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:04:27 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:04:26 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:22:56 +02:00
src/class_sqlThread.py
@ -641,0 +398,4 @@
"MainWindow",
"Disk full"),
_translate(
"MainWindow",
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:22:56 +02:00

changed to upgrade_schema_if_old_version

changed to ```upgrade_schema_if_old_version```
cis-muzahid (Migrated from github.com) reviewed 2021-08-23 23:23:50 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-23 23:23:50 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-08-26 09:51:10 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-26 09:51:10 +02:00

its changed into

        self.cur.execute(item, self.__get_current_settings_version() + 1)
its changed into ``` item = '''update settings set value=? WHERE key='version';''' self.cur.execute(item, self.__get_current_settings_version() + 1) ```
cis-muzahid (Migrated from github.com) reviewed 2021-08-26 09:52:33 +02:00
src/class_sqlThread.py
cis-muzahid (Migrated from github.com) commented 2021-08-26 09:52:32 +02:00

Changed def name to upgrade_schema_if_old_version

Changed def name to ```upgrade_schema_if_old_version```
PeterSurda (Migrated from github.com) requested changes 2021-08-30 10:56:16 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
PeterSurda (Migrated from github.com) commented 2021-08-30 10:27:37 +02:00 
def __connection_build_internal(self, file_name='messages.dat', memory=False)
    if memory:
         self.conn = sqlite3.connect(':memory:')
    else
         self.conn = sqlite3.connect(os.path.join(state.appdata, file_name))
``` def __connection_build_internal(self, file_name='messages.dat', memory=False) if memory: self.conn = sqlite3.connect(':memory:') else self.conn = sqlite3.connect(os.path.join(state.appdata, file_name)) ```
PeterSurda (Migrated from github.com) commented 2021-08-30 10:34:11 +02:00

use getter

use getter
PeterSurda (Migrated from github.com) commented 2021-08-30 10:39:28 +02:00 
increment = True

while self.settings_version < self.max_level:
   self._upgrade()
   self.settings_version = increment

and create an empty sql script for level 1 (one that doesn't change anything), like SELECT NULL

``` increment = True while self.settings_version < self.max_level: self._upgrade() self.settings_version = increment ``` and create an empty sql script for level 1 (one that doesn't change anything), like `SELECT NULL`
src/class_sqlThread.py
@ -49,1 +69,4 @@
self.initialise_sql("init_version_{}".format(file_name))
def initialise_sql(self, file_name):
try:
PeterSurda (Migrated from github.com) commented 2021-08-30 10:47:14 +02:00 
def _connection_build(self, db_name='messages.dat'):
    self.__connection_build_internal(file_name=db_name)
``` def _connection_build(self, db_name='messages.dat'): self.__connection_build_internal(file_name=db_name) ```
src/class_sqlThread.py
@ -641,0 +407,4 @@
'Major error occurred when trying to execute a SQL statement within the sqlThread.'
' Please tell Atheros about this error message or post it in the forum!'
' Error occurred while trying to execute statement: "%s" Here are the parameters;'
' you might want to censor this data with asterisks (***)'
PeterSurda (Migrated from github.com) commented 2021-08-30 10:42:45 +02:00

make this into a separate method

make this into a separate method
PeterSurda (Migrated from github.com) commented 2021-08-30 10:47:51 +02:00

no

no
PeterSurda (Migrated from github.com) commented 2021-08-30 10:48:08 +02:00 
class TestDB(UpgradeDB)
``` class TestDB(UpgradeDB) ```
PeterSurda (Migrated from github.com) commented 2021-08-30 10:48:56 +02:00 
def _connection_build(self, db_name=None)
    self.__connection_build_internal(memory=True)
``` def _connection_build(self, db_name=None) self.__connection_build_internal(memory=True) ```
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
PeterSurda (Migrated from github.com) commented 2021-08-30 10:45:24 +02:00

should be called something like TestUpgradeDB

should be called something like `TestUpgradeDB`
PeterSurda (Migrated from github.com) commented 2021-08-30 10:52:53 +02:00 
self.db = TestDB()
res = self.db.execute()
self.assertwhatever
``` self.db = TestDB() res = self.db.execute() self.assertwhatever ```
PeterSurda (Migrated from github.com) commented 2021-08-30 10:54:12 +02:00

maybe here in the decorator create the self.db object

maybe here in the decorator create the `self.db` object
PeterSurda (Migrated from github.com) requested changes 2021-08-31 09:42:21 +02:00
src/sql/init_version_2.sql
PeterSurda (Migrated from github.com) commented 2021-08-31 09:32:39 +02:00

receivedtime and payload are swapped

`receivedtime` and `payload` are swapped
PeterSurda (Migrated from github.com) commented 2021-08-31 09:33:23 +02:00

receivedtime and payload are swapped

`receivedtime` and `payload` are swapped
cis-muzahid (Migrated from github.com) reviewed 2021-09-02 14:44:49 +02:00
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
cis-muzahid (Migrated from github.com) commented 2021-09-02 14:44:48 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-09-02 17:31:12 +02:00
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
cis-muzahid (Migrated from github.com) commented 2021-09-02 17:31:12 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-09-02 17:31:23 +02:00
src/class_sqlThread.py
@ -49,1 +69,4 @@
self.initialise_sql("init_version_{}".format(file_name))
def initialise_sql(self, file_name):
try:
cis-muzahid (Migrated from github.com) commented 2021-09-02 17:31:23 +02:00

Done

Done
cis-muzahid (Migrated from github.com) reviewed 2021-09-02 17:31:39 +02:00
src/class_sqlThread.py
@ -641,0 +407,4 @@
'Major error occurred when trying to execute a SQL statement within the sqlThread.'
' Please tell Atheros about this error message or post it in the forum!'
' Error occurred while trying to execute statement: "%s" Here are the parameters;'
' you might want to censor this data with asterisks (***)'
cis-muzahid (Migrated from github.com) commented 2021-09-02 17:31:39 +02:00

Changed approach

Changed approach
cis-muzahid (Migrated from github.com) reviewed 2021-09-02 17:31:49 +02:00
src/class_sqlThread.py
@ -641,0 +407,4 @@
'Major error occurred when trying to execute a SQL statement within the sqlThread.'
' Please tell Atheros about this error message or post it in the forum!'
' Error occurred while trying to execute statement: "%s" Here are the parameters;'
' you might want to censor this data with asterisks (***)'
cis-muzahid (Migrated from github.com) commented 2021-09-02 17:31:49 +02:00

Done

Done
PeterSurda (Migrated from github.com) requested changes 2021-09-07 11:08:15 +02:00
src/class_sqlThread.py
@ -24,4 +23,3 @@
from .addresses import encodeAddress
from .bmconfigparser import config, config_ready
from .debug import logger
from .tr import _translate
PeterSurda (Migrated from github.com) commented 2021-09-07 10:27:53 +02:00

os.path.join

`os.path.join`
PeterSurda (Migrated from github.com) commented 2021-09-07 10:30:29 +02:00

not upgrade, initialize

not `upgrade`, `initialize`
PeterSurda (Migrated from github.com) commented 2021-09-07 10:30:50 +02:00

also initialize instead of upgrade

also `initialize` instead of `upgrade`
PeterSurda (Migrated from github.com) commented 2021-09-07 10:34:56 +02:00

don't exit, log an error message

don't exit, log an error message
src/class_sqlThread.py
@ -93,0 +84,4 @@
def sql_schema_version(self):
"""
Getter for get current schema version
"""
PeterSurda (Migrated from github.com) commented 2021-09-07 10:28:39 +02:00

this should remain here

this should remain here
src/class_sqlThread.py
@ -96,3 +90,1 @@
# the settings version to 4.
settingsversion = config.getint(
'bitmessagesettings', 'settingsversion')
@sql_schema_version.setter
PeterSurda (Migrated from github.com) commented 2021-09-07 11:02:07 +02:00

To make life easier, this should remain.

To make life easier, this should remain.
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-09-07 10:39:06 +02:00

columns

`columns`
PeterSurda (Migrated from github.com) commented 2021-09-07 10:41:14 +02:00 
Check if sqlite can store binary zeroes
``` Check if sqlite can store binary zeroes ```
src/class_sqlThread.py
@ -641,0 +401,4 @@
"MainWindow",
'Alert: Your disk or data storage volume is full. Bitmessage will now exit.'),
True)))
os._exit(0)
PeterSurda (Migrated from github.com) commented 2021-09-07 10:52:34 +02:00

you can put it together with the vaccum time update

you can put it together with the vaccum time update
PeterSurda (Migrated from github.com) commented 2021-09-07 10:55:38 +02:00 
while self.sql_config_settings_version < config_setting_version_max:
    upgrade_one_level(self.sql_config_settings_version)
    self.sql_config_settings_version += 1
``` while self.sql_config_settings_version < config_setting_version_max: upgrade_one_level(self.sql_config_settings_version) self.sql_config_settings_version += 1 ```
src/class_sqlThread.py
@ -641,0 +407,4 @@
'Major error occurred when trying to execute a SQL statement within the sqlThread.'
' Please tell Atheros about this error message or post it in the forum!'
' Error occurred while trying to execute statement: "%s" Here are the parameters;'
' you might want to censor this data with asterisks (***)'
PeterSurda (Migrated from github.com) commented 2021-09-07 10:52:00 +02:00

CAST(strftime('%s', 'now') AS STR)

```CAST(strftime('%s', 'now') AS STR)```
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
PeterSurda (Migrated from github.com) commented 2021-09-07 11:05:10 +02:00

remove this

remove this
PeterSurda (Migrated from github.com) commented 2021-09-07 11:05:17 +02:00

remove this

remove this
PeterSurda (Migrated from github.com) commented 2021-09-07 11:05:24 +02:00

remove this method

remove this method
PeterSurda (Migrated from github.com) commented 2021-09-07 11:05:48 +02:00 
cls.test_db = TestDB()
``` cls.test_db = TestDB() ```
PeterSurda (Migrated from github.com) commented 2021-09-07 11:06:28 +02:00

delete his method

delete his method
PeterSurda (Migrated from github.com) commented 2021-09-07 11:06:36 +02:00

delete this method

delete this method
PeterSurda (Migrated from github.com) requested changes 2021-09-16 10:00:22 +02:00
PeterSurda (Migrated from github.com) left a comment

There are still "DEFAULT" and "NOT NULL" declarations in some places where they are missing in the old ones.

There are still "DEFAULT" and "NOT NULL" declarations in some places where they are missing in the old ones.
src/sql/run.sql
PeterSurda (Migrated from github.com) commented 2021-09-16 09:56:21 +02:00

this file isn't used

this file isn't used
PeterSurda (Migrated from github.com) reviewed 2021-09-22 10:41:48 +02:00
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-09-22 10:41:47 +02:00

if table inbox not already exists, then initialize_schema

if table inbox not already exists, then initialize_schema
PeterSurda commented 2021-09-29 08:58:44 +02:00 (Migrated from github.com)
Copy Link

Fixes #1710

Fixes #1710
PeterSurda (Migrated from github.com) requested changes 2021-09-29 09:11:04 +02:00
PeterSurda (Migrated from github.com) left a comment

The comments in the sql/init_version_*.sql are not helpful. They just repeat the statements in different words. Instead, the original comments should be kept.

The comments in the `sql/init_version_*.sql` are not helpful. They just repeat the statements in different words. Instead, the original comments should be kept.
PeterSurda (Migrated from github.com) reviewed 2021-09-29 09:18:14 +02:00
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
PeterSurda (Migrated from github.com) commented 2021-09-29 09:18:13 +02:00

why do you need this? how about just using cls.test_db.conn and cls.test_db.cur?

why do you need this? how about just using `cls.test_db.conn` and `cls.test_db.cur`?
PeterSurda commented 2021-09-29 09:49:08 +02:00 (Migrated from github.com)
Copy Link

check_vacuumed should end at line 229. Line 231 should be moved to parent method, and the while loop that follows should have its own function and also be called from parent.

`check_vacuumed` should end at line 229. Line 231 should be moved to parent method, and the `while` loop that follows should have its own function and also be called from parent.
PeterSurda (Migrated from github.com) reviewed 2021-09-29 10:13:11 +02:00
src/class_sqlThread.py
@ -641,0 +345,4 @@
'(while movemessagstoappdata) Alert: Your disk or data storage volume is full.'
' sqlThread will now exit.')
queues.UISignalQueue.put((
'alert', (
PeterSurda (Migrated from github.com) commented 2021-09-29 10:13:11 +02:00

vacuum not vaccum

`vacuum` not `vaccum`
PeterSurda (Migrated from github.com) reviewed 2021-09-29 10:22:31 +02:00
setup.py
PeterSurda (Migrated from github.com) commented 2021-09-29 10:22:31 +02:00

sql/*.py, sql/*.sql so that it doesn't pick up things like __init__.pyc

`sql/*.py`, `sql/*.sql` so that it doesn't pick up things like `__init__.pyc`
PeterSurda (Migrated from github.com) reviewed 2021-09-30 13:38:48 +02:00
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-09-30 13:38:48 +02:00 
# Now we're ready to process queue
``` # Now we're ready to process queue ```
PeterSurda (Migrated from github.com) reviewed 2021-10-04 08:43:39 +02:00
src/tests/sql/init_version_5.sql
PeterSurda (Migrated from github.com) commented 2021-10-04 08:43:39 +02:00

I think the table was deleted in a later version.

I think the table was deleted in a later version.
PeterSurda (Migrated from github.com) requested changes 2021-10-05 08:53:11 +02:00
src/class_sqlThread.py
@ -594,0 +229,4 @@
logger.info('Created messages database file')
except Exception as err:
if str(err) == 'table inbox already exists':
logger.debug('Database file already exists.')
PeterSurda (Migrated from github.com) commented 2021-10-05 08:51:46 +02:00

something like this

while self.loop_queue():
    pass
something like this ``` while self.loop_queue(): pass ```
PeterSurda (Migrated from github.com) commented 2021-10-05 08:52:19 +02:00 
return False
``` return False ```
PeterSurda (Migrated from github.com) requested changes 2021-10-07 06:57:52 +02:00
PeterSurda (Migrated from github.com) left a comment

SQL statements mostly ok, couple of minor changes requested.

SQL statements mostly ok, couple of minor changes requested.
src/sql/init_version_9.sql
PeterSurda (Migrated from github.com) commented 2021-10-07 06:45:22 +02:00

time is a reserved word, should use

`time`

instead, just in case.

`time` is a reserved word, should use ``` `time` ``` instead, just in case.
PeterSurda (Migrated from github.com) commented 2021-10-07 06:45:39 +02:00

here too

here too
src/sql/sent_messages.sql
PeterSurda (Migrated from github.com) commented 2021-10-07 06:51:59 +02:00

duplicate of upg_sc_if_old_ver_2, delete file.

duplicate of `upg_sc_if_old_ver_2`, delete file.
src/sql/upg_sc_if_old_ver_1.sql
PeterSurda (Migrated from github.com) commented 2021-10-07 06:53:37 +02:00

remove DEFAULT, should be the same as original, in the whole file.

remove `DEFAULT`, should be the same as original, in the whole file.
PeterSurda (Migrated from github.com) commented 2021-10-07 06:54:27 +02:00 
`time`

also below once again

``` `time` ``` also below once again
PeterSurda (Migrated from github.com) reviewed 2021-10-07 07:02:19 +02:00
src/tests/core.py
PeterSurda (Migrated from github.com) commented 2021-10-07 07:02:18 +02:00

Here sqlExecute so that we test it too.

Here `sqlExecute` so that we test it too.
PeterSurda (Migrated from github.com) reviewed 2021-10-07 07:02:58 +02:00
src/tests/core.py
PeterSurda (Migrated from github.com) commented 2021-10-07 07:02:58 +02:00

use sqlExecute here and check it's return value (number of modified rows, i.e. should be 1 here)

use `sqlExecute` here and check it's return value (number of modified rows, i.e. should be `1` here)
PeterSurda commented 2021-10-08 08:43:20 +02:00 (Migrated from github.com)
Copy Link

By the way, the core test suite reports an interesting issue here: https://buildbot.bitmessage.org/#builders/25/builds/1515

This has been fixed now, as most of the other things you pointed out. It should be ready for you to review in a couple of days, I'll let you know.

> By the way, the core test suite reports an interesting issue here: https://buildbot.bitmessage.org/#builders/25/builds/1515 This has been fixed now, as most of the other things you pointed out. It should be ready for you to review in a couple of days, I'll let you know.
🎉 1
PeterSurda (Migrated from github.com) reviewed 2021-10-13 08:48:08 +02:00
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
PeterSurda (Migrated from github.com) commented 2021-10-13 08:48:08 +02:00

67-70 not needed anymore

67-70 not needed anymore
PeterSurda (Migrated from github.com) reviewed 2021-10-13 08:49:02 +02:00
src/tests/sql/create_function.sql
PeterSurda (Migrated from github.com) commented 2021-10-13 08:49:02 +02:00

remove file

remove file
PeterSurda (Migrated from github.com) reviewed 2021-10-13 08:49:40 +02:00
src/tests/test_sqlthread.py
@ -45,0 +84,4 @@
and Upgrade DB schema for all versions
"""
def wrapper(*args):
"""
PeterSurda (Migrated from github.com) commented 2021-10-13 08:49:40 +02:00 
# setup
self.test_db.create_function()
``` # setup self.test_db.create_function() ```
PeterSurda (Migrated from github.com) reviewed 2021-10-13 08:51:25 +02:00
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
PeterSurda (Migrated from github.com) commented 2021-10-13 08:51:25 +02:00

just remove this section, no comment

just remove this section, no comment
PeterSurda (Migrated from github.com) reviewed 2021-10-13 08:51:37 +02:00
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
PeterSurda (Migrated from github.com) commented 2021-10-13 08:51:37 +02:00

duplicate line

duplicate line
PeterSurda (Migrated from github.com) reviewed 2021-10-13 08:52:21 +02:00
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
PeterSurda (Migrated from github.com) commented 2021-10-13 08:52:21 +02:00

this is not helpful

this is not helpful
PeterSurda (Migrated from github.com) requested changes 2021-10-13 08:53:15 +02:00
PeterSurda (Migrated from github.com) left a comment

Please make the changes.

Please make the changes.
PeterSurda (Migrated from github.com) reviewed 2021-10-14 08:38:06 +02:00
src/tests/test_sqlthread.py
@ -2,43 +2,214 @@
# flake8: noqa:E402
import os
import tempfile
PeterSurda (Migrated from github.com) commented 2021-10-14 08:38:06 +02:00

setUp shouldn't be a class method

`setUp` shouldn't be a class method
PeterSurda (Migrated from github.com) requested changes 2021-10-22 09:13:00 +02:00
PeterSurda (Migrated from github.com) left a comment

Remove obsolete comments.

Remove obsolete comments.
PeterSurda (Migrated from github.com) reviewed 2021-10-26 07:37:48 +02:00
src/class_sqlThread.py
@ -423,0 +131,4 @@
def check_columns_can_store_binary_null(self): # pylint: disable=too-many-locals,
# too-many-branches, too-many-statements
"""
Check if sqlite can store binary zeros.
PeterSurda (Migrated from github.com) commented 2021-10-26 07:37:48 +02:00

why line 149 and 150? Makes no sense to put it here.

why line 149 and 150? Makes no sense to put it here.
g1itch (Migrated from github.com) reviewed 2021-11-24 18:13:05 +01:00
g1itch (Migrated from github.com) left a comment

Barely readable ); The tests/sql seems duplicating sql

Barely readable ); The `tests/sql` seems duplicating `sql`
setup.py
g1itch (Migrated from github.com) commented 2021-11-24 17:41:02 +01:00

I don't understand the purpose of this empty sql/__init__.py and especially listing it in the package_data.

I don't understand the purpose of this empty `sql/__init__.py` and especially listing it in the package_data.
src/class_sqlThread.py
@ -4,3 +4,4 @@
import logging
import os
import shutil # used for moving the messages.dat file
g1itch (Migrated from github.com) commented 2021-11-24 17:41:19 +01:00

isort

isort
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
g1itch (Migrated from github.com) commented 2021-11-24 17:42:13 +01:00

strange formatting

strange formatting
g1itch (Migrated from github.com) commented 2021-11-24 17:43:11 +01:00

No need for semicolon

No need for semicolon
g1itch (Migrated from github.com) commented 2021-11-24 17:43:46 +01:00

Why are you introducing more pylint warnings here?

Why are you introducing more pylint warnings here?
g1itch (Migrated from github.com) commented 2021-11-24 17:46:31 +01:00

I think logger.debug('Error while trying to initialize...', exc_info=True) is enough here. No need for sys.stderr.write() and err.

I think `logger.debug('Error while trying to initialize...', exc_info=True)` is enough here. No need for `sys.stderr.write()` and err.
g1itch (Migrated from github.com) commented 2021-11-24 17:49:08 +01:00

It doesn't look like the method docstring

It doesn't look like the method docstring
src/class_sqlThread.py
@ -576,0 +220,4 @@
def initialize_schema(self):
"""
Initialise Db schema
"""
g1itch (Migrated from github.com) commented 2021-11-24 17:52:55 +01:00

I don't understand this os._exit(0). Why don't you use sys.exit('Error while trying to create database file..')? What is it mean "in1111"?

I don't understand this `os._exit(0)`. Why don't you use `sys.exit('Error while trying to create database file..')`? What is it mean "in1111"?
g1itch (Migrated from github.com) commented 2021-11-24 17:54:43 +01:00

BMConfigParser (obj)

`BMConfigParser` (obj)
src/class_sqlThread.py
g1itch (Migrated from github.com) commented 2021-11-24 17:57:10 +01:00

Weird formatting

Weird formatting
src/class_sqlThread.py
g1itch (Migrated from github.com) commented 2021-11-24 17:58:41 +01:00

These casts seem redundant

These casts seem redundant
src/tests/core.py
g1itch (Migrated from github.com) commented 2021-11-24 18:02:15 +01:00

"SELECT * FROM testtbl"

"SELECT * FROM testtbl"
g1itch (Migrated from github.com) commented 2021-11-24 18:03:44 +01:00

What it tests?

What it tests?
src/tests/sql/init_version_7.sql
g1itch (Migrated from github.com) commented 2021-11-24 17:38:13 +01:00

strange expression

strange expression
g1itch (Migrated from github.com) reviewed 2021-12-09 16:55:12 +01:00
g1itch (Migrated from github.com) left a comment

I see no point in another review since my previous comments are mostly ignored.

I see no point in another review since my previous comments are mostly ignored.
PeterSurda (Migrated from github.com) reviewed 2021-12-10 08:22:23 +01:00
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-12-10 08:22:22 +01:00

There are two exception hanlders, one for IOError (disk failure, disk full), and everything else. We may want to handle these situations differently, and we don't know what all kinds of exceptions can be thrown, so I recommended doing it this way.

There are two exception hanlders, one for IOError (disk failure, disk full), and everything else. We may want to handle these situations differently, and we don't know what all kinds of exceptions can be thrown, so I recommended doing it this way.
PeterSurda (Migrated from github.com) reviewed 2021-12-10 08:23:11 +01:00
src/class_sqlThread.py
@ -17,128 +17,100 @@ try:
import state
PeterSurda (Migrated from github.com) commented 2021-12-10 08:23:11 +01:00

There was a problem, we found out what it is, will be fixed in next commit.

There was a problem, we found out what it is, will be fixed in next commit.
PeterSurda (Migrated from github.com) reviewed 2021-12-10 08:26:22 +01:00
src/class_sqlThread.py
@ -488,3 +207,3 @@
True)))
os._exit(0)
item = '''update settings set value=? WHERE key='lastvacuumtime';'''
query = "update settings set value=? WHERE key='lastvacuumtime'"
PeterSurda (Migrated from github.com) commented 2021-12-10 08:26:22 +01:00

item is a bad name. Use query.

`item` is a bad name. Use `query`.
PeterSurda (Migrated from github.com) reviewed 2021-12-10 08:28:31 +01:00
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2021-12-10 08:28:31 +01:00

I tend to agree. Something like traceback.format_exc would be better.

I tend to agree. Something like `traceback.format_exc` would be better.
PeterSurda (Migrated from github.com) reviewed 2021-12-10 08:30:18 +01:00
src/tests/sql/init_version_5.sql
PeterSurda (Migrated from github.com) commented 2021-12-10 08:30:17 +01:00

you're right, if it's removed and there is an error, it means there is a bug somewhere

you're right, if it's removed and there is an error, it means there is a bug somewhere
PeterSurda (Migrated from github.com) reviewed 2021-12-10 08:33:38 +01:00
src/tests/sql/init_version_5.sql
PeterSurda (Migrated from github.com) commented 2021-12-10 08:33:38 +01:00

It looks like some weird leftover of deleted code. Since the current upgrade code deletes it, we should keep it, otherwise we'd have to test against ancient versions.

It looks like some weird leftover of deleted code. Since the current upgrade code deletes it, we should keep it, otherwise we'd have to test against ancient versions.
PeterSurda (Migrated from github.com) reviewed 2021-12-10 09:14:24 +01:00
src/tests/sql/init_version_7.sql
PeterSurda (Migrated from github.com) commented 2021-12-10 09:14:24 +01:00

I looked through the tests and instructed to redesign how they perform setup.

I looked through the tests and instructed to redesign how they perform setup.
PeterSurda (Migrated from github.com) requested changes 2022-01-18 06:23:30 +01:00
PeterSurda (Migrated from github.com) left a comment

There are still a couple of unresolved conversations.

There are still a couple of unresolved conversations.
setup.py
PeterSurda (Migrated from github.com) commented 2022-01-18 06:18:47 +01:00

The file was removed, but the reference in setup.py still persists. @cis-muzahid please delete this reference, and also delete the file src/tests/sql/__init__.py, that is also unnecessary.

The file was removed, but the reference in `setup.py` still persists. @cis-muzahid please delete this reference, and also delete the file `src/tests/sql/__init__.py`, that is also unnecessary.
src/class_sqlThread.py
PeterSurda (Migrated from github.com) commented 2022-01-18 06:22:19 +01:00

I agree

I agree
PeterSurda (Migrated from github.com) requested changes 2022-03-16 10:37:15 +01:00
PeterSurda (Migrated from github.com) left a comment

The PR shouldn't remove the *.sql files. It should ignore them.

The PR shouldn't remove the `*.sql` files. It should ignore them.
PeterSurda (Migrated from github.com) reviewed 2022-03-17 07:38:40 +01:00
PeterSurda (Migrated from github.com) left a comment

unstage the sql files

**unstage** the sql files
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
g1itch
PeterSurda
Labels
Clear labels   
bug

Something isn't working as it's supposed to

  
build

Related to building or build system

  
dependencies

Pull requests that update a dependency file

  
developers

The issue is relevant mainly for developers rather than users

  
documentation

   
duplicate

   
enhancement

New feature

  
formatting

Code or UI text formatting

  
invalid

   
legal

   
mobile

Frontend for mobile devices (kivy)

  
obsolete

   
packaging

   
performance

   
protocol

Protocol change

  
question

   
refactoring

   
regression

Some issue, fixed in the past, appears again

  
security

   
test

   
translation

   
usability

   
wontfix
No Label
bug
build
dependencies
developers
documentation
duplicate
enhancement
formatting
invalid
legal
mobile
obsolete
packaging
performance
protocol
question
refactoring
regression
security
test
translation
usability
wontfix
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Bitmessage/PyBitmessage-2024-12-05#1794
No description provided.
Delete Branch "cis-muzahid/test_refactor_sqlthread"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?