Refactor sqlthread #1760

cis-muzahid · 2021-05-05T18:08:44Z

No description provided.

PeterSurda

Too big, can't review. Needs to be split. Separate code quality, support functions, and the individual levels.

PeterSurda · 2021-06-02T07:11:32Z

stdeb.cfg

 Package: pybitmessage
 Section: net
-Build-Depends: dh-python, libssl-dev, python-all-dev, python-setuptools
+Build-Depends: dh-python, libssl-dev, python-all-dev, python-setuptools, python-six


this should be a separate PR

It is 16a1177 merged from v0.6.

Is it fine now?

PeterSurda · 2021-06-02T07:12:28Z

src/helper_startup.py

    get_plugin = None

+# pylint: disable=too-many-branches,too-many-statements
+


is this a code quality only change? Then it should go into a separate PR.

PeterSurda · 2021-06-03T23:41:32Z

The upgrade design is too complicated. There is no need for a separate function per level.

PeterSurda · 2021-06-09T08:35:34Z

src/class_sqlThread.py

+            self.upgrade_one_level(l)
+            self.run_migrations(l)
+
+    def upgrade_schema_data_level(self, level):


```def increment_settings_version``

PeterSurda · 2021-06-09T08:36:32Z

src/class_sqlThread.py

+        method = getattr(self, method_name, lambda: "Invalid version")
+        return method()
+
+    def run_migrations(self, file_name):


def _upgrade_one_level_sql_statement

PeterSurda · 2021-06-09T08:37:02Z

src/class_sqlThread.py

+        self.current_level = None
+        self.max_level = 11
+
+    def get_current_level(self):


def __get_current_settings_version
Maybe this can't be private, that's ok.

PeterSurda · 2021-06-09T08:37:20Z

src/class_sqlThread.py

+        self.cur.execute(item, parameters)
+        return int(self.cur.fetchall()[0][0])
+
+    def upgrade_one_level(self, level):


def _upgrade_one_level_method

PeterSurda · 2021-06-09T08:50:01Z

src/class_sqlThread.py

        settingsversion = BMConfigParser().getint(
            'bitmessagesettings', 'settingsversion')

+        # Process SQL queries first20bytesofencryptedmessage in table inventory


this section can probably be deleted

PeterSurda · 2021-06-09T08:51:18Z

src/class_sqlThread.py

+            parameters = (2,)
+            self.cur.execute(item, parameters)
+
        # People running earlier versions of PyBitmessage do not have the


lines 201-227 can be made into a separate method

lines 227-292 can be split into 2 or 3 methods.

Done, please check the method name.

PeterSurda · 2021-06-10T09:06:05Z

src/tests/test_sqlthread.py

+
+            # Test versions
+            upgrade_db = UpgradeDB()
+            upgrade_db._upgrade_one_level_sql_statement(int(version))


# pylint: disable=protected-access

PeterSurda · 2021-06-24T03:19:46Z

rebase
make the versions into properties
rename versioning into something else, inside the methods you can use version, as a decorator perhaps sql_schema_upgrade
delete obsolete stuff
I haven't thoroughly reviewed tests but for starters they are probably ok
rename the version 1 init/test to version 3. I checked the original source and it may break things if you're upgrading from version 1. If you just rename it to 3, then it would only break if we stopped the upgrade at version 2, which isn't a practical problem. You just need to make sure version 2 and 3 tests still succeeds.

PeterSurda

Please make the requested changes.

PeterSurda · 2021-06-24T02:58:45Z

src/class_sqlThread.py

+            return None
+
+        # Migrate Db with level
+        method_name = 'upgrade_schema_data_' + str(level)


I don't see this defined anywhere, so delete this method.

PeterSurda · 2021-06-24T03:04:57Z

src/class_sqlThread.py

+        item = '''update settings set value=? WHERE key='version';'''
+        parameters = (level + 1,)
+        self.cur.execute(item, parameters)
+


this should be more transparent. For example, you can use a @property decorator for properties sql_schema_version and configparser_schema_version and use these instead calling the methods directly. And setter can enforce a +1 irrespective of what you pass as an argument.

PeterSurda · 2021-06-24T03:20:30Z

It is certainly an improvement but still work left to do.

cis-muzahid · 2021-11-08T10:21:04Z

Closing because having changes in #1794

cis-muzahid force-pushed the refactor_sqlthread branch from 14b23b8 to b072dc7 Compare May 10, 2021 10:20

cis-muzahid force-pushed the refactor_sqlthread branch from b072dc7 to f63d253 Compare May 18, 2021 16:54

g1itch marked this pull request as draft May 28, 2021 13:06

g1itch mentioned this pull request May 28, 2021

create separate method for each currentVersion of class_sqlThread module #1715

Draft

PeterSurda requested changes Jun 2, 2021

View reviewed changes

cis-muzahid force-pushed the refactor_sqlthread branch 3 times, most recently from 2d9ad93 to 359553b Compare June 3, 2021 15:27

cis-muzahid force-pushed the refactor_sqlthread branch from ad37a9e to 0097965 Compare June 8, 2021 08:25

PeterSurda requested changes Jun 9, 2021

View reviewed changes

PeterSurda reviewed Jun 9, 2021

View reviewed changes

PeterSurda reviewed Jun 10, 2021

View reviewed changes

cis-muzahid force-pushed the refactor_sqlthread branch 2 times, most recently from f335968 to 560c2ba Compare June 10, 2021 16:24

cis-muzahid marked this pull request as ready for review June 11, 2021 11:11

PeterSurda requested changes Jun 24, 2021

View reviewed changes

cis-muzahid added 2 commits June 29, 2021 13:10

refactor sqlthread and add test cases

f6ffd64

rebase and merge conflicts

a00c898

cis-muzahid force-pushed the refactor_sqlthread branch from c04cc05 to a00c898 Compare June 29, 2021 07:56

cis-muzahid added 3 commits June 29, 2021 22:16

fixed code quality stuff

78f90ef

open for py3 porting

1000a2d

update import

3276af8

g1itch marked this pull request as draft June 30, 2021 16:35

cis-muzahid added 3 commits July 7, 2021 21:38

add property setter for upgrade sqlversion

3d29a05

fixed version till 8

edf2b65

fixed

26979b8

g1itch added the duplicate label Aug 13, 2021

cis-muzahid closed this Nov 8, 2021

		get_plugin = None

		# pylint: disable=too-many-branches,too-many-statements

Refactor sqlthread #1760

Refactor sqlthread #1760

Uh oh!

Conversation

cis-muzahid commented May 5, 2021 • edited by g1itch Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PeterSurda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeterSurda commented Jun 3, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeterSurda commented Jun 24, 2021

Uh oh!

PeterSurda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PeterSurda commented Jun 24, 2021

Uh oh!

cis-muzahid commented Nov 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cis-muzahid commented May 5, 2021 •

edited by g1itch

Loading